[PATCH] D128745: [c++] implements DR692, DR1395 and tentatively DR1432, about partial ordering of variadic template partial specialization or function template

Tue Oct 4 05:55:03 PDT 2022

aaron.ballman added a reviewer: hubert.reinterpretcast.
aaron.ballman added a subscriber: hubert.reinterpretcast.
aaron.ballman added a comment.

In D128745#3832598 <https://reviews.llvm.org/D128745#3832598>, @ychen wrote:

> In D128745#3832432 <https://reviews.llvm.org/D128745#3832432>, @rjmccall wrote:
>
>> Pinging this thread because it has more reviewers who probably have opinions about this.
>>
>> https://reviews.llvm.org/D134507 is a patch that adds `-fclang-abi-compat` support around the breaking change here, which basically means that targets using old `-fclang-abi-compat` settings never get this change.  I've argued on that patch that that isn't the right way of resolving this issue.  Basically, the DR here is changing the language behavior in a way that you can imagine having ABI impact, but it's not primarily an ABI change, it's a language semantics change.  The ABI impact is purely that we potentially select a different overload in some cases, which can have downstream ODR impact.  I think the appropriate way to handle language semantics changes like this which potentially have compatibility impact is to condition them on language version.  Changing the target language standard is already broadly understood to have source/semantic compatibility impact, which is why we allow different target language standards to be specified in the first place.  This also makes it straightforward to document this potential break as a consequence of moving to `-std=c++23`, and it removes a potential rather bizarre portability issue where platforms that embrace stable ABIs are permanently stuck with a language dialect.
>
> This is very reasonable to me except that I hope, for this particular patch, to key on `-std=c++20` instead of `-std=c++23` since this is needed for P2113 <https://reviews.llvm.org/P2113>, a C++20 change.

There were multiple DRs implemented in this patch.

DR692 was a fix against C++11
DR1395 was a fix against C++17
DR1432 is still open but was opened against C++11

Which DRs do we wish to implement in what language modes?

@rjmccall is correct about us not being required to apply DRs when they're disruptive, but at the same time, WG21 DRs are intended to be handled as if the original standard they were reported against had always been using the fixed form. So *not* applying a DR in order to avoid problems for existing code can cause problems for existing and future code in terms of portability between compilers (and ABI impacts that stem from the semantic changes). So I think we wish to apply the DRs as broadly as we can. The question is: do we think users with existing code should not have to change or do we think it's reasonable to give them a flag to opt into the old behavior? My personal feeling is -- the default compiler mode should be as conforming as we can make it be within reason, and since this has some impact but not a massive break (no major OS SDKs or third party libraries seem to be impacted as best I can tell), my feeling is that we should strongly consider adding a feature flag (other than ABI compat, as that does seem like a bit of an odd choice to key on) to opt into the older behavior, esp since the break is a loud one and not a silent one.

Adding @hubert.reinterpretcast as C++ conformance code owner in case he's got opinions as well.

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D128745