[libcxx-commits] [PATCH] D68480: Implementation of C++20's P1135R6 for libcxx

Mon Jan 3 14:11:31 PST 2022

ldionne added a subscriber: dim.
ldionne added a comment.

In D68480#3216178 <https://reviews.llvm.org/D68480#3216178>, @theraven wrote:

> In D68480#3179526 <https://reviews.llvm.org/D68480#3179526>, @ldionne wrote:
>
>> In D68480#3179355 <https://reviews.llvm.org/D68480#3179355>, @theraven wrote:
>>
>>> This appears to have baked in some ABI details that don't permit efficient implementation on some of our supported platforms.
>>
>> This is unfortunate. Have you folks shipped this yet? Perhaps it's not too late to fix things now if you've only released it very recently.
>
> I am not sure who you mean when you say 'you' here.  I am writing this as:
>
> - A member of the LLVM project and libc++ contributor
> - A libc++ consumer.

Right, "you" might not be the appropriate target here, I was thinking about whoever vends libc++ on FreeBSD (I wrongly assumed you were working for that vendor). My point is that nobody cares about ABI breaks for a platform unless the library is vended on that platform, in which case that vendor is the one who cares about ABI stability. In this case, that vendor is FreeBSD, and I consider that whoever vends libc++ on FreeBSD unfortunately missed the boat on this review. In FreeBSD's defence, it's true that we were not as well organized as we are now when we shipped this patch -- we now have a more detailed support policy and ties between various vendors and developers are somewhat clearer.

I will argue that breaking this ABI on Windows is not a big deal since nobody's vending libc++ on Windows. Or at least if they do, they are completely invisible to us ("us" being the people who develop libc++).

> Since Howard's initial commit, libc++ has had strong ABI backwards compatibility guarantees for anything outside of the `experimental` namespace.  Any time we define an interface between the library and the headers, that is an ABI that we need to support in the long term.

While I agree to some extent, I think it is important to highlight that ABI stability is not a property of the library itself, but a property of a specific vended instance of the library. For example, Chrome uses libc++, but they don't care about ABI stability because they link it statically. So we provide them with knobs to change libc++ behavior in various ways that are not ABI stable. On the other hand, some vendors like Apple do care about ABI stability for various reasons, and those vendors ensure that the ABI isn't broken on their platform, with the flavor of libc++ that they ship. And similarly, libc++ provides various knobs to help these vendors NOT break the ABI on their platforms.

In other words, over time, libc++ has grown from "ABI stability at all costs" to "providing tools to tweak it the way you want and be ABI stable if you want, but ABI unstable if you don't". That was the response we came up with because different consumers have different use cases for the library.

Where I'm going with this is that when Olivier created this review, I saw it had ABI implications and did what I needed to do for the vendor I represent. In the presence of a vast number of consumers of libc++, it's impossible for Olivier, or I, or any individual, to know about all the consumers that might not like the specific ABI that was selected. It might be obvious to you cause you've looked into it, but in the general case, the responsibility is on the vendors of the library to be at least a bit aware of what's going on and make sure they know what they are shipping on their platform. Cause that's the root cause of the issue -- this sub-optimal ABI was shipped on FreeBSD (hence locking them into it) without FreeBSD even knowing about it. That's really unfortunate, but like I said, libc++ provides ways to help vendors control what they ship on their platform, and they need only minimal involvement for this to happen. For instance, I would really welcome FreeBSD (and other vendors) taking advantage of the availability annotations I maintain. If FreeBSD had used those, it might have given some indication that FreeBSD might not want to ship those new symbols immediately.

> I came to this particular patch because one of my colleagues pointed me at `atomic_wait` as a possible replacement for platform-specific wrappers around futex-like abstractions in our code.  I then tried to add FreeBSD and Windows support (the two platforms I care about for our code that do not have platform-specific code paths in libc++) and discovered that the ABI that we have committed to supporting cannot be implemented *at all* on 32-bit architectures with FreeBSD and cannot expose all of the functionality on Windows.

Would you be willing to open a patch showing what changes are necessary for each of those? I think we can totally break the ABI on Windows since nobody's shipping it. And on 32-bit FreeBSD, if things don't work at all right now, then we can also "break the ABI" on that configuration since nobody can be depending on something that doesn't work. Perhaps things are not as bad as they seem?

Also, on a different note: I'm pretty serious about FreeBSD needing to implement pre-commit CI if it wants to be supported officially. It's a small investment, but it needs to happen for libc++ to keep FreeBSD as part of its list of supported platforms. @dim can you help with this?

Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D68480/new/

https://reviews.llvm.org/D68480