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

Wed Dec 8 07:30:01 PST 2021

ldionne added a comment.

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.

> In the future, please can you post an RFC from things that need to integrate with platform-specific code? We're now stuck with this interface until we are willing to do an ABI-breaking change.

With all due respect, I consider that the responsibility is yours. In May 2019, Olivier started a thread where he was gathering feedback on what was to become the synchronization library: https://lists.llvm.org/pipermail/libcxx-dev/2019-May/000396.html. The thread got very little traction (see the June 2019 archive for the only few replies). Then, this patch was created and it was under review for roughly 6 months. Anyone paying attention to libc++ development could have seen this go by.

The reality is that if you ship libc++ on your platform, you should have someone caring about libc++ development to ensure it plays well with your platform. There are so many people using libc++ in various ways (vendors and others) that it's difficult for the core contributors to make sure that everybody "gets the memo" whenever we do something. Don't get me wrong, I very often go out of my way to ensure that vendors get notified when we make potentially contentious changes (I often ping a bunch of people that I know vend the library). However, all of that is based on basically me knowing (or thinking I know) who vends the library for what platform. None of it is really formalized, and misses can and do happen.

Furthermore, I would like to point out that libc++ goes out of its way to ensure that these sorts of things don't happen. We have availability annotations, for example, that allow vendors to control when users are able to start relying on features. In this case, I used those annotations for solving exactly the problem you encountered: I ensured that users were not going to start relying on the synchronization library on Apple platforms until we could confirm it was ABI stable, and I only flipped the switch one year after it had been implemented (see D96790 <https://reviews.llvm.org/D96790>). I've set things up so that any vendor could take advantage of those (they were previously Apple-specific), and I've always been surprised that no other vendors cared enough about QoI on their platforms to start using that infrastructure. If you're not using the tools provided to control what you ship on your platform, you can't expect someone who might not even know your existence to do it for you.

I would also like to point out that more than one year after pre-commit CI has been set up, we still don't have a FreeBSD and OpenBSD bots in BuildKite. Technically, that makes those two platforms unsupported. They are important platforms and we definitely want to support them, but we need a bit more energy from the vendor side here. Not very much, just a bit.

So, concretely, here are some action items for OpenBSD and FreeBSD vendors to become a well-behaved citizen and make sure such issues don't happen again in the future:

1. Please set up BuildKite bots for OpenBSD and FreeBSD. I'll help you do it, it's easy. Then, we'll officially support those platforms and document it in our documentation.
2. If you vend libc++, please add yourself to this Herald group: https://reviews.llvm.org/project/view/109/. I created that group recently so I could formalize the notion of "begin a libc++ vendor" and make it easier to ping everybody.
3. Please have someone subscribe to libcxx-commits or use some Herald rules to be notified about reviews. This should ensure that, even if we don't ping the vendor group, there's some chance that you folks will see it. You don't have to engage in reviews or anything, but even just reviewing the subject lines once per week should be enough to ensure things like this don't happen again. That seems like a reasonable investment for an organization that ships libc++ on their platform.
4. Setup availability annotations for your platform. See `libcxx/include/__availability` for details.

Now, concretely, please let me know if you think it's feasible to fix this now that you've shipped it. If so, we can work together on that. Otherwise, I'm sorry this happened, and the steps above should ensure it doesn't again.

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D68480