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.
Details
Diff Detail
- Repository
- rL LLVM
Event Timeline
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.
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:
- How long is a major and minor ABI version supported?
- When is the major and minor ABI version bumped?
- 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.
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?
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.
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.
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:
- Previous: The previous major ABI version.
- Default: The current major ABI version.
- 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.
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.
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?
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.
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)?
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).
Remove minor version, added abi_unstable,.
Keeping __config_version until the other change lands.
Tests use headers from the build directory.
LGTM but this is still blocked by D13407. Ill let you know once that has landed.
include/__config | ||
---|---|---|
251 | Is this the last remaining reference to _LIBCPP_ALTERNATIVE_STRING_LAYOUT? | |
252 | 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. |
include/__config | ||
---|---|---|
252 | Exactly. |
Please address the inline comment. I think with that change we can hold off modifying libc++abi.
test/libcxx/test/config.py | ||
---|---|---|
444 | 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. |
include/__config_site.in | ||
---|---|---|
13 | 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? |
include/__config_site.in | ||
---|---|---|
13 | Never mind. After re-reading the cmake docs, I see what it's supposed to do. |
test/libcxx/test/config.py | ||
---|---|---|
444 | OK. This would hold while ABI version is 1. |
This seems unused? The renaming for the major version would be unnecessary if this is not being added.