ABI versioning macros for libc++

[eugenis]

Summary

A few very basic macros that specify the default ABI version of the library, and can be overridden to pick up newer ABI-changing features.

[compnerd]

include/__config

31

This seems unused? The renaming for the major version would be unnecessary if this is not being added.

243

Whats the rationale for renaming this?

[eugenis]

I'm trying to follow this proposal (which people seem to agree with in the general):
http://lists.cs.uiuc.edu/pipermail/cfe-dev/2014-December/040587.html

Renaming _LIBCPP_ALTERNATE_STRING_LAYOUT is to give a unified naming scheme for all ABI-changing features, as proposed in the initial message in the above mail discussion.

[EricWF]

I don't think the tricky part of this patch is actually implementing the ABI macros. The tricky part is defining how libc++ should use the macros. Some questions I would like to see answered:

1. How long is a major and minor ABI version supported?
2. When is the major and minor ABI version bumped?
3. How will maintaining multiple ABI versions affect code complexity and maintainability?

I would want to see documentation accompany this patch that anwsers some of these questions. It could we written in reStructured text after D12129 lands.

[EricWF]

Some more questions:

• Should bumping the ABI major version bump the SO version?
• Should bumping the ABI major version change the include path from include/c++/v1 to include/c++/v2? What kind of clang support do we need to do this?

[eugenis]

Introduced cmake options for specifying the desired ABI version.
ABI version affects library soname and include path (include/c++/vN).
Baked ABI version into the headers (autogenerated __config_version) so that, ex. with -I/usr/include/c++/v2 you get major abi version=2 w/o additional -D settings.

Clang support for selecting vN include path is coming as a separate change.

[eugenis]

How long is a major and minor ABI version supported?

We don't want to bump major version too often, and we want to support both +1 and -1 of the current major version, along with all possible minor versions.

When is the major and minor ABI version bumped?

See Abi.rst for very brief description.

How will maintaining multiple ABI versions affect code complexity and maintainability?

I'm not sure how to answer this. Depends on the feature in question. The logic of selecting features based on the ABI version number should not cause any maintenance burden by itself.

Should bumping the ABI major version bump the SO version?
Should bumping the ABI major version change the include path from include/c++/v1 to include/c++/v2? What kind of clang support do we need to do this?

I've implemented both. We might not need this support now, but it could be useful at the point of v1->v2 migration.

[EricWF]

Thanks for doing all of this work. It's really appreciated.

First, I don't really think we should have the notion of a "minor" ABI version. It will be hard enough to maintain 2 major versions and I don't understand what a minor ABI version buys us.

In my opinion libc++ should support the following ABI configurations:

1. Previous: The previous major ABI version.
2. Default: The current major ABI version.
3. Unstable: The current major ABI version + all pending ABI breaking changes.

I would like to see the ABI minor version replaced with _LIBCPP_ABI_UNSTABLE macro that @mclow.lists originally proposed.

[EricWF]

In D11740#234552, @EricWF wrote:

Thanks for doing all of this work. It's really appreciated.

First, I don't really think we should have the notion of a "minor" ABI version. It will be hard enough to maintain 2 major versions and I don't understand what a minor ABI version buys us.

Woops, I realize now that I suggested a minor version originally. My apologies for being misleading.

One big problem with this patch is that it prevents the libc++ headers in libcxx/include from being used directly. This restriction seems artificial to me. If you want the "default" ABI configuration then you should be able to use the headers in the source tree. Only if you want some custom configuration (either the previous ABI version or _LIBCPP_ABI_UNSTABLE) should you have to use the headers in the build directory. In this case we should use the mechanism in D11963 (whatever that ends up being after review) and not add a __config_version.

[eugenis]

Yes, not being able to use headers in the libcxx source tree is quite unpleasant. It can be fixed by providing a config_version in libcxx/include with the default version values. Or, in the approach of D11963, do something smart in config to default to the right version numbers.

Why do we need _LIBCPP_ABI_UNSTABLE at all? How is it different from setting LIBCPP_ABI_MAJOR_VERSION to the current default version + 1?

[EricWF]

In D11740#234575, @eugenis wrote:

