Details
- Reviewers
ldionne Mordante - Group Reviewers
Restricted Project - Commits
- rG287847dace44: [libc++] Update ABI docs. NFCI.
Diff Detail
- Repository
- rG LLVM Github Monorepo
Unit Tests
Event Timeline
I like this, the text looks much better now. Did you verify whether the documentation changes build locally? If not can you rebase the patch after Buildkite becomes stable again?
libcxx/docs/DesignDocs/ABIVersioning.rst | ||
---|---|---|
15 | I dislike unstable here. |
We need to decide whether the ABI version is something that can be selected by users when they use libc++, or whether it's something that's baked in when the library is built and shipped (usually by a vendor). In my opinion, the former doesn't make sense, because the ABI version has an influence on what is provided in the .so - so the behavior of the headers has to match exactly how the .so was built. I'd like to get a common understanding and agreement that this is how things should work.
If we're all on the same page with that, I would not mention the _LIBCPP_ABI_VERSION macro anywhere since it can be considered an implementation detail of the library. The only thing that really matters is the LIBCXX_ABI_VERSION CMake setting, which is set when libc++ is built/packaged. I would also drop the mention of _LIBCPP_ABI_UNSTABLE.
libcxx/docs/DesignDocs/ABIVersioning.rst | ||
---|---|---|
22 | @ldionne wrote:
I'm willing to stipulate that end-users should mess only with LIBCXX_ABI_VERSION (and never touch any of the individual _LIBCPP_ABI_XXX macros directly); however, I still think it's worth mentioning these macros somehow in this design doc, because otherwise we have literally no documentation of the C++ macro design, and I assume collecting that kind of documentation is the entire point of this "DesignDocs" directory. How about I talk about the CMake flags first, and then start a new paragraph with "Internally, each ABI-changing feature is placed under... ...Libc++ does not intend users to interact with these C++ macros directly." |
Updated in the general direction of @ldionne's and @mclow.lists's review comments. What do we think of this now?
libcxx/docs/DesignDocs/ABIVersioning.rst | ||
---|---|---|
15 | What is the intension when version "2" becomes stable? Vendors will have compilers in service that support version 1 and the vendor customers will have many applications and shared libraries that use version 1. What will happen when the vendor wants to step up to version 2? Will the vendor be able to ship both version 1 & version 2? Will applications be able to use both versions either via object file dependencies (eg. static libs) or via shared libraries? Or is there a complete breakage? I hope that is not the case as that would force 3rd party providers to provide two versions of any shared library they sell. |
@ldionne and @mclow.lists , how does this look now? I still claim it's a step in the forward direction, and would like to get it landed.
libcxx/docs/DesignDocs/ABIVersioning.rst | ||
---|---|---|
15 | I don't know, but I bet "there is a complete breakage" is closest to the truth. |
I think this looks really good now, thanks a lot and sorry for the delay!
libcxx/docs/DesignDocs/ABIVersioning.rst | ||
---|---|---|
15 |
I don't foresee a point where we will say version 1 doesn't exist anymore. That would imply forcing an ABI break on all vendors simultaneously, which is both a crazy idea and not up to an open source community to decide, IMO. What I could see happening is that we'll provide version 1, but also provide a stable version 2 that vendors can individually opt into, if it's possible for them to do so. For example, most vendors have an occasion to perform a complete ABI break when they introduce a new architecture, since no software exists on that architecture at the start. That's an open door for them to pick the newer yet stable ABI v2 (once it is stable). If we do come to a point (a long time from now) where we are OK with dropping support for v1 completely (e.g. because nobody ships it anymore), then we could remove it. |
I dislike unstable here.