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

[Mark de Wever via Phabricator via libcxx-commits]

Mordante added a comment.

I haven't looked at the patch itself in detail, but the stats look nice.

However I'm very concerned with the approach of this patch and the decision making in libc++.
As mentioned on Discord I think we open ourselves to Hyrum's law by depending on undocumented compiler features.
AFAIK no compiler documents these extensions and the guarantees they offer for these features. 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.

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 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?

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.

If we want to go in this direction we should document which extensions are allowed and which are not.
Preferably after making sure what the policy of these undocumented compiler features for different vendors
mean. Note that libc++ has removed backported features when they resulted in a maintenance burden.
I don't expect that be as troublesome in compilers, but I'm not sure.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D148432