Yes, not being able to use headers in the libcxx source tree is quite unpleasant. It can be fixed by providing a config_version in libcxx/include with the default version values. Or, in the approach of D11963, do something smart in config to default to the right version numbers.

I'm not sure what you mean by "smart" because IMO D11963 is pretty dumb, but I would like to see __config have a default value for _LIBCPP_ABI_VERSION wrapped in a #ifndef _LIBCPP_ABI_VERSION.

Why do we need _LIBCPP_ABI_UNSTABLE at all? How is it different from setting LIBCPP_ABI_MAJOR_VERSION to the current default version + 1?

Interesting question. I'm think trying to draw a distinction between the stable ABI versions and unversioned ABI changes that are currently being staged for the next release. My main concern is that using default version + 1 to stage future changes is that it could look like that is a "stable" ABI configuration.

[eugenis]

In D11740#234610, @EricWF wrote:

In D11740#234575, @eugenis wrote:

Yes, not being able to use headers in the libcxx source tree is quite unpleasant. It can be fixed by providing a config_version in libcxx/include with the default version values. Or, in the approach of D11963, do something smart in config to default to the right version numbers.

I'm not sure what you mean by "smart" because IMO D11963 is pretty dumb, but I would like to see __config have a default value for _LIBCPP_ABI_VERSION wrapped in a #ifndef _LIBCPP_ABI_VERSION.

Yes, that.

Why do we need _LIBCPP_ABI_UNSTABLE at all? How is it different from setting LIBCPP_ABI_MAJOR_VERSION to the current default version + 1?

Interesting question. I'm think trying to draw a distinction between the stable ABI versions and unversioned ABI changes that are currently being staged for the next release. My main concern is that using default version + 1 to stage future changes is that it could look like that is a "stable" ABI configuration.

OK. Then _LIBCPP_ABI_UNSTABLE won't bump the ABI version (as seen in library soname and header path)?

[EricWF]

In D11740#234642, @eugenis wrote:

OK. Then _LIBCPP_ABI_UNSTABLE won't bump the ABI version (as seen in library soname and header path)?

Yeah. That was how I imagined it.

[eugenis]

There is a bit of a problem with testing libc++. If the library is built for the non-default ABI, we can not build tests against headers in libc++ source. And the headers in the build directory are only updated when cmake is re-run. I guess the latter can be fixed by updating the headers with a custom_target instead of a file(COPY).

[EricWF]

In D11740#234760, @eugenis wrote:

There is a bit of a problem with testing libc++. If the library is built for the non-default ABI, we can not build tests against headers in libc++ source. And the headers in the build directory are only updated when cmake is re-run. I guess the latter can be fixed by updating the headers with a custom_target instead of a file(COPY).

Yuck. I'll fix that in D11963.

[eugenis]

Remove minor version, added abi_unstable,.
Keeping __config_version until the other change lands.
Tests use headers from the build directory.

[EricWF]

FYI I'm focusing on getting D11963 in so that it stops blocking this patch.

[EricWF]

FYI I'm focusing on getting D11963 in so that it stops blocking this patch.

[eugenis]

Rebased on D13407

[EricWF]

LGTM but this is still blocked by D13407. Ill let you know once that has landed.

include/__config

242

Is this the last remaining reference to _LIBCPP_ALTERNATIVE_STRING_LAYOUT?
If so please leave a note about how this is the old name and is used here to be backward compatible.

243

I think he's demonstrating the patches functionality and intended usage by converting an existing option. Future ABI options should probably start with the macro prefix '_LIBCPP_ABI' anyway.

[EricWF]

D13407 has landed and this is good to go.

[eugenis]

include/__config

243

Exactly.

[EricWF]

Please address the inline comment. I think with that change we can hold off modifying libc++abi.

test/libcxx/test/config.py

444 ↗

