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

[Nikolas Klauser via Phabricator via libcxx-commits]

philnik added a comment.

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

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

Clang has actually started documenting these kinds of extensions: https://clang.llvm.org/docs/LanguageExtensions.html#language-extensions-back-ported-to-previous-standards. We already only discover breakages after the fact, so I don't see how that's relevant.

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

You were the only person to object to this. Everybody else who commented (Costa, Louis, Eric) was in favor of it.

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

We obviously don't know whether people are affected by this, but I would be very surprised if anybody was. We already assume lots of extensions specific to clang and GCC, and no vendor has expressed any concerns so far (also not with this patch).

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

IMO that doesn't make sense. We only support Clang and GCC, which support these extensions, and vendors test everything pre-release. If anybody finds a problem with it, we can still revert. Even if that lets things fall through the cracks, we can still revert in 17.0.whatever, and the far majority of users won't have a problem.

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

If I understand @aaron.ballman's comments correctly, they consider all documented extensions to be very hard to impossible to remove, and AFAICT quite easy to keep support for (at least for the from-later-standards ones). If you want, we can reach out to GCC folks to make sure they don't see things drastically different (which I don't expect given the amount of, even non-conforming, GNU extensions).


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D148432