Re: [PATCH] D11740: ABI versioning macros for libc++
eugenis closed this revision. eugenis added a comment. Landed as r250254, thanks for the review! Repository: rL LLVM http://reviews.llvm.org/D11740 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
Re: [PATCH] D11740: ABI versioning macros for libc++
EricWF added a comment. LGTM. We'll fix the libc++abi issue later. Repository: rL LLVM http://reviews.llvm.org/D11740 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
Re: [PATCH] D11740: ABI versioning macros for libc++
eugenis updated this revision to Diff 37299.
Repository:
rL LLVM
http://reviews.llvm.org/D11740
Files:
CMakeLists.txt
cmake/Modules/HandleLibcxxFlags.cmake
docs/Abi.rst
docs/BuildingLibcxx.rst
include/__config
include/__config_site.in
include/string
lib/CMakeLists.txt
test/CMakeLists.txt
test/libcxx/test/config.py
test/lit.site.cfg.in
Index: test/lit.site.cfg.in
===
--- test/lit.site.cfg.in
+++ test/lit.site.cfg.in
@@ -7,6 +7,8 @@
config.enable_rtti = "@LIBCXX_ENABLE_RTTI@"
config.enable_shared= "@LIBCXX_ENABLE_SHARED@"
config.enable_32bit = "@LIBCXX_BUILD_32_BITS@"
+config.abi_version = "@LIBCXX_ABI_VERSION@"
+config.abi_unstable = "@LIBCXX_ABI_UNSTABLE@"
config.enable_global_filesystem_namespace = "@LIBCXX_ENABLE_GLOBAL_FILESYSTEM_NAMESPACE@"
config.enable_stdin = "@LIBCXX_ENABLE_STDIN@"
config.enable_stdout= "@LIBCXX_ENABLE_STDOUT@"
Index: test/libcxx/test/config.py
===
--- test/libcxx/test/config.py
+++ test/libcxx/test/config.py
@@ -387,6 +387,7 @@
# Configure feature flags.
self.configure_compile_flags_exceptions()
self.configure_compile_flags_rtti()
+self.configure_compile_flags_abi_version()
self.configure_compile_flags_no_global_filesystem_namespace()
self.configure_compile_flags_no_stdin()
self.configure_compile_flags_no_stdout()
@@ -440,6 +441,15 @@
self.config.available_features.add('libcpp-no-rtti')
self.cxx.compile_flags += ['-fno-rtti', '-D_LIBCPP_NO_RTTI']
+def configure_compile_flags_abi_version(self):
+abi_version = self.get_lit_conf('abi_version', '').strip()
+abi_unstable = self.get_lit_bool('abi_unstable')
+if abi_version:
+ self.cxx.compile_flags += ['-D_LIBCPP_ABI_VERSION=' + abi_version]
+if abi_unstable:
+ self.config.available_features.add('libcpp-abi-unstable')
+ self.cxx.compile_flags += ['-D_LIBCPP_ABI_UNSTABLE']
+
def configure_compile_flags_no_global_filesystem_namespace(self):
enable_global_filesystem_namespace = self.get_lit_bool(
'enable_global_filesystem_namespace', True)
Index: test/CMakeLists.txt
===
--- test/CMakeLists.txt
+++ test/CMakeLists.txt
@@ -13,6 +13,7 @@
pythonize_bool(LIBCXX_ENABLE_RTTI)
pythonize_bool(LIBCXX_ENABLE_SHARED)
pythonize_bool(LIBCXX_BUILD_32_BITS)
+pythonize_bool(LIBCXX_ABI_UNSTABLE)
pythonize_bool(LIBCXX_ENABLE_GLOBAL_FILESYSTEM_NAMESPACE)
pythonize_bool(LIBCXX_ENABLE_STDIN)
pythonize_bool(LIBCXX_ENABLE_STDOUT)
Index: lib/CMakeLists.txt
===
--- lib/CMakeLists.txt
+++ lib/CMakeLists.txt
@@ -129,8 +129,8 @@
COMPILE_FLAGS "${LIBCXX_COMPILE_FLAGS}"
LINK_FLAGS"${LIBCXX_LINK_FLAGS}"
OUTPUT_NAME "c++"
-VERSION "1.0"
-SOVERSION "1"
+VERSION "${LIBCXX_ABI_VERSION}.0"
+SOVERSION "${LIBCXX_ABI_VERSION}"
)
if (LIBCXX_INSTALL_LIBRARY)
Index: include/string
===
--- include/string
+++ include/string
@@ -1185,7 +1185,7 @@
#pragma warning( pop )
#endif // _LIBCPP_MSVC
-#ifdef _LIBCPP_ALTERNATE_STRING_LAYOUT
+#ifdef _LIBCPP_ABI_ALTERNATE_STRING_LAYOUT
template
struct __padding
@@ -1198,7 +1198,7 @@
{
};
-#endif // _LIBCPP_ALTERNATE_STRING_LAYOUT
+#endif // _LIBCPP_ABI_ALTERNATE_STRING_LAYOUT
template
class _LIBCPP_TYPE_VIS_ONLY basic_string
@@ -1234,7 +1234,7 @@
private:
-#ifdef _LIBCPP_ALTERNATE_STRING_LAYOUT
+#ifdef _LIBCPP_ABI_ALTERNATE_STRING_LAYOUT
struct __long
{
@@ -1294,7 +1294,7 @@
value_type __data_[__min_cap];
};
-#endif // _LIBCPP_ALTERNATE_STRING_LAYOUT
+#endif // _LIBCPP_ABI_ALTERNATE_STRING_LAYOUT
union __ulx{__long __lx; __short __lxx;};
@@ -1698,7 +1698,7 @@
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
@@ -1716,7 +1716,7 @@
{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
@@ -1734,7 +1734,7 @@
{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
Index: include/__config_site.in
===
Re: [PATCH] D11740: ABI versioning macros for libc++
eugenis added inline comments.
Comment at: test/libcxx/test/config.py:444
@@ -442,1 +443,3 @@
+def configure_compile_flags_abi_version(self):
+abi_version = self.get_lit_conf('abi_version', '').strip()
EricWF wrote:
> 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.
OK. This would hold while ABI version is 1.
Repository:
rL LLVM
http://reviews.llvm.org/D11740
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
Re: [PATCH] D11740: ABI versioning macros for libc++
jroelofs added inline comments. Comment at: include/__config_site.in:13 @@ -12,1 +12,3 @@ +#cmakedefine _LIBCPP_ABI_VERSION @_LIBCPP_ABI_VERSION@ +#cmakedefine _LIBCPP_ABI_UNSTABLE jroelofs wrote: > 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? Never mind. After re-reading the cmake docs, I see what it's supposed to do. Repository: rL LLVM http://reviews.llvm.org/D11740 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
Re: [PATCH] D11740: ABI versioning macros for libc++
jroelofs added a subscriber: jroelofs. Comment at: include/__config_site.in:13 @@ -12,1 +12,3 @@ +#cmakedefine _LIBCPP_ABI_VERSION @_LIBCPP_ABI_VERSION@ +#cmakedefine _LIBCPP_ABI_UNSTABLE 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? Repository: rL LLVM http://reviews.llvm.org/D11740 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
Re: [PATCH] D11740: ABI versioning macros for libc++
EricWF added a comment.
Please address the inline comment. I think with that change we can hold off
modifying libc++abi.
Comment at: test/libcxx/test/config.py:444
@@ -442,1 +443,3 @@
+def configure_compile_flags_abi_version(self):
+abi_version = self.get_lit_conf('abi_version', '').strip()
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.
Repository:
rL LLVM
http://reviews.llvm.org/D11740
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
Re: [PATCH] D11740: ABI versioning macros for libc++
eugenis updated this revision to Diff 37293.
eugenis marked an inline comment as done.
Repository:
rL LLVM
http://reviews.llvm.org/D11740
Files:
CMakeLists.txt
cmake/Modules/HandleLibcxxFlags.cmake
docs/Abi.rst
docs/BuildingLibcxx.rst
include/__config
include/__config_site.in
include/string
lib/CMakeLists.txt
test/CMakeLists.txt
test/libcxx/test/config.py
test/lit.site.cfg.in
Index: test/lit.site.cfg.in
===
--- test/lit.site.cfg.in
+++ test/lit.site.cfg.in
@@ -7,6 +7,8 @@
config.enable_rtti = "@LIBCXX_ENABLE_RTTI@"
config.enable_shared= "@LIBCXX_ENABLE_SHARED@"
config.enable_32bit = "@LIBCXX_BUILD_32_BITS@"
+config.abi_version = "@LIBCXX_ABI_VERSION@"
+config.abi_unstable = "@LIBCXX_ABI_UNSTABLE@"
config.enable_global_filesystem_namespace = "@LIBCXX_ENABLE_GLOBAL_FILESYSTEM_NAMESPACE@"
config.enable_stdin = "@LIBCXX_ENABLE_STDIN@"
config.enable_stdout= "@LIBCXX_ENABLE_STDOUT@"
Index: test/libcxx/test/config.py
===
--- test/libcxx/test/config.py
+++ test/libcxx/test/config.py
@@ -387,6 +387,7 @@
# Configure feature flags.
self.configure_compile_flags_exceptions()
self.configure_compile_flags_rtti()
+self.configure_compile_flags_abi_version()
self.configure_compile_flags_no_global_filesystem_namespace()
self.configure_compile_flags_no_stdin()
self.configure_compile_flags_no_stdout()
@@ -440,6 +441,14 @@
self.config.available_features.add('libcpp-no-rtti')
self.cxx.compile_flags += ['-fno-rtti', '-D_LIBCPP_NO_RTTI']
+def configure_compile_flags_abi_version(self):
+abi_version = self.get_lit_conf('abi_version', '').strip()
+abi_unstable = self.get_lit_bool('abi_unstable')
+self.cxx.compile_flags += ['-D_LIBCPP_ABI_VERSION=' + abi_version]
+if abi_unstable:
+ self.config.available_features.add('libcpp-abi-unstable')
+ self.cxx.compile_flags += ['-D_LIBCPP_ABI_UNSTABLE']
+
def configure_compile_flags_no_global_filesystem_namespace(self):
enable_global_filesystem_namespace = self.get_lit_bool(
'enable_global_filesystem_namespace', True)
Index: test/CMakeLists.txt
===
--- test/CMakeLists.txt
+++ test/CMakeLists.txt
@@ -13,6 +13,7 @@
pythonize_bool(LIBCXX_ENABLE_RTTI)
pythonize_bool(LIBCXX_ENABLE_SHARED)
pythonize_bool(LIBCXX_BUILD_32_BITS)
+pythonize_bool(LIBCXX_ABI_UNSTABLE)
pythonize_bool(LIBCXX_ENABLE_GLOBAL_FILESYSTEM_NAMESPACE)
pythonize_bool(LIBCXX_ENABLE_STDIN)
pythonize_bool(LIBCXX_ENABLE_STDOUT)
Index: lib/CMakeLists.txt
===
--- lib/CMakeLists.txt
+++ lib/CMakeLists.txt
@@ -129,8 +129,8 @@
COMPILE_FLAGS "${LIBCXX_COMPILE_FLAGS}"
LINK_FLAGS"${LIBCXX_LINK_FLAGS}"
OUTPUT_NAME "c++"
-VERSION "1.0"
-SOVERSION "1"
+VERSION "${LIBCXX_ABI_VERSION}.0"
+SOVERSION "${LIBCXX_ABI_VERSION}"
)
if (LIBCXX_INSTALL_LIBRARY)
Index: include/string
===
--- include/string
+++ include/string
@@ -1185,7 +1185,7 @@
#pragma warning( pop )
#endif // _LIBCPP_MSVC
-#ifdef _LIBCPP_ALTERNATE_STRING_LAYOUT
+#ifdef _LIBCPP_ABI_ALTERNATE_STRING_LAYOUT
template
struct __padding
@@ -1198,7 +1198,7 @@
{
};
-#endif // _LIBCPP_ALTERNATE_STRING_LAYOUT
+#endif // _LIBCPP_ABI_ALTERNATE_STRING_LAYOUT
template
class _LIBCPP_TYPE_VIS_ONLY basic_string
@@ -1234,7 +1234,7 @@
private:
-#ifdef _LIBCPP_ALTERNATE_STRING_LAYOUT
+#ifdef _LIBCPP_ABI_ALTERNATE_STRING_LAYOUT
struct __long
{
@@ -1294,7 +1294,7 @@
value_type __data_[__min_cap];
};
-#endif // _LIBCPP_ALTERNATE_STRING_LAYOUT
+#endif // _LIBCPP_ABI_ALTERNATE_STRING_LAYOUT
union __ulx{__long __lx; __short __lxx;};
@@ -1698,7 +1698,7 @@
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
@@ -1716,7 +1716,7 @@
{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
@@ -1734,7 +1734,7 @@
{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
Index: include/__config_site.in
Re: [PATCH] D11740: ABI versioning macros for libc++
eugenis added inline comments. Comment at: include/__config:252 @@ -246,1 +251,3 @@ +defined(_LIBCPP_ALTERNATE_STRING_LAYOUT) +#define _LIBCPP_ABI_ALTERNATE_STRING_LAYOUT #endif EricWF wrote: > 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. Exactly. Repository: rL LLVM http://reviews.llvm.org/D11740 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
Re: [PATCH] D11740: ABI versioning macros for libc++
EricWF added a comment. http://reviews.llvm.org/D13407 has landed and this is good to go. Repository: rL LLVM http://reviews.llvm.org/D11740 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
Re: [PATCH] D11740: ABI versioning macros for libc++
EricWF accepted this revision. EricWF added a comment. This revision is now accepted and ready to land. LGTM but this is still blocked by http://reviews.llvm.org/D13407. Ill let you know once that has landed. Comment at: include/__config:251 @@ +250,3 @@ + !defined(__arm__)) || \ +defined(_LIBCPP_ALTERNATE_STRING_LAYOUT) +#define _LIBCPP_ABI_ALTERNATE_STRING_LAYOUT 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. Comment at: include/__config:252 @@ -246,1 +251,3 @@ +defined(_LIBCPP_ALTERNATE_STRING_LAYOUT) +#define _LIBCPP_ABI_ALTERNATE_STRING_LAYOUT #endif 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. Repository: rL LLVM http://reviews.llvm.org/D11740 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
Re: [PATCH] D11740: ABI versioning macros for libc++
eugenis set the repository for this revision to rL LLVM.
eugenis updated this revision to Diff 37015.
eugenis added a comment.
Rebased on http://reviews.llvm.org/D13407
Repository:
rL LLVM
http://reviews.llvm.org/D11740
Files:
CMakeLists.txt
cmake/Modules/HandleLibcxxFlags.cmake
docs/Abi.rst
docs/BuildingLibcxx.rst
include/__config
include/__config_site.in
include/string
lib/CMakeLists.txt
test/CMakeLists.txt
test/libcxx/test/config.py
test/lit.site.cfg.in
Index: test/lit.site.cfg.in
===
--- test/lit.site.cfg.in
+++ test/lit.site.cfg.in
@@ -7,6 +7,8 @@
config.enable_rtti = "@LIBCXX_ENABLE_RTTI@"
config.enable_shared= "@LIBCXX_ENABLE_SHARED@"
config.enable_32bit = "@LIBCXX_BUILD_32_BITS@"
+config.abi_version = "@LIBCXX_ABI_VERSION@"
+config.abi_unstable = "@LIBCXX_ABI_UNSTABLE@"
config.enable_global_filesystem_namespace = "@LIBCXX_ENABLE_GLOBAL_FILESYSTEM_NAMESPACE@"
config.enable_stdin = "@LIBCXX_ENABLE_STDIN@"
config.enable_stdout= "@LIBCXX_ENABLE_STDOUT@"
Index: test/libcxx/test/config.py
===
--- test/libcxx/test/config.py
+++ test/libcxx/test/config.py
@@ -370,6 +370,7 @@
# Configure feature flags.
self.configure_compile_flags_exceptions()
self.configure_compile_flags_rtti()
+self.configure_compile_flags_abi_version()
self.configure_compile_flags_no_global_filesystem_namespace()
self.configure_compile_flags_no_stdin()
self.configure_compile_flags_no_stdout()
@@ -423,6 +424,14 @@
self.config.available_features.add('libcpp-no-rtti')
self.cxx.compile_flags += ['-fno-rtti', '-D_LIBCPP_NO_RTTI']
+def configure_compile_flags_abi_version(self):
+abi_version = self.get_lit_conf('abi_version', '').strip()
+abi_unstable = self.get_lit_bool('abi_unstable')
+self.cxx.compile_flags += ['-D_LIBCPP_ABI_VERSION=' + abi_version]
+if abi_unstable:
+ self.config.available_features.add('libcpp-abi-unstable')
+ self.cxx.compile_flags += ['-D_LIBCPP_ABI_UNSTABLE']
+
def configure_compile_flags_no_global_filesystem_namespace(self):
enable_global_filesystem_namespace = self.get_lit_bool(
'enable_global_filesystem_namespace', True)
Index: test/CMakeLists.txt
===
--- test/CMakeLists.txt
+++ test/CMakeLists.txt
@@ -13,6 +13,7 @@
pythonize_bool(LIBCXX_ENABLE_RTTI)
pythonize_bool(LIBCXX_ENABLE_SHARED)
pythonize_bool(LIBCXX_BUILD_32_BITS)
+pythonize_bool(LIBCXX_ABI_UNSTABLE)
pythonize_bool(LIBCXX_ENABLE_GLOBAL_FILESYSTEM_NAMESPACE)
pythonize_bool(LIBCXX_ENABLE_STDIN)
pythonize_bool(LIBCXX_ENABLE_STDOUT)
Index: lib/CMakeLists.txt
===
--- lib/CMakeLists.txt
+++ lib/CMakeLists.txt
@@ -129,8 +129,8 @@
COMPILE_FLAGS "${LIBCXX_COMPILE_FLAGS}"
LINK_FLAGS"${LIBCXX_LINK_FLAGS}"
OUTPUT_NAME "c++"
-VERSION "1.0"
-SOVERSION "1"
+VERSION "${LIBCXX_ABI_VERSION}.0"
+SOVERSION "${LIBCXX_ABI_VERSION}"
)
if (LIBCXX_INSTALL_LIBRARY)
Index: include/string
===
--- include/string
+++ include/string
@@ -1185,7 +1185,7 @@
#pragma warning( pop )
#endif // _LIBCPP_MSVC
-#ifdef _LIBCPP_ALTERNATE_STRING_LAYOUT
+#ifdef _LIBCPP_ABI_ALTERNATE_STRING_LAYOUT
template
struct __padding
@@ -1198,7 +1198,7 @@
{
};
-#endif // _LIBCPP_ALTERNATE_STRING_LAYOUT
+#endif // _LIBCPP_ABI_ALTERNATE_STRING_LAYOUT
template
class _LIBCPP_TYPE_VIS_ONLY basic_string
@@ -1234,7 +1234,7 @@
private:
-#ifdef _LIBCPP_ALTERNATE_STRING_LAYOUT
+#ifdef _LIBCPP_ABI_ALTERNATE_STRING_LAYOUT
struct __long
{
@@ -1294,7 +1294,7 @@
value_type __data_[__min_cap];
};
-#endif // _LIBCPP_ALTERNATE_STRING_LAYOUT
+#endif // _LIBCPP_ABI_ALTERNATE_STRING_LAYOUT
union __ulx{__long __lx; __short __lxx;};
@@ -1698,7 +1698,7 @@
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
@@ -1716,7 +1716,7 @@
{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
@@ -1734,7 +1734,7 @@
{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) _NOEXC
Re: [PATCH] D11740: ABI versioning macros for libc++
EricWF added a comment. FYI I'm focusing on getting http://reviews.llvm.org/D11963 in so that it stops blocking this patch. http://reviews.llvm.org/D11740 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
Re: [PATCH] D11740: ABI versioning macros for libc++
EricWF added a comment. FYI I'm focusing on getting http://reviews.llvm.org/D11963 in so that it stops blocking this patch. http://reviews.llvm.org/D11740 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
Re: [PATCH] D11740: ABI versioning macros for libc++
eugenis updated this revision to Diff 33381.
eugenis added a comment.
Remove minor version, added abi_unstable,.
Keeping __config_version until the other change lands.
Tests use headers from the build directory.
http://reviews.llvm.org/D11740
Files:
CMakeLists.txt
docs/Abi.rst
docs/BuildingLibcxx.rst
include/CMakeLists.txt
include/__config
include/__config_version
include/__config_version.cmake
include/string
lib/CMakeLists.txt
test/CMakeLists.txt
test/lit.site.cfg.in
Index: test/lit.site.cfg.in
===
--- test/lit.site.cfg.in
+++ test/lit.site.cfg.in
@@ -2,6 +2,7 @@
config.cxx_under_test = "@LIBCXX_COMPILER@"
config.libcxx_src_root = "@LIBCXX_SOURCE_DIR@"
config.libcxx_obj_root = "@LIBCXX_BINARY_DIR@"
+config.libcxx_headers = "@LIBCXX_HEADERS@"
config.cxx_library_root = "@LIBCXX_LIBRARY_DIR@"
config.enable_exceptions= "@LIBCXX_ENABLE_EXCEPTIONS@"
config.enable_rtti = "@LIBCXX_ENABLE_RTTI@"
Index: test/CMakeLists.txt
===
--- test/CMakeLists.txt
+++ test/CMakeLists.txt
@@ -34,6 +34,7 @@
set(AUTO_GEN_COMMENT "## Autogenerated by libcxx configuration.\n# Do not edit!")
+set(LIBCXX_HEADERS "${CMAKE_BINARY_DIR}/include/c++/v${LIBCXX_ABI_VERSION}")
configure_file(
${CMAKE_CURRENT_SOURCE_DIR}/lit.site.cfg.in
${CMAKE_CURRENT_BINARY_DIR}/lit.site.cfg
Index: lib/CMakeLists.txt
===
--- lib/CMakeLists.txt
+++ lib/CMakeLists.txt
@@ -129,8 +129,8 @@
COMPILE_FLAGS "${LIBCXX_COMPILE_FLAGS}"
LINK_FLAGS"${LIBCXX_LINK_FLAGS}"
OUTPUT_NAME "c++"
-VERSION "1.0"
-SOVERSION "1"
+VERSION "${LIBCXX_ABI_VERSION}.0"
+SOVERSION "${LIBCXX_ABI_VERSION}"
)
if (LIBCXX_INSTALL_LIBRARY)
Index: include/string
===
--- include/string
+++ include/string
@@ -1185,7 +1185,7 @@
#pragma warning( pop )
#endif // _LIBCPP_MSVC
-#ifdef _LIBCPP_ALTERNATE_STRING_LAYOUT
+#ifdef _LIBCPP_ABI_ALTERNATE_STRING_LAYOUT
template
struct __padding
@@ -1198,7 +1198,7 @@
{
};
-#endif // _LIBCPP_ALTERNATE_STRING_LAYOUT
+#endif // _LIBCPP_ABI_ALTERNATE_STRING_LAYOUT
template
class _LIBCPP_TYPE_VIS_ONLY basic_string
@@ -1234,7 +1234,7 @@
private:
-#ifdef _LIBCPP_ALTERNATE_STRING_LAYOUT
+#ifdef _LIBCPP_ABI_ALTERNATE_STRING_LAYOUT
struct __long
{
@@ -1294,7 +1294,7 @@
value_type __data_[__min_cap];
};
-#endif // _LIBCPP_ALTERNATE_STRING_LAYOUT
+#endif // _LIBCPP_ABI_ALTERNATE_STRING_LAYOUT
union __ulx{__long __lx; __short __lxx;};
@@ -1696,7 +1696,7 @@
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
@@ -1714,7 +1714,7 @@
{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
@@ -1732,7 +1732,7 @@
{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
Index: include/__config_version.cmake
===
--- include/__config_version.cmake
+++ include/__config_version.cmake
@@ -0,0 +1,17 @@
+// -*- C++ -*-
+//===--- __config_version -===//
+//
+// The LLVM Compiler Infrastructure
+//
+// This file is dual licensed under the MIT and the University of Illinois Open
+// Source Licenses. See LICENSE.TXT for details.
+//
+//===--===//
+
+#ifndef _LIBCPP_CONFIG_VERSION
+#define _LIBCPP_CONFIG_VERSION
+
+#define _LIBCPP_ABI_VERSION ${LIBCXX_ABI_VERSION}
+#cmakedefine _LIBCPP_ABI_UNSTABLE
+
+#endif
Index: include/__config_version
===
--- include/__config_version
+++ include/__config_version
@@ -0,0 +1,17 @@
+// -*- C++ -*-
+//===--- __config_version -===//
+//
+// The LLVM Compiler Infrastructure
+//
+// This file is dual licensed under the MIT and the University of Illinois Open
+// Source Licenses. See LICENSE.TXT for details.
+//
+//===--===//
+
+#ifndef _LIBCPP_CONFIG_VERSION
+#define _LIBCPP_CONFIG_VERSION
+
+#define _LIBCPP_ABI_VERSION 1
+#undef _LIBCPP_A
Re: [PATCH] D11740: ABI versioning macros for libc++
EricWF added a comment. In http://reviews.llvm.org/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 http://reviews.llvm.org/D11963. http://reviews.llvm.org/D11740 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
Re: [PATCH] D11740: ABI versioning macros for libc++
eugenis added a comment. 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). http://reviews.llvm.org/D11740 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
Re: [PATCH] D11740: ABI versioning macros for libc++
EricWF added a comment. In http://reviews.llvm.org/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. http://reviews.llvm.org/D11740 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
Re: [PATCH] D11740: ABI versioning macros for libc++
eugenis added a comment. In http://reviews.llvm.org/D11740#234610, @EricWF wrote: > In http://reviews.llvm.org/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 > > http://reviews.llvm.org/D11963, do something smart in __config to default > > to the right version numbers. > > > I'm not sure what you mean by "smart" because IMO > http://reviews.llvm.org/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)? http://reviews.llvm.org/D11740 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
Re: [PATCH] D11740: ABI versioning macros for libc++
EricWF added a comment. In http://reviews.llvm.org/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 > http://reviews.llvm.org/D11963, do something smart in __config to default to > the right version numbers. I'm not sure what you mean by "smart" because IMO http://reviews.llvm.org/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. http://reviews.llvm.org/D11740 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
Re: [PATCH] D11740: ABI versioning macros for libc++
eugenis added a comment. 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 http://reviews.llvm.org/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? http://reviews.llvm.org/D11740 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
Re: [PATCH] D11740: ABI versioning macros for libc++
EricWF added a comment. In http://reviews.llvm.org/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 http://reviews.llvm.org/D11963 (whatever that ends up being after review) and not add a `__config_version`. http://reviews.llvm.org/D11740 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
Re: [PATCH] D11740: ABI versioning macros for libc++
EricWF added a comment. 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. http://reviews.llvm.org/D11740 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
Re: [PATCH] D11740: ABI versioning macros for libc++
eugenis added a comment. > 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. http://reviews.llvm.org/D11740 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
Re: [PATCH] D11740: ABI versioning macros for libc++
eugenis removed rL LLVM as the repository for this revision.
eugenis updated this revision to Diff 33246.
eugenis added a comment.
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.
http://reviews.llvm.org/D11740
Files:
CMakeLists.txt
docs/Abi.rst
docs/BuildingLibcxx.rst
include/CMakeLists.txt
include/__config
include/__config_version.cmake
include/string
lib/CMakeLists.txt
Index: lib/CMakeLists.txt
===
--- lib/CMakeLists.txt
+++ lib/CMakeLists.txt
@@ -129,8 +129,8 @@
COMPILE_FLAGS "${LIBCXX_COMPILE_FLAGS}"
LINK_FLAGS"${LIBCXX_LINK_FLAGS}"
OUTPUT_NAME "c++"
-VERSION "1.0"
-SOVERSION "1"
+VERSION "${LIBCXX_ABI_MAJOR_VERSION}.${LIBCXX_ABI_MINOR_VERSION}"
+SOVERSION "${LIBCXX_ABI_MAJOR_VERSION}"
)
install(TARGETS cxx
Index: include/string
===
--- include/string
+++ include/string
@@ -1185,7 +1185,7 @@
#pragma warning( pop )
#endif // _LIBCPP_MSVC
-#ifdef _LIBCPP_ALTERNATE_STRING_LAYOUT
+#ifdef _LIBCPP_ABI_ALTERNATE_STRING_LAYOUT
template
struct __padding
@@ -1198,7 +1198,7 @@
{
};
-#endif // _LIBCPP_ALTERNATE_STRING_LAYOUT
+#endif // _LIBCPP_ABI_ALTERNATE_STRING_LAYOUT
template
class _LIBCPP_TYPE_VIS_ONLY basic_string
@@ -1234,7 +1234,7 @@
private:
-#ifdef _LIBCPP_ALTERNATE_STRING_LAYOUT
+#ifdef _LIBCPP_ABI_ALTERNATE_STRING_LAYOUT
struct __long
{
@@ -1294,7 +1294,7 @@
value_type __data_[__min_cap];
};
-#endif // _LIBCPP_ALTERNATE_STRING_LAYOUT
+#endif // _LIBCPP_ABI_ALTERNATE_STRING_LAYOUT
union __ulx{__long __lx; __short __lxx;};
@@ -1696,7 +1696,7 @@
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
@@ -1714,7 +1714,7 @@
{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
@@ -1732,7 +1732,7 @@
{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
Index: include/__config_version.cmake
===
--- include/__config_version.cmake
+++ include/__config_version.cmake
@@ -0,0 +1,19 @@
+// -*- C++ -*-
+//===--- __config_version -===//
+//
+// The LLVM Compiler Infrastructure
+//
+// This file is dual licensed under the MIT and the University of Illinois Open
+// Source Licenses. See LICENSE.TXT for details.
+//
+//===--===//
+
+#ifndef _LIBCPP_CONFIG_VERSION
+#define _LIBCPP_CONFIG_VERSION
+
+#define _LIBCPP_ABI_MAJOR_VERSION ${LIBCXX_ABI_MAJOR_VERSION}
+#define _LIBCPP_ABI_MINOR_VERSION ${LIBCXX_ABI_MINOR_VERSION}
+
+#define _LIBCPP_ABI_VERSION (_LIBCPP_ABI_MAJOR_VERSION * 100 + _LIBCPP_ABI_MINOR_VERSION)
+
+#endif
Index: include/__config
===
--- include/__config
+++ include/__config
@@ -11,6 +11,8 @@
#ifndef _LIBCPP_CONFIG
#define _LIBCPP_CONFIG
+#include <__config_version>
+
#if !defined(_MSC_VER) || defined(__clang__)
#pragma GCC system_header
#endif
@@ -23,12 +25,14 @@
#define _LIBCPP_VERSION 3800
-#define _LIBCPP_ABI_VERSION 1
+#if _LIBCPP_ABI_VERSION >= 200
+#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
@@ -230,9 +234,10 @@
#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
Re: [PATCH] D11740: ABI versioning macros for libc++
EricWF added a comment. 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? Repository: rL LLVM http://reviews.llvm.org/D11740 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
Re: [PATCH] D11740: ABI versioning macros for libc++
EricWF added a comment. 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 http://reviews.llvm.org/D12129 lands. Repository: rL LLVM http://reviews.llvm.org/D11740 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
Re: [PATCH] D11740: ABI versioning macros for libc++
eugenis added a comment. 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. Repository: rL LLVM http://reviews.llvm.org/D11740 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