(On Diff #37293)

Please allow abi_version to be optional before committing. IE

abi_version = self.get_lit_conf('abi_version',  None)
if abi_version is not None:
  self.cxx.compile_flags += ['-D_LIBCPP_ABI_VERSION=' + abi_version']

That should allow us to put off the changes to libc++abi.

[jroelofs]

include/__config_site.in

13 ↗

(On Diff #37293)

This doesn't look right to me. What do you want this to expand to when it is defined, and what do you want it to expand to when it is not defined?

[jroelofs]

include/__config_site.in

13 ↗

(On Diff #37293)

Never mind. After re-reading the cmake docs, I see what it's supposed to do.

[eugenis]

test/libcxx/test/config.py

444 ↗

(On Diff #37293)

OK. This would hold while ABI version is 1.

[EricWF]

LGTM. We'll fix the libc++abi issue later.

[eugenis]

Landed as r250254, thanks for the review!

include/__config

 #ifdef __GNUC__
 #define _GNUC_VER (__GNUC__ * 100 + __GNUC_MINOR__)
 #else
 #define _GNUC_VER 0
 #endif
 
 #define _LIBCPP_VERSION 3800
 
-#define _LIBCPP_ABI_VERSION 1
+#ifndef _LIBCPP_ABI_MAJOR_VERSION
+#define _LIBCPP_ABI_MAJOR_VERSION 1
+#endif
+
+#ifndef _LIBCPP_ABI_MINOR_VERSION
+#define _LIBCPP_ABI_MINOR_VERSION 0
+#endif
+
+#if _LIBCPP_ABI_MAJOR_VERSION >= 2
+#define _LIBCPP_ABI_ALTERNATE_STRING_LAYOUT
+#endif
 
 #define _LIBCPP_CONCAT1(_LIBCPP_X,_LIBCPP_Y) _LIBCPP_X##_LIBCPP_Y
 #define _LIBCPP_CONCAT(_LIBCPP_X,_LIBCPP_Y) _LIBCPP_CONCAT1(_LIBCPP_X,_LIBCPP_Y)
 
-#define _LIBCPP_NAMESPACE _LIBCPP_CONCAT(__,_LIBCPP_ABI_VERSION)
+#define _LIBCPP_NAMESPACE _LIBCPP_CONCAT(__,_LIBCPP_ABI_MAJOR_VERSION)
 
 
 #ifndef __has_attribute
 #define __has_attribute(__x) 0
 #endif
 #ifndef __has_builtin
 #define __has_builtin(__x) 0
 #endif

 #endif
 
 #ifndef _LIBCPP_ALWAYS_INLINE
 #define _LIBCPP_ALWAYS_INLINE  __attribute__ ((__visibility__("hidden"), __always_inline__))
 #endif
 
 #if defined(__clang__)
 
-#if defined(__APPLE__) && !defined(__i386__) && !defined(__x86_64__) &&        \
-    !defined(__arm__)
-#define _LIBCPP_ALTERNATE_STRING_LAYOUT
+#if (defined(__APPLE__) && !defined(__i386__) && !defined(__x86_64__) &&       \
+     !defined(__arm__)) ||                                                     \
+    defined(_LIBCPP_ALTERNATE_STRING_LAYOUT)
+#define _LIBCPP_ABI_ALTERNATE_STRING_LAYOUT
 #endif
 
 #if __has_feature(cxx_alignas)
 #  define _ALIGNAS_TYPE(x) alignas(x)
 #  define _ALIGNAS(x) alignas(x)
 #else
 #  define _ALIGNAS_TYPE(x) __attribute__((__aligned__(__alignof(x))))
 #  define _ALIGNAS(x) __attribute__((__aligned__(x)))

include/string

 #pragma warning( push )
 #pragma warning( disable: 4231 )
 #endif // _LIBCPP_MSVC
 _LIBCPP_EXTERN_TEMPLATE(class _LIBCPP_TYPE_VIS __basic_string_common<true>)
 #ifdef _LIBCPP_MSVC
 #pragma warning( pop )
 #endif // _LIBCPP_MSVC
 
-#ifdef _LIBCPP_ALTERNATE_STRING_LAYOUT
+#ifdef _LIBCPP_ABI_ALTERNATE_STRING_LAYOUT
 
 template <class _CharT, size_t = sizeof(_CharT)>
 struct __padding
 {
     unsigned char __xx[sizeof(_CharT)-1];
 };
 
 template <class _CharT>
 struct __padding<_CharT, 1>
 {
 };
 
-#endif  // _LIBCPP_ALTERNATE_STRING_LAYOUT
+#endif  // _LIBCPP_ABI_ALTERNATE_STRING_LAYOUT
 
 template<class _CharT, class _Traits, class _Allocator>
 class _LIBCPP_TYPE_VIS_ONLY basic_string
     : private __basic_string_common<true>
 {
 public:
     typedef basic_string                                 __self;
     typedef _Traits                                      traits_type;
#else  // defined(_LIBCPP_RAW_ITERATORS)
     typedef __wrap_iter<pointer>                         iterator;
     typedef __wrap_iter<const_pointer>                   const_iterator;
 #endif  // defined(_LIBCPP_RAW_ITERATORS)
     typedef _VSTD::reverse_iterator<iterator>             reverse_iterator;
     typedef _VSTD::reverse_iterator<const_iterator>       const_reverse_iterator;
 
 private:
 
-#ifdef _LIBCPP_ALTERNATE_STRING_LAYOUT
+#ifdef _LIBCPP_ABI_ALTERNATE_STRING_LAYOUT
 
     struct __long
     {
         pointer   __data_;
         size_type __size_;
         size_type __cap_;
     };
 
struct __short
         union
         {
             unsigned char __size_;
             value_type __lx;
         };
         value_type __data_[__min_cap];
     };
 
-#endif  // _LIBCPP_ALTERNATE_STRING_LAYOUT
+#endif  // _LIBCPP_ABI_ALTERNATE_STRING_LAYOUT
 
     union __ulx{__long __lx; __short __lxx;};
 
     enum {__n_words = sizeof(__ulx) / sizeof(size_type)};
 
     struct __raw
     {
         size_type __words[__n_words];

 private:
     _LIBCPP_INLINE_VISIBILITY
     allocator_type& __alloc() _NOEXCEPT
         {return __r_.second();}
     _LIBCPP_INLINE_VISIBILITY
     const allocator_type& __alloc() const _NOEXCEPT
         {return __r_.second();}
 
-#ifdef _LIBCPP_ALTERNATE_STRING_LAYOUT
+#ifdef _LIBCPP_ABI_ALTERNATE_STRING_LAYOUT
 
     _LIBCPP_INLINE_VISIBILITY
     void __set_short_size(size_type __s) _NOEXCEPT
 #   if _LIBCPP_BIG_ENDIAN
         {__r_.first().__s.__size_ = (unsigned char)(__s << 1);}
 #   else
         {__r_.first().__s.__size_ = (unsigned char)(__s);}
 #   endif
 
     _LIBCPP_INLINE_VISIBILITY
     size_type __get_short_size() const _NOEXCEPT
 #   if _LIBCPP_BIG_ENDIAN
         {return __r_.first().__s.__size_ >> 1;}
 #   else
         {return __r_.first().__s.__size_;}
 #   endif
 
-#else  // _LIBCPP_ALTERNATE_STRING_LAYOUT
+#else  // _LIBCPP_ABI_ALTERNATE_STRING_LAYOUT
 
     _LIBCPP_INLINE_VISIBILITY
     void __set_short_size(size_type __s) _NOEXCEPT
 #   if _LIBCPP_BIG_ENDIAN
         {__r_.first().__s.__size_ = (unsigned char)(__s);}
 #   else
         {__r_.first().__s.__size_ = (unsigned char)(__s << 1);}
 #   endif
 
     _LIBCPP_INLINE_VISIBILITY
     size_type __get_short_size() const _NOEXCEPT
 #   if _LIBCPP_BIG_ENDIAN
         {return __r_.first().__s.__size_;}
 #   else
         {return __r_.first().__s.__size_ >> 1;}
 #   endif
 
-#endif  // _LIBCPP_ALTERNATE_STRING_LAYOUT
+#endif  // _LIBCPP_ABI_ALTERNATE_STRING_LAYOUT
 
     _LIBCPP_INLINE_VISIBILITY
     void __set_long_size(size_type __s) _NOEXCEPT
         {__r_.first().__l.__size_ = __s;}
     _LIBCPP_INLINE_VISIBILITY
     size_type __get_long_size() const _NOEXCEPT
         {return __r_.first().__l.__size_;}
     _LIBCPP_INLINE_VISIBILITY