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/list
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
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/CM
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 o
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
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
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
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
inc
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
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/cf
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__)
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/A
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
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
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/Buildi
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 upda
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 up
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
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
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
>
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
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
>
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++
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
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 heade
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://re
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 a
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 propos
28 matches
Mail list logo
