[libcxx-commits] [PATCH] D148432: [libc++] Simplify the tuple constructor overload set

Mon May 15 12:21:31 PDT 2023

ldionne added a comment.

In D148432#4339935 <https://reviews.llvm.org/D148432#4339935>, @Mordante wrote:

> [...]
> I really have no idea what GCC guarantees or wants to promise. Since we don't test ToT GCC we will only discover breakage
> after the fact.

That's a fair point, I think we can and should reach out to the GCC folks to clarify the guarantees with respect to compiler extensions.

> As mentioned before, discussions on Discord often don't end in any conclusion/consensus before moving ahead.
> I really would like to see libc++ improve in this area. I've talked @ldionne privately about this in the past.

I am very happy to work on improving this area of the project, but I'd like to understand better what you want. Would you prefer if this were discussed on Discourse instead of Discord? I'm fine with that although I don't believe it would meaningfully change the amount of consensus we'd get versus doing the same on Discord -- for example if you look at most RFCs on Discourse there is no clear consensus at the end of the discussion, and someone usually steps up and determines what consensus means in the context and decides to move forward or not. We've also discussed setting up a recurring VC meeting with the libc++ developers -- do you feel like it would have made a difference in this case? We could do it ~monthly and accumulate an agenda of items to discuss "in person".

> I think it would be very good to discuss some of these policies, the same like considering the removal of
> C++03 support. I agree these kind of changes make maintenance for us, the libc++ developers, a lot easier.
> However, what is the size of our community that are negatively affected by these changes?

I agree, and in fact these policies are always discussed somewhere, usually on Discord. Perhaps that's the main sticking point and we should have something more formal.

> Since it's only discussed on Discord interested/affected parties may have missed the entire discussion.
> I think it's important to communicate these kind of changes upfront. We don't have a
> `_LIBCPP_DISABLE_FEATURE_CONDITIONAL_EXPLICIT_CONSTRUCTOR` like we do when we do in our normal deprecation.
> So if this breaks somebody after LLVM 17 is released they have no easy way to opt-out. IMO we should add
> that and keep it for (at least) two releases, just like we do with deprecation.

I think the context of this change is extremely relevant in this case. This change should have no impact whatsoever on users. If it did, then I would immediately argue for us *not* to make that change. In fact, I asked for the cautious approach taken by this patch where we only use the compiler extension in one place and don't start using any new extensions until LLVM 17 is released, so as to make it as easy as possible to revert this decision if we find that this is user-affecting. Regarding deprecation/communication: since this is not user-impacting, I don't think it makes sense to have a deprecation or a macro to opt into the old way to write the same constructors. Basically, this patch is either "it works and we do it, or we realize that it has user impact and we revert the patch".

Because this is not user-affecting, I think the set of interested parties is basically just the libc++ developers themselves. So while we may want to use a more formal process to discuss these changes, I don't think anyone was left out of the discussion we had on Discord. Let's figure out what concrete steps we want to take to improve the decision making process and then take this change through it. How does that sound?

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D148432