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

[ychen]

Summary

DR692 handles two cases: pack expansion (for class/var template) and function parameter pack. The former needs DR1432 as a fix, and the latter needs DR1395 as a fix. However, DR1432 has not yet made a wording change. so I made a tentative fix for DR1432 with the same spirit as DR1395.

[ychen]

fix typo

[aaron.ballman]

Thank you for working on this! Can you please add more details to the patch summary about the changes?

clang/lib/Sema/SemaTemplateDeduction.cpp

5191

Does GCC still implement it this way?

One concern I have is that this will be an ABI breaking change, and I'm not certain how disruptive it will be. If GCC already made that change, it may be reasonable for us to also break it without having to add ABI tags or something special. But if these changes diverge us from GCC, that may require some extra effort to mitigate.

5198–5199

We tend not to use top-level const on locals in the project as a matter of style.

clang/test/CXX/drs/dr6xx.cpp

1082–1084

This DR is also about partial class specializations -- are changes not needed in Sema::getMoreSpecializedPartialSpecialization() as well?

It'd be helpful to add the examples from the DR here to make sure they're handled properly.

[ychen]

In D128745#3632850, @aaron.ballman wrote:

Thank you for working on this! Can you please add more details to the patch summary about the changes?

Thanks for taking a look.

This was intended to correctly implement https://eel.is/c++draft/temp.deduct.partial#11. The test case is from https://eel.is/c++draft/temp.func.order#example-4.

If, after considering the above, function template F is at least as specialized as function template G and vice-versa, and if G has a trailing function parameter pack for which F does not have a corresponding parameter, and if F does not have a trailing function parameter pack, then F is more specialized than G.

It looks like this is not too far from implementing DR692. I'll go ahead and include the necessary change for fixing the template parameter packs case.

[ychen]

clang/lib/Sema/SemaTemplateDeduction.cpp

5191

Unfortunately, GCC is still using the old/non-conforming behavior. https://clang.godbolt.org/z/5K4916W71. What is the usual way to mitigate this?

[aaron.ballman]

clang/lib/Sema/SemaTemplateDeduction.cpp

5191

You would use the ClangABI enumeration to alter behavior between ABI versions: https://github.com/llvm/llvm-project/blob/main/clang/include/clang/Basic/LangOptions.h#L174 like done in: https://github.com/llvm/llvm-project/blob/main/clang/lib/AST/DeclCXX.cpp#L929

[ychen]

• handle more cases to implement DR692, DR1395
• tentatively implement DR1432 which DR692 needs, otherwise there would be regressions

[ychen]

• fix a bug
• update comments
• check-all, check-runtimes pass

[ychen]

clang/lib/Sema/SemaTemplateDeduction.cpp

2490

FYI: isSameTemplateArg is for class/variable partial ordering deduction. The corresponding check for function template deduction is skipped by intention. See https://reviews.llvm.org/rG86a1b135f0c67a022ef628f092c6691892752876 and https://github.com/llvm/llvm-project/blob/e6f1f062457c928c18a88c612f39d9e168f65a85/clang/lib/Sema/SemaTemplateDeduction.cpp#L5064-L5066.

5191

Looking at how ClangABI is currently used, it looks like it is for low-level object layout ABI compatibility. For library/language ABI compatibility, maybe we should not use ClangABI? Looking at https://reviews.llvm.org/rG86a1b135f0c67a022ef628f092c6691892752876, I guess the practice is just committed and see? If it breaks important or many existing libraries, just revert or add compiler options?

[ychen]

clang/lib/Sema/SemaTemplateDeduction.cpp

5191

I guess the practice is just committed and see? If it breaks important or many existing libraries, just revert or add compiler options?

I'm fairly optimistic considering check-runtimes passes.

[aaron.ballman]

I admit that I'm feeling a bit out of my element with all the template machinery involved, so I've reviewed as best I'm able (so if any of my questions seem odd to you, push back on them if you want).

[aaron.ballman]

clang/lib/Sema/SemaTemplateDeduction.cpp

1110

Why are you checking ArgIdx + 1 == NumArgs here? It looks like you're only handling the case where the pack is the last argument, but packs can appear anywhere, right?

2490

Same question here about looking at just the last element.

5191

Looking at how ClangABI is currently used, it looks like it is for low-level object layout ABI compatibility. For library/language ABI compatibility, maybe we should not use ClangABI?

I don't know that there's any specific guidance that we only use it for object layout ABI compatibility; we have used it for behavior beyond layout in the past: https://github.com/llvm/llvm-project/blob/main/clang/lib/Sema/SemaDeclCXX.cpp#L9786

I guess the practice is just committed and see? If it breaks important or many existing libraries, just revert or add compiler options?

Somewhat, yes. We aim for full ABI compatibility with GCC and consider ABI differences to be bugs (generally speaking). If we break some important existing library, that bug is critical to get fixed ASAP, but any ABI different can potentially bite users.

I would say: if matching the old ABI requires convoluting the implementation too much, we can skip it; otherwise, we should probably match GCC's ABI just to avoid concerns.

CC @rsmith in case he has different opinions as code owner.

5408

You should end the anonymous namespace here per our coding style guidelines.

5432–5433

Can't this be a local constexpr variable instead of an NTTP, or are there reasons you want to allow callers to be able to override that?

5435–5436

Curiosity question -- can you make P1 and P2 be pointers to const without many other viral changes to existing code?

5453

Again, should you be looking at all the packs here and not just trailing ones?

clang/www/cxx_dr_status.html

4197

You should use unreleased as the class for these because Clang 15 hasn't been release yet (they'll switch over to full once we release).

8181

Same here

[erichkeane]

clang/lib/Sema/SemaTemplateDeduction.cpp

1110

This is definitely a concern to me as well, we probably have to check ALL packs and figure out what was deduced.

5207

This is still the same issue I think? This should probably be a 'parameters contains pack', not just checking the last.

[ychen]

Thanks for the review. @aaron.ballman @erichkeane. About the non-trailing pack, I had the impression that DR692 is only about the trailing pack in practice but don't remember the exact wording, so I dig the standardese further.

For partial specialization:

https://eel.is/c++draft/temp.spec.partial#general-9.4

An argument shall not contain an unexpanded pack.
If an argument is a pack expansion ([temp.variadic]), it shall be the last argument in the template argument list.

For the primary template, the rule is the same for the partial ordering purpose (See the second code example of DR1495).

DR692 does not need to consider a non-trailing function parameter pack since it is a non-deduced context.

https://eel.is/c++draft/temp.deduct#type-5.7

The non-deduced contexts are:
  ...
  - A function parameter pack that does not occur at the end of the parameter-declaration-list.

[cor3ntin]

clang/lib/Sema/SemaTemplateDeduction.cpp

5453

If it's not trailing, it's not deduced. and afaik, if it's not deduced it can't be involved spacial specialization ordering

[ychen]

• add Clang ABI check
• replace one NTTP with a local constexpr variable
• style changes

[ychen]

clang/lib/Sema/SemaTemplateDeduction.cpp

5191

That makes sense to me. Added the ABI compatibility check.

5432–5433

Ah, I didn't think of that. The caller has no need to override this.

5435–5436

isAtLeastAsSpecializedAs uses template instantiations which modify P1/P2.

[aaron.ballman]

Thank you for the explanation on non-trailing packs, I think I'm convinced. Aside from some test coverage, I think this LGTM. Please hold off on landing until @erichkeane has had a chance to look again though.

clang/lib/Sema/SemaTemplateDeduction.cpp

1110

Can you add CodeGen test coverage for both ABI modes (new RUN line with -fclang-abi-compat=14)

5435–5436

Good to know, then no changes needed here, thanks!

5451

Can you add CodeGen test coverage for both ABI modes (new RUN line with -fclang-abi-compat=14)

[erichkeane]

Nothing sticks out to me, LGTM too.

[ychen]

• add CodeGen tests

[ychen]

clang/lib/Sema/SemaTemplateDeduction.cpp

5451

Some test cases could reach the CodeGen phase *with* -fclang-abi-compat=14(i.e without this patch), some test cases could reach the CodeGen phase *without* -fclang-abi-compat=14(i.e with this patch). Please let me know if the added CodeGen tests are what you expected.

[aaron.ballman]

LGTM!

clang/lib/Sema/SemaTemplateDeduction.cpp

5451

Yes, that test is what I was looking for, thank you!

[ychen]

• rebase

[abrachet]

This is breaking us.

template <typename T, typename... Ts> struct S; // #1

template <typename T> struct S<T> {}; // #2

The following compiled before but is now broken, (we use -fclang-abi-compat=13.0). We get a warning from -Winvalid-partial-specialization

This change _does_ make https://eel.is/c++draft/temp.func.order#example-5 work, ie for function overload resolution but regresses the type deduction example above.

There seem to be examples in the standard that suggest #2 is more specialized. @mcgrathr found https://www.open-std.org/jtc1/sc22/wg21/docs/cwg_active.html#1432 and https://www.open-std.org/jtc1/sc22/wg21/docs/cwg_defects.html#1395 among others. WDYT?

[ychen]

In D128745#3727697, @abrachet wrote:

This is breaking us.

template <typename T, typename... Ts> struct S; // #1

template <typename T> struct S<T> {}; // #2

The following compiled before but is now broken, (we use -fclang-abi-compat=13.0). We get a warning from -Winvalid-partial-specialization

This change _does_ make https://eel.is/c++draft/temp.func.order#example-5 work, ie for function overload resolution but regresses the type deduction example above.

There seem to be examples in the standard that suggest #2 is more specialized. @mcgrathr found https://www.open-std.org/jtc1/sc22/wg21/docs/cwg_active.html#1432 and https://www.open-std.org/jtc1/sc22/wg21/docs/cwg_defects.html#1395 among others. WDYT?

I tried your test case but was unable to reproduce the issue. Is this the complete test case? I'm asking because the provided test case does not involve deduction.

[ychen]

In D128745#3727715, @ychen wrote:

In D128745#3727697, @abrachet wrote:

This is breaking us.

template <typename T, typename... Ts> struct S; // #1

template <typename T> struct S<T> {}; // #2

The following compiled before but is now broken, (we use -fclang-abi-compat=13.0). We get a warning from -Winvalid-partial-specialization

This change _does_ make https://eel.is/c++draft/temp.func.order#example-5 work, ie for function overload resolution but regresses the type deduction example above.

There seem to be examples in the standard that suggest #2 is more specialized. @mcgrathr found https://www.open-std.org/jtc1/sc22/wg21/docs/cwg_active.html#1432 and https://www.open-std.org/jtc1/sc22/wg21/docs/cwg_defects.html#1395 among others. WDYT?

I tried your test case but was unable to reproduce the issue. Is this the complete test case? I'm asking because the provided test case does not involve deduction.

Or do you mean the duction for partial ordering, then I did a simple S<int> a which seems fine?

[abrachet]

In D128745#3727715, @ychen wrote:

In D128745#3727697, @abrachet wrote:

This is breaking us.

template <typename T, typename... Ts> struct S; // #1

template <typename T> struct S<T> {}; // #2

The following compiled before but is now broken, (we use -fclang-abi-compat=13.0). We get a warning from -Winvalid-partial-specialization

This change _does_ make https://eel.is/c++draft/temp.func.order#example-5 work, ie for function overload resolution but regresses the type deduction example above.

There seem to be examples in the standard that suggest #2 is more specialized. @mcgrathr found https://www.open-std.org/jtc1/sc22/wg21/docs/cwg_active.html#1432 and https://www.open-std.org/jtc1/sc22/wg21/docs/cwg_defects.html#1395 among others. WDYT?

I tried your test case but was unable to reproduce the issue. Is this the complete test case? I'm asking because the provided test case does not involve deduction.

Just that input is enough to get the error. I did clang++ test.cpp -fsyntax-only -fclang-abi-compat=14.0

[ychen]

In D128745#3727770, @abrachet wrote:

In D128745#3727715, @ychen wrote:

In D128745#3727697, @abrachet wrote:

This is breaking us.

template <typename T, typename... Ts> struct S; // #1

template <typename T> struct S<T> {}; // #2

The following compiled before but is now broken, (we use -fclang-abi-compat=13.0). We get a warning from -Winvalid-partial-specialization

This change _does_ make https://eel.is/c++draft/temp.func.order#example-5 work, ie for function overload resolution but regresses the type deduction example above.

There seem to be examples in the standard that suggest #2 is more specialized. @mcgrathr found https://www.open-std.org/jtc1/sc22/wg21/docs/cwg_active.html#1432 and https://www.open-std.org/jtc1/sc22/wg21/docs/cwg_defects.html#1395 among others. WDYT?

I tried your test case but was unable to reproduce the issue. Is this the complete test case? I'm asking because the provided test case does not involve deduction.

Just that input is enough to get the error. I did clang++ test.cpp -fsyntax-only -fclang-abi-compat=14.0

Thanks, it works now. I was missing -fclang-abi-compat=14.0.

[michele.scandale]

I can see failures related to this change in a downstream version of clang where the default ClangABICompat value is not Latest (after the followup https://reviews.llvm.org/rGda6187f566b7881cb8350621aea9bd582de569b9).
My understanding is that with -fclang-abi-compat=14 we should get the behavior prior to this change. If my understanding is correct, then I'd think that such failure shouldn't occur.

These are the tests that I see failing with new diagnostic messages class template partial specialization is not more specialized than the primary template being raised:

Clang :: CXX/temp/temp.decls/temp.class.spec/p8-0x.cpp
Clang :: CXX/temp/temp.decls/temp.variadic/example-bind.cpp
Clang :: CXX/temp/temp.decls/temp.variadic/example-tuple.cpp
Clang :: CXX/temp/temp.decls/temp.variadic/injected-class-name.cpp
Clang :: CXX/temp/temp.decls/temp.variadic/metafunctions.cpp
Clang :: CXX/temp/temp.fct.spec/temp.arg.explicit/p3-0x.cpp
Clang :: CXX/temp/temp.fct.spec/temp.arg.explicit/p9-0x.cpp
Clang :: CXX/temp/temp.fct.spec/temp.deduct/temp.deduct.call/p1-0x.cpp
Clang :: CXX/temp/temp.fct.spec/temp.deduct/temp.deduct.type/p21.cpp
Clang :: CXX/temp/temp.fct.spec/temp.deduct/temp.deduct.type/p9-0x.cpp
Clang :: CXX/temp/temp.param/p11-0x.cpp
Clang :: SemaCXX/coreturn-exp-namespace.cpp
Clang :: SemaCXX/coreturn.cpp
Clang :: SemaCXX/coroutine-unhandled_exception-warning-exp-namespace.cpp
Clang :: SemaCXX/coroutine-unhandled_exception-warning.cpp
Clang :: SemaCXX/coroutines-exp-namespace.cpp
Clang :: SemaCXX/coroutines.cpp
Clang :: SemaCXX/crash-lambda-12645424.cpp
Clang :: SemaCXX/discrim-union.cpp
Clang :: SemaTemplate/attributes.cpp
Clang :: SemaTemplate/class-template-spec.cpp
Clang :: SemaTemplate/deduction.cpp
Clang :: SemaTemplate/pack-deduction.cpp
Clang :: SemaTemplate/temp_arg_nontype_cxx1z.cpp
Clang :: SemaTemplate/temp_arg_template_cxx1z.cpp

I've reproduced this locally with the following patch to inject -fclang-abi-compat=14 on the command line executed for those tests:

diff --git a/clang/test/CXX/temp/lit.local.cfg b/clang/test/CXX/temp/lit.local.cfg
new file mode 100644
index 000000000000..23d23bb6c46e
--- /dev/null
+++ b/clang/test/CXX/temp/lit.local.cfg
@@ -0,0 +1,3 @@
+config.substitutions.insert(0,
+  ('%clang_cc1',
+   '%clang_cc1 -fclang-abi-compat=14.0'))
diff --git a/clang/test/SemaCXX/lit.local.cfg b/clang/test/SemaCXX/lit.local.cfg
new file mode 100644
index 000000000000..23d23bb6c46e
--- /dev/null
+++ b/clang/test/SemaCXX/lit.local.cfg
@@ -0,0 +1,3 @@
+config.substitutions.insert(0,
+  ('%clang_cc1',
+   '%clang_cc1 -fclang-abi-compat=14.0'))
diff --git a/clang/test/SemaTemplate/lit.local.cfg b/clang/test/SemaTemplate/lit.local.cfg
new file mode 100644
index 000000000000..23d23bb6c46e
--- /dev/null
+++ b/clang/test/SemaTemplate/lit.local.cfg
@@ -0,0 +1,3 @@
+config.substitutions.insert(0,
+  ('%clang_cc1',
+   '%clang_cc1 -fclang-abi-compat=14.0'))

(Note that by adding the lit.local.cfg file abovementioned there will also be the failure of SemaCXX/class-layout.cpp, but that can be ignored as for some command lines the expected behavior is equivalent to pass -fclang-abi-compat=latest)

[royjacobson]

Stopping by - I assume this targets Clang 16 now, so the abi compat level should be adjusted to 15.

[ychen]

In D128745#3728087, @michele.scandale wrote:

I can see failures related to this change in a downstream version of clang where the default ClangABICompat value is not Latest (after the followup https://reviews.llvm.org/rGda6187f566b7881cb8350621aea9bd582de569b9).

Fixed in b296aed8ae239c20e

[ychen]

In D128745#3728196, @royjacobson wrote:

Stopping by - I assume this targets Clang 16 now, so the abi compat level should be adjusted to 15.

Sure, I'll make a new patch for that.

[joanahalili]

We have some compilation failures on our end because of function template parameter deduction when passing the function template to a function pointer.

typedef void (*f)(const int&);

template <typename T>
void F(T value) {}

template <typename T>
void F(const T& value){}

void q(f);

void w() {
    q(&F);
}

https://gcc.godbolt.org/z/faoq74q7G
Is this an intended outcome for this patch?

[ychen]

In D128745#3751471, @joanahalili wrote:

We have some compilation failures on our end because of function template parameter deduction when passing the function template to a function pointer.

typedef void (*f)(const int&);

template <typename T>
void F(T value) {}

template <typename T>
void F(const T& value){}

void q(f);

void w() {
    q(&F);
}

https://gcc.godbolt.org/z/faoq74q7G
Is this an intended outcome for this patch?

Thanks for the report. No that's not the intent. This should only affect partial ordering but not which candidate is viable (error message candidate function not viable ...). I'll take a look.

[ychen]

In D128745#3752148, @ychen wrote:

In D128745#3751471, @joanahalili wrote:

We have some compilation failures on our end because of function template parameter deduction when passing the function template to a function pointer.

typedef void (*f)(const int&);

template <typename T>
void F(T value) {}

template <typename T>
void F(const T& value){}

void q(f);

void w() {
    q(&F);
}

https://gcc.godbolt.org/z/faoq74q7G
Is this an intended outcome for this patch?

Thanks for the report. No that's not the intent. This should only affect partial ordering but not which candidate is viable (error message candidate function not viable ...). I'll take a look.

This new behavior is correct. It is due to the change on line 1758 in SemaTemplateDeduction.cpp. Basically, both the taking address of and regular function call to the overloaded function set using the same partial ordering rule (Relevant wording https://eel.is/c++draft/over.over#5). GCC is inconsistent in this regard (https://gcc.godbolt.org/z/WrKsaKrdz). MSVC agrees with this new/correct behavior.

[ychen]

In D128745#3752593, @ychen wrote:

In D128745#3752148, @ychen wrote:

In D128745#3751471, @joanahalili wrote:

We have some compilation failures on our end because of function template parameter deduction when passing the function template to a function pointer.

typedef void (*f)(const int&);

template <typename T>
void F(T value) {}

template <typename T>
void F(const T& value){}

void q(f);

void w() {
    q(&F);
}

https://gcc.godbolt.org/z/faoq74q7G
Is this an intended outcome for this patch?

Thanks for the report. No that's not the intent. This should only affect partial ordering but not which candidate is viable (error message candidate function not viable ...). I'll take a look.

This new behavior is correct. It is due to the change on line 1758 in SemaTemplateDeduction.cpp. Basically, both the taking address of and regular function call to the overloaded function set using the same partial ordering rule (Relevant wording https://eel.is/c++draft/over.over#5). GCC is inconsistent in this regard (https://gcc.godbolt.org/z/WrKsaKrdz). MSVC agrees with this new/correct behavior.

TBH, the Clang diagnosis could be better here. It kinda misleading about what is happening. MSVC's diagnosis is more clear (https://gcc.godbolt.org/z/WrKsaKrdz).

[tahonermann]

This change appears to be responsible for the following test case now being rejected. That seems right and is consistent with the intent, but use of -fclang-abi-compat=14 doesn't seem to restore the prior behavior. https://godbolt.org/z/ad1TPjTza

struct map
{
  template <typename Sequence>
  map(Sequence&& seq , void* = 0 ); // g++ prefers this one
  template <typename First, typename ...T_>
  map(First&& first, T_&&... rest);
};

int main()
{
  int i;
  map m(i);
  return 0;
}

[rjmccall]

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.

[ychen]

In 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, a C++20 change.

[aaron.ballman]

In D128745#3832598, @ychen wrote:

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

[erichkeane]

Is there any ability here to diagnose the difference? It would probably be helpful if at any point (in both directions!) we diagnose that our behavior changes. We did something similar for the reversible operators at one point (but WG21 might have fixed it since?).

IMO, if we decide that a DR is 'too breaking' to implement in all language modes it applies to, we shouldn't implement it at all, and instead should question the standards committee about their breaks. Flipping it based on language version increases the friction (in a silent and really breaking way!) to switch language modes.

[royjacobson]

Is there consensus that applying this DR resolution is distruptive enough that it's actually an issue? If clang-abi-compat is not a good way to offer backwards compatibility in this case, maybe we should just leave it be.

FWIW, I worry that applying it according to standard version will make it harder to write code that works cross version and the benefit here is not clear.

[hubert.reinterpretcast]

In D128745#3833291, @aaron.ballman wrote:

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

I happen to be on vacation this week ahead of Canadian Thanksgiving (or trying as much as I can anyway). I agree that broad application of DRs is generally what has been expected in the context of Clang and GCC (except where it is believed the DR resolution itself is defective, in which case the committee is consulted).

It seems there is a question of whether the DR resolutions are defective/too breaking to be advisable. Do we have a useful summary of what led to that opinion?

In considering a compatibility flag, I think some thought should be given to whether to time-limit it.

[ychen]

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?

There are regressions if DR1395 is not applied on top of DR692. So I'd consider all three fixes on C++11.

FYI, I don't see users reporting that this is very disruptive. So I think these DRs should be the compiler default.

I think the consensus is some flag is needed to put it back to legacy behavior just in case. Keying on -fclang-abi-compat is confusing. Keying on language versions works but is less conforming. My feeling is that it is rare that we have a situation like this, so I'm hesitant to come up with a general solution like a language compatibility flag. The past experience is that we could simply introduce a specific language feature flag like -faligned-new. How about adding a flag -fcxx-dr692 / -fno-cxx-dr692, defaulted on? The naming convention could be used for similar purposes in the future.

[hubert.reinterpretcast]

In D128745#3834313, @ychen wrote:

I think the consensus is some flag is needed to put it back to legacy behavior just in case.

I am not sure there is strong consensus to add a flag "just in case". Historically, many DRs had effects on overload resolution (via SFINAE or otherwise). Once we start adding flags "just in case", we might end up having a lot of them. These flags can exacerbate fracturing of the ecosystem.

[ychen]

In D128745#3834387, @hubert.reinterpretcast wrote:

In D128745#3834313, @ychen wrote:

I think the consensus is some flag is needed to put it back to legacy behavior just in case.

I am not sure there is strong consensus to add a flag "just in case". Historically, many DRs had effects on overload resolution (via SFINAE or otherwise). Once we start adding flags "just in case", we might end up having a lot of them. These flags can exacerbate fracturing of the ecosystem.

New flag or not, as long as we allow users to use the old behavior, it is already fracturing the ecosystem. Do you mean (1) we shouldn't give the user the choices or (2) we allow users to use the old behavior but only for a few releases and then remove the flag? I think (1) is a little bit harsh but I would not say it is disruptive. (2) is more user-friendly.

[tahonermann]

Is it known what gcc and Microsoft maintainers are going to do with these DRs? I don't see any recent changes in gcc, but the following is interesting...

For the test below, Clang has historically accepted it but now rejects it. MSVC has historically rejected it, but started accepting it for v19.34 (perhaps a regression?). Gcc continues to accept it. See https://godbolt.org/z/jax3GsYY5.

struct map {
  template <typename Sequence>
  map(Sequence&& seq , void* = 0 );
  template <typename First, typename ...T_>
  map(First&& first, T_&&... rest);
};
map m(0);

[ychen]

In D128745#3834454, @tahonermann wrote:

Is it known what gcc and Microsoft maintainers are going to do with these DRs? I don't see any recent changes in gcc, but the following is interesting...

It's hard to tell if they didn't implement these DRs or if they're broken. But for P2113 (C++20) to work, DR692 is needed.

For the test below, Clang has historically accepted it but now rejects it. MSVC has historically rejected it, but started accepting it for v19.34 (perhaps a regression?). Gcc continues to accept it. See https://godbolt.org/z/jax3GsYY5.

struct map {
  template <typename Sequence>
  map(Sequence&& seq , void* = 0 );
  template <typename First, typename ...T_>
  map(First&& first, T_&&... rest);
};
map m(0);

This should be ambiguous. Only the first parameter should be considered for partial ordering. This is basically the g(42) example in https://eel.is/c++draft/temp.func.order#example-4.

[rjmccall]

In D128745#3833393, @royjacobson wrote:

Is there consensus that applying this DR resolution is distruptive enough that it's actually an issue? If clang-abi-compat is not a good way to offer backwards compatibility in this case, maybe we should just leave it be.

FWIW, I worry that applying it according to standard version will make it harder to write code that works cross version and the benefit here is not clear.

The same argument applies to limiting by target. Platforms that choose to maintain a stable ABI should not be condemned to permanent non-conformance, creating additional obstacles to C++ portability. And honestly, I think the target portability argument is much stronger than the standard-version portabillity argument. It's not by any means uncommon to have C++ headers that need to work consistently under multiple language standards, but they also generally need to work on multiple compilers, and so they cannot necessarily rely on having this DR in place. There's also an awful lot of C++ code that *doesn't* have to work under multiple standards because it's internal to a project that uses consistent build settings; but a lot of that code still needs to be portable to different platforms. While some portability differences are unavoidable, we certainly don't want to create new differences.

More generally, language users understand that new language releases sometimes change existing language rules. They don't always like it, but it is a standard part of the story of changing the target language standard.

It does seem like we don't have a good assessment of how likely a compatibility break is here in practice — whether this pattern comes up in common libraries and so on. If it's vanishingly unlikely, maybe we can change this behavior unconditionally. If it's likely enough for this to be a major problem, then I think the C++ committee really needs to think about whether this change is wise. Maybe we can get more data on that.

I know that people sometimes have a strong reaction to feeling like the language is constrained by ABI compatibility concerns, so let me reiterate that this is *not* primarily about the ABI; it is about C++ introducing a semantic break something like 15 years after variadic templates were introduced to the language (11 years after standard release, but C++11 features were in wide use well before they became officially standardized).

[hubert.reinterpretcast]

In D128745#3834435, @ychen wrote:

New flag or not, as long as we allow users to use the old behavior, it is already fracturing the ecosystem. Do you mean (1) we shouldn't give the user the choices or (2) we allow users to use the old behavior but only for a few releases and then remove the flag? I think (1) is a little bit harsh but I would not say it is disruptive. (2) is more user-friendly.

(1) has been the default absent information to indicate that the new behaviour is not ready for deployment (because it would cause widespread disruption). To consider alternatives would require information about the nature of the deployment difficulties.

[ychen]

In D128745#3850667, @hubert.reinterpretcast wrote:

In D128745#3834435, @ychen wrote:

New flag or not, as long as we allow users to use the old behavior, it is already fracturing the ecosystem. Do you mean (1) we shouldn't give the user the choices or (2) we allow users to use the old behavior but only for a few releases and then remove the flag? I think (1) is a little bit harsh but I would not say it is disruptive. (2) is more user-friendly.

(1) has been the default absent information to indicate that the new behaviour is not ready for deployment (because it would cause widespread disruption). To consider alternatives would require information about the nature of the deployment difficulties.

Agreed.

To all:
As a next step, I'll remove the ClangABICompat checks for these DRs (make these DRs effective unconditionally). If we saw proof that these have deployment difficulties. We can (1) re-run the rules with the committee as suggested by @rjmccall; (2) consider alternatives (including reverting these DRs) based on the feedback. Please let me know if you object to this or have any other concerns.

[rjmccall]

I think that's fair. I don't know how much concrete information we'll get about this in a short period of time, but it's worth trying to collect it.

Was this change in LLVM 15, or is it still unreleased?

[aaron.ballman]

In D128745#3851225, @ychen wrote:

As a next step, I'll remove the ClangABICompat checks for these DRs (make these DRs effective unconditionally). If we saw proof that these have deployment difficulties. We can (1) re-run the rules with the committee as suggested by @rjmccall; (2) consider alternatives (including reverting these DRs) based on the feedback. Please let me know if you object to this or have any other concerns.

This sounds like a good plan to me. When you remove the ABI compat checks, please be sure to add the clang-vendors group to the review and a release note to the potentially breaking changes section. Once that lands, you should also make an announcement in Discourse.

[ychen]

In D128745#3851458, @rjmccall wrote:

Was this change in LLVM 15, or is it still unreleased?

This is still unreleased. It will be released in Clang16.

In D128745#3852409, @aaron.ballman wrote:

This sounds like a good plan to me. When you remove the ABI compat checks, please be sure to add the clang-vendors group to the review and a release note to the potentially breaking changes section. Once that lands, you should also make an announcement in Discourse.

Will do. Thanks for the heads-up.

clang/docs/ReleaseNotes.rst

 ---------------------------
 
 C2x Feature Support
 -------------------
 
 C++ Language Changes in Clang
 -----------------------------
 
+- Implemented DR692, DR1395 and DR1432. Use the ``-fclang-abi-compat=14`` option
+  to get the old partial ordering behavior regarding packs.
+
 C++20 Feature Support
 ^^^^^^^^^^^^^^^^^^^^^
 
 - Support capturing structured bindings in lambdas
   (`P1091R3 <https://wg21.link/p1091r3>`_ and `P1381R1 <https://wg21.link/P1381R1>`).
   This fixes issues `GH52720 <https://github.com/llvm/llvm-project/issues/52720>`_,
   `GH54300 <https://github.com/llvm/llvm-project/issues/54300>`_,
   `GH54301 <https://github.com/llvm/llvm-project/issues/54301>`_,

clang/lib/Sema/SemaTemplateDeduction.cpp

 #include "llvm/ADT/SmallPtrSet.h"
 #include "llvm/ADT/SmallVector.h"
 #include "llvm/Support/Casting.h"
 #include "llvm/Support/Compiler.h"
 #include "llvm/Support/ErrorHandling.h"
 #include <algorithm>
 #include <cassert>
 #include <tuple>
+#include <type_traits>
 #include <utility>
 
 namespace clang {
 
   /// Various flags that control template argument deduction.
   ///
   /// These flags can be bitwise-OR'd together.
   enum TemplateDeductionFlags {
for (; ParamIdx != NumParams; ++ParamIdx) {
     }
 
     // Build argument packs for each of the parameter packs expanded by this
     // pack expansion.
     if (auto Result = PackScope.finish())
       return Result;
   }
 
+  // DR692, DR1395
+  // C++0x [temp.deduct.type]p10:
+  // If the parameter-declaration corresponding to P_i ...
+  // During partial ordering, if Ai was originally a function parameter pack:
+  // - if P does not contain a function parameter type corresponding to Ai then
+  //   Ai is ignored;
+  bool ClangABICompat14 = S.Context.getLangOpts().getClangABICompat() <=
+                          LangOptions::ClangABI::Ver14;
+  if (!ClangABICompat14 && PartialOrdering && ArgIdx + 1 == NumArgs &&
+      isa<PackExpansionType>(Args[ArgIdx]))
+    return Sema::TDK_Success;
+
   // Make sure we don't have any extra arguments.
   if (ArgIdx < NumArgs)
     return Sema::TDK_MiscellaneousDeductionFailure;
 
   return Sema::TDK_Success;
 }
 
 /// Determine whether the parameter has qualifiers that the argument
case Type::FunctionProto: {
               /*PartialOrdering=*/false,
               /*DeducedFromArrayBound=*/false))
         return Result;
 
       // Check parameter types.
       if (auto Result = DeduceTemplateArguments(
               S, TemplateParams, FPP->param_type_begin(), FPP->getNumParams(),
               FPA->param_type_begin(), FPA->getNumParams(), Info, Deduced,
-              TDF & TDF_TopLevelParameterTypeList))
+              TDF & TDF_TopLevelParameterTypeList, PartialOrdering))
         return Result;
 
       if (TDF & TDF_AllowCompatibleFunctionType)
         return Sema::TDK_Success;
 
       // FIXME: Per core-2016/10/1019 (no corresponding core issue yet), permit
       // deducing through the noexcept-specifier if it's part of the canonical
       // type. libstdc++ relies on this.
return DeduceTemplateArguments(S, TemplateParams, ParamList.asArray(),
                                  ArgList.asArray(), Info, Deduced,
                                  /*NumberOfArgumentsMustMatch=*/false);
 }
 
 /// Determine whether two template arguments are the same.
 static bool isSameTemplateArg(ASTContext &Context,
                               TemplateArgument X,
                               const TemplateArgument &Y,
+                              bool PartialOrdering,
                               bool PackExpansionMatchesPack = false) {
   // If we're checking deduced arguments (X) against original arguments (Y),
   // we will have flattened packs to non-expansions in X.
   if (PackExpansionMatchesPack && X.isPackExpansion() && !Y.isPackExpansion())
     X = X.getPackExpansionPattern();
 
   if (X.getKind() != Y.getKind())
     return false;
switch (X.getKind()) {
     case TemplateArgument::Expression: {
       llvm::FoldingSetNodeID XID, YID;
       X.getAsExpr()->Profile(XID, Context, true);
       Y.getAsExpr()->Profile(YID, Context, true);
       return XID == YID;
     }
 
     case TemplateArgument::Pack:
-      if (X.pack_size() != Y.pack_size())
+      unsigned PackIterationSize = X.pack_size();
+      if (X.pack_size() != Y.pack_size()) {
+        if (!PartialOrdering)
           return false;
 
-      for (TemplateArgument::pack_iterator XP = X.pack_begin(),
-                                        XPEnd = X.pack_end(),
-                                           YP = Y.pack_begin();
-           XP != XPEnd; ++XP, ++YP)
-        if (!isSameTemplateArg(Context, *XP, *YP, PackExpansionMatchesPack))
+        // C++0x [temp.deduct.type]p9:
+        // During partial ordering, if Ai was originally a pack expansion:
+        // - if P does not contain a template argument corresponding to Ai then
+        //   Ai is ignored;
+        bool XHasMoreArg = X.pack_size() > Y.pack_size();
+        if (!(XHasMoreArg && X.pack_elements().back().isPackExpansion()) &&
+            !(!XHasMoreArg && Y.pack_elements().back().isPackExpansion()))
           return false;
 
+        if (XHasMoreArg)
+          PackIterationSize = Y.pack_size();
+      }
+
+      ArrayRef<TemplateArgument> XP = X.pack_elements();
+      ArrayRef<TemplateArgument> YP = Y.pack_elements();
+      for (unsigned i = 0; i < PackIterationSize; ++i)
+        if (!isSameTemplateArg(Context, XP[i], YP[i], PartialOrdering,
+                               PackExpansionMatchesPack))
+          return false;
       return true;
   }
 
   llvm_unreachable("Invalid TemplateArgument Kind!");
 }
 
 /// Allocate a TemplateArgumentLoc where all locations have
 /// been initialized to the given location.
if (S.CheckTemplateArgumentList(Template, Partial->getLocation(), InstArgs,
                                   /*UpdateArgsWithConversions=*/true,
                                   &ConstraintsNotSatisfied))
     return ConstraintsNotSatisfied ? Sema::TDK_ConstraintsNotSatisfied :
                                      Sema::TDK_SubstitutionFailure;
 
   TemplateParameterList *TemplateParams = Template->getTemplateParameters();
   for (unsigned I = 0, E = TemplateParams->size(); I != E; ++I) {
     TemplateArgument InstArg = ConvertedInstArgs.data()[I];
-    if (!isSameTemplateArg(S.Context, TemplateArgs[I], InstArg)) {
+    if (!isSameTemplateArg(S.Context, TemplateArgs[I], InstArg,
+                           IsPartialOrdering)) {
       Info.Param = makeTemplateParameter(TemplateParams->getParam(I));
       Info.FirstArg = TemplateArgs[I];
       Info.SecondArg = InstArg;
       return Sema::TDK_NonDeducedMismatch;
     }
   }
 
   if (Trap.hasErrorOccurred())
static Sema::TemplateDeductionResult FinishTemplateArgumentDeduction(
   if (auto Result = ConvertDeducedTemplateArguments(
           S, Template, /*IsDeduced*/PartialOrdering, Deduced, Info, Builder))
     return Result;
 
   // Check that we produced the correct argument list.
   TemplateParameterList *TemplateParams = Template->getTemplateParameters();
   for (unsigned I = 0, E = TemplateParams->size(); I != E; ++I) {
     TemplateArgument InstArg = Builder[I];
-    if (!isSameTemplateArg(S.Context, TemplateArgs[I], InstArg,
-                           /*PackExpansionMatchesPack*/true)) {
+    if (!isSameTemplateArg(S.Context, TemplateArgs[I], InstArg, PartialOrdering,
+                           /*PackExpansionMatchesPack*/ true)) {
       Info.Param = makeTemplateParameter(TemplateParams->getParam(I));
       Info.FirstArg = TemplateArgs[I];
       Info.SecondArg = InstArg;
       return Sema::TDK_NonDeducedMismatch;
     }
   }
 
   if (Trap.hasErrorOccurred())
for (; ArgIdx != NumArgs; ++ArgIdx)
     // If this argument had no value deduced but was used in one of the types
     // used for partial ordering, then deduction fails.
     if (Deduced[ArgIdx].isNull() && UsedParameters[ArgIdx])
       return false;
 
   return true;
 }
 
-/// Determine whether this a function template whose parameter-type-list
-/// ends with a function parameter pack.
-static bool isVariadicFunctionTemplate(FunctionTemplateDecl *FunTmpl) {
-  FunctionDecl *Function = FunTmpl->getTemplatedDecl();
-  unsigned NumParams = Function->getNumParams();
-  if (NumParams == 0)
-    return false;
-
-  ParmVarDecl *Last = Function->getParamDecl(NumParams - 1);
-  if (!Last->isParameterPack())
-    return false;
-
-  // Make sure that no previous parameter is a parameter pack.
-  while (--NumParams > 0) {
-    if (Function->getParamDecl(NumParams - 1)->isParameterPack())
-      return false;
-  }
-
-  return true;
-}
-
 /// Returns the more specialized function template according
 /// to the rules of function template partial ordering (C++ [temp.func.order]).
 ///
 /// \param FT1 the first function template
 ///
 /// \param FT2 the second function template
 ///
 /// \param TPOC the context in which we are performing partial ordering of
FunctionTemplateDecl *Sema::getMoreSpecializedTemplate(
   bool Better2 = isAtLeastAsSpecializedAs(*this, Loc, FT2, FT1, TPOC,
                                           NumCallArguments2, Reversed);
 
   if (Better1 != Better2) // We have a clear winner
     return Better1 ? FT1 : FT2;
 
   if (!Better1 && !Better2) // Neither is better than the other
     return JudgeByConstraints();
 
-  // FIXME: This mimics what GCC implements, but doesn't match up with the
-  // proposed resolution for core issue 692. This area needs to be sorted out,
-  // but for now we attempt to maintain compatibility.
-  bool Variadic1 = isVariadicFunctionTemplate(FT1);
-  bool Variadic2 = isVariadicFunctionTemplate(FT2);
-  if (Variadic1 != Variadic2)
-    return Variadic1? FT2 : FT1;
+  // C++ [temp.deduct.partial]p11:
+  //   ... and if G has a trailing function parameter pack for which F does not
+  //   have a corresponding parameter, and if F does not have a trailing
+  //   function parameter pack, then F is more specialized than G.
+  FunctionDecl *FD1 = FT1->getTemplatedDecl();
+  FunctionDecl *FD2 = FT2->getTemplatedDecl();
+  unsigned NumParams1 = FD1->getNumParams();
+  unsigned NumParams2 = FD2->getNumParams();
+  bool Variadic1 = NumParams1 && FD1->parameters().back()->isParameterPack();
+  bool Variadic2 = NumParams2 && FD2->parameters().back()->isParameterPack();
+  if (Variadic1 != Variadic2) {
+    if (Variadic1 && NumParams1 > NumParams2)
+      return FT2;
+    if (Variadic2 && NumParams2 > NumParams1)
+      return FT1;
+  }
 
   return JudgeByConstraints();
 }
 
 /// Determine if the two templates are equivalent.
 static bool isSameTemplate(TemplateDecl *T1, TemplateDecl *T2) {
   if (T1 == T2)
     return true;
AtLeastAsSpecialized = !FinishTemplateArgumentDeduction(
         S, P2, /*IsPartialOrdering=*/true,
         TemplateArgumentList(TemplateArgumentList::OnStack,
                              TST1->template_arguments()),
         Deduced, Info);
   });
   return AtLeastAsSpecialized;
 }
 
+namespace {
+// A dummy pass to return nullptr instead of P2 when performing "more
+// specialized than primary" check.
+struct GetP2 {
+  template <typename T1, typename T2,
+            std::enable_if_t<std::is_same<T1, T2>::value, bool> = true>
+  T2 *operator()(T1 *, T2 *P2) {
+    return P2;
+  }
+  template <typename T1, typename T2,
+            std::enable_if_t<!std::is_same<T1, T2>::value, bool> = true>
+  T1 *operator()(T1 *, T2 *) {
+    return nullptr;
+  }
+};
+} // namespace
+
+/// Returns the more specialized template specialization between T1/P1 and
+/// T2/P2.
+/// - If IsMoreSpecialThanPrimaryCheck is true, T1/P1 is the partial
+///   specialization and T2/P2 is the primary template.
+/// - otherwise, both T1/P1 and T2/P2 are the partial specialization.
+///
+/// \param T1 the type of the first template partial specialization
+///
+/// \param T2 if IsMoreSpecialThanPrimaryCheck is true, the type of the second
+///           template partial specialization; otherwise, the type of the
+///           primary template.
+///
+/// \param P1 the first template partial specialization
+///
+/// \param P2 if IsMoreSpecialThanPrimaryCheck is true, the second template
+///           partial specialization; otherwise, the primary template.
+///
+/// \returns - If IsMoreSpecialThanPrimaryCheck is true, returns P1 if P1 is
+///            more specialized, returns nullptr if P1 is not more specialized.
+///          - otherwise, returns the more specialized template partial
+///            specialization. If neither partial specialization is more
+///            specialized, returns NULL.
+template <typename TemplateLikeDecl, typename PrimaryDel>
+static TemplateLikeDecl *
+getMoreSpecialized(Sema &S, QualType T1, QualType T2, TemplateLikeDecl *P1,
+                   PrimaryDel *P2, TemplateDeductionInfo &Info) {
+  constexpr bool IsMoreSpecialThanPrimaryCheck =
+      !std::is_same<TemplateLikeDecl, PrimaryDel>::value;
+
+  bool Better1 = isAtLeastAsSpecializedAs(S, T1, T2, P2, Info);
+  if (IsMoreSpecialThanPrimaryCheck && !Better1)
+    return nullptr;
+
+  bool Better2 = isAtLeastAsSpecializedAs(S, T2, T1, P1, Info);
+  if (IsMoreSpecialThanPrimaryCheck && !Better2)
+    return P1;
+
+  if (!Better1 && !Better2)
+    return nullptr;
+
+  if (Better1 && Better2) {
+    bool ClangABICompat14 = S.Context.getLangOpts().getClangABICompat() <=
+                            LangOptions::ClangABI::Ver14;
+    if (!ClangABICompat14) {
+      // Consider this a fix for CWG1432. Similar to the fix for CWG1395.
+      auto *TST1 = T1->castAs<TemplateSpecializationType>();
+      auto *TST2 = T2->castAs<TemplateSpecializationType>();
+      if (TST1->getNumArgs()) {
+        const TemplateArgument &TA1 = TST1->template_arguments().back();
+        if (TA1.getKind() == TemplateArgument::Pack) {
+          assert(TST1->getNumArgs() == TST2->getNumArgs());
+          const TemplateArgument &TA2 = TST2->template_arguments().back();
+          assert(TA2.getKind() == TemplateArgument::Pack);
+          unsigned PackSize1 = TA1.pack_size();
+          unsigned PackSize2 = TA2.pack_size();
+          bool IsPackExpansion1 =
+              PackSize1 && TA1.pack_elements().back().isPackExpansion();
+          bool IsPackExpansion2 =
+              PackSize2 && TA2.pack_elements().back().isPackExpansion();
+          if (PackSize1 != PackSize2 && IsPackExpansion1 != IsPackExpansion2) {
+            if (PackSize1 > PackSize2 && IsPackExpansion1)
+              return GetP2()(P1, P2);
+            if (PackSize1 < PackSize2 && IsPackExpansion2)
+              return P1;
+          }
+        }
+      }
+    }
+
+    llvm::SmallVector<const Expr *, 3> AC1, AC2;
+    P1->getAssociatedConstraints(AC1);
+    P2->getAssociatedConstraints(AC2);
+    bool AtLeastAsConstrained1, AtLeastAsConstrained2;
+    if (S.IsAtLeastAsConstrained(P1, AC1, P2, AC2, AtLeastAsConstrained1) ||
+        (IsMoreSpecialThanPrimaryCheck && !AtLeastAsConstrained1))
+      return nullptr;
+    if (S.IsAtLeastAsConstrained(P2, AC2, P1, AC1, AtLeastAsConstrained2))
+      return nullptr;
+    if (AtLeastAsConstrained1 == AtLeastAsConstrained2)
+      return nullptr;
+    return AtLeastAsConstrained1 ? P1 : GetP2()(P1, P2);
+  }
+
+  return Better1 ? P1 : GetP2()(P1, P2);
+}
+
 /// Returns the more specialized class template partial specialization
 /// according to the rules of partial ordering of class template partial
 /// specializations (C++ [temp.class.order]).
 ///
 /// \param PS1 the first class template partial specialization
 ///
 /// \param PS2 the second class template partial specialization
 ///
 /// \returns the more specialized class template partial specialization. If
 /// neither partial specialization is more specialized, returns NULL.
 ClassTemplatePartialSpecializationDecl *
 Sema::getMoreSpecializedPartialSpecialization(
                                   ClassTemplatePartialSpecializationDecl *PS1,
                                   ClassTemplatePartialSpecializationDecl *PS2,
                                               SourceLocation Loc) {
   QualType PT1 = PS1->getInjectedSpecializationType();
   QualType PT2 = PS2->getInjectedSpecializationType();
 
   TemplateDeductionInfo Info(Loc);
-  bool Better1 = isAtLeastAsSpecializedAs(*this, PT1, PT2, PS2, Info);
-  bool Better2 = isAtLeastAsSpecializedAs(*this, PT2, PT1, PS1, Info);
-
-  if (!Better1 && !Better2)
-      return nullptr;
-  if (Better1 && Better2) {
-    llvm::SmallVector<const Expr *, 3> AC1, AC2;
-    PS1->getAssociatedConstraints(AC1);
-    PS2->getAssociatedConstraints(AC2);
-    bool AtLeastAsConstrained1, AtLeastAsConstrained2;
-    if (IsAtLeastAsConstrained(PS1, AC1, PS2, AC2, AtLeastAsConstrained1))
-      return nullptr;
-    if (IsAtLeastAsConstrained(PS2, AC2, PS1, AC1, AtLeastAsConstrained2))
-      return nullptr;
-    if (AtLeastAsConstrained1 == AtLeastAsConstrained2)
-      return nullptr;
-    return AtLeastAsConstrained1 ? PS1 : PS2;
-  }
-
-  return Better1 ? PS1 : PS2;
+  return getMoreSpecialized(*this, PT1, PT2, PS1, PS2, Info);
 }
 
 bool Sema::isMoreSpecializedThanPrimary(
     ClassTemplatePartialSpecializationDecl *Spec, TemplateDeductionInfo &Info) {
   ClassTemplateDecl *Primary = Spec->getSpecializedTemplate();
   QualType PrimaryT = Primary->getInjectedClassNameSpecialization();
   QualType PartialT = Spec->getInjectedSpecializationType();
-  if (!isAtLeastAsSpecializedAs(*this, PartialT, PrimaryT, Primary, Info))
-    return false;
-  if (!isAtLeastAsSpecializedAs(*this, PrimaryT, PartialT, Spec, Info))
-    return true;
+
+  ClassTemplatePartialSpecializationDecl *MaybeSpec =
+      getMoreSpecialized(*this, PartialT, PrimaryT, Spec, Primary, Info);
+  if (MaybeSpec)
     Info.clearSFINAEDiagnostic();
-  llvm::SmallVector<const Expr *, 3> PrimaryAC, SpecAC;
-  Primary->getAssociatedConstraints(PrimaryAC);
-  Spec->getAssociatedConstraints(SpecAC);
-  bool AtLeastAsConstrainedPrimary, AtLeastAsConstrainedSpec;
-  if (IsAtLeastAsConstrained(Spec, SpecAC, Primary, PrimaryAC,
-                             AtLeastAsConstrainedSpec))
-    return false;
-  if (!AtLeastAsConstrainedSpec)
-    return false;
-  if (IsAtLeastAsConstrained(Primary, PrimaryAC, Spec, SpecAC,
-                             AtLeastAsConstrainedPrimary))
-    return false;
-  return !AtLeastAsConstrainedPrimary;
+  return MaybeSpec;
 }
 
 VarTemplatePartialSpecializationDecl *
 Sema::getMoreSpecializedPartialSpecialization(
     VarTemplatePartialSpecializationDecl *PS1,
     VarTemplatePartialSpecializationDecl *PS2, SourceLocation Loc) {
   // Pretend the variable template specializations are class template
   // specializations and form a fake injected class name type for comparison.
   assert(PS1->getSpecializedTemplate() == PS2->getSpecializedTemplate() &&
          "the partial specializations being compared should specialize"
          " the same template.");
   TemplateName Name(PS1->getSpecializedTemplate());
   TemplateName CanonTemplate = Context.getCanonicalTemplateName(Name);
   QualType PT1 = Context.getTemplateSpecializationType(
       CanonTemplate, PS1->getTemplateArgs().asArray());
   QualType PT2 = Context.getTemplateSpecializationType(
       CanonTemplate, PS2->getTemplateArgs().asArray());
 
   TemplateDeductionInfo Info(Loc);
-  bool Better1 = isAtLeastAsSpecializedAs(*this, PT1, PT2, PS2, Info);
-  bool Better2 = isAtLeastAsSpecializedAs(*this, PT2, PT1, PS1, Info);
-
-  if (!Better1 && !Better2)
-    return nullptr;
-  if (Better1 && Better2) {
-    llvm::SmallVector<const Expr *, 3> AC1, AC2;
-    PS1->getAssociatedConstraints(AC1);
-    PS2->getAssociatedConstraints(AC2);
-    bool AtLeastAsConstrained1, AtLeastAsConstrained2;
-    if (IsAtLeastAsConstrained(PS1, AC1, PS2, AC2, AtLeastAsConstrained1))
-      return nullptr;
-    if (IsAtLeastAsConstrained(PS2, AC2, PS1, AC1, AtLeastAsConstrained2))
-      return nullptr;
-    if (AtLeastAsConstrained1 == AtLeastAsConstrained2)
-      return nullptr;
-    return AtLeastAsConstrained1 ? PS1 : PS2;
-  }
-
-  return Better1 ? PS1 : PS2;
+  return getMoreSpecialized(*this, PT1, PT2, PS1, PS2, Info);
 }
 
 bool Sema::isMoreSpecializedThanPrimary(
     VarTemplatePartialSpecializationDecl *Spec, TemplateDeductionInfo &Info) {
   TemplateDecl *Primary = Spec->getSpecializedTemplate();
   // FIXME: Cache the injected template arguments rather than recomputing
   // them for each partial specialization.
   SmallVector<TemplateArgument, 8> PrimaryArgs;
   Context.getInjectedTemplateArgs(Primary->getTemplateParameters(),
                                   PrimaryArgs);
 
   TemplateName CanonTemplate =
       Context.getCanonicalTemplateName(TemplateName(Primary));
   QualType PrimaryT = Context.getTemplateSpecializationType(
       CanonTemplate, PrimaryArgs);
   QualType PartialT = Context.getTemplateSpecializationType(
       CanonTemplate, Spec->getTemplateArgs().asArray());
 
-  if (!isAtLeastAsSpecializedAs(*this, PartialT, PrimaryT, Primary, Info))
-    return false;
-  if (!isAtLeastAsSpecializedAs(*this, PrimaryT, PartialT, Spec, Info))
-    return true;
+  VarTemplatePartialSpecializationDecl *MaybeSpec =
+      getMoreSpecialized(*this, PartialT, PrimaryT, Spec, Primary, Info);
+  if (MaybeSpec)
     Info.clearSFINAEDiagnostic();
-  llvm::SmallVector<const Expr *, 3> PrimaryAC, SpecAC;
-  Primary->getAssociatedConstraints(PrimaryAC);
-  Spec->getAssociatedConstraints(SpecAC);
-  bool AtLeastAsConstrainedPrimary, AtLeastAsConstrainedSpec;
-  if (IsAtLeastAsConstrained(Spec, SpecAC, Primary, PrimaryAC,
-                             AtLeastAsConstrainedSpec))
-    return false;
-  if (!AtLeastAsConstrainedSpec)
-    return false;
-  if (IsAtLeastAsConstrained(Primary, PrimaryAC, Spec, SpecAC,
-                             AtLeastAsConstrainedPrimary))
-    return false;
-  return !AtLeastAsConstrainedPrimary;
+  return MaybeSpec;
 }
 
 bool Sema::isTemplateTemplateParameterAtLeastAsSpecializedAs(
      TemplateParameterList *P, TemplateDecl *AArg, SourceLocation Loc) {
   // C++1z [temp.arg.template]p4: (DR 150)
   //   A template template-parameter P is at least as specialized as a
   //   template template-argument A if, given the following rewrite to two
   //   function templates...

clang/test/CXX/drs/dr6xx.cpp

#if __cplusplus <= 201703L
     // expected-error@-2 {{C++20 extension}}
 #endif
 
     // This is not.
     template g<int>(a); // expected-error {{expected expression}}
   }
 }
 
 namespace dr692 { // dr692: no
+  // Also see dr1395.
+
   namespace temp_func_order_example2 {
     template <typename T, typename U> struct A {};
     template <typename T, typename U> void f(U, A<U, T> *p = 0); // expected-note {{candidate}}
     template <typename U> int &f(U, A<U, U> *p = 0); // expected-note {{candidate}}
-    template <typename T> void g(T, T = T());
-    template <typename T, typename... U> void g(T, U...); // expected-error 0-1{{C++11}}
+    template <typename T> void g(T, T = T()); // expected-note {{candidate}}
+    template <typename T, typename... U> void g(T, U...); // expected-note {{candidate}} expected-error 0-1{{C++11}}
     void h() {
       int &r = f<int>(42, (A<int, int> *)0);
       f<int>(42); // expected-error {{ambiguous}}
-      // FIXME: We should reject this due to ambiguity between the pack and the
-      // default argument. Only parameters with arguments are considered during
-      // partial ordering of function templates.
-      g(42);
+      g(42); // expected-error {{ambiguous}}
     }
   }
 
   namespace temp_func_order_example3 {
     template <typename T, typename... U> void f(T, U...); // expected-error 0-1{{C++11}}
     template <typename T> void f(T);
     template <typename T, typename... U> int &g(T *, U...); // expected-error 0-1{{C++11}}
     template <typename T> void g(T);
namespace dr692 { // dr692: no
   }
 
   namespace temp_deduct_type_example1 {
     template <class T1, class ...Z> class S; // expected-error 0-1{{C++11}}
     template <class T1, class ...Z> class S<T1, const Z&...>; // expected-error 0-1{{C++11}}
     template <class T1, class T2> class S<T1, const T2&> {};
     S<int, const int&> s;
 
-    // FIXME: This should select the first partial specialization. Deduction of
-    // the second from the first should succeed, because we should ignore the
-    // trailing pack in A with no corresponding P.
     template<class T, class... U> struct A; // expected-error 0-1{{C++11}}
-    template<class T1, class T2, class... U> struct A<T1,T2*,U...>; // expected-note {{matches}} expected-error 0-1{{C++11}}
-    template<class T1, class T2> struct A<T1,T2> {}; // expected-note {{matches}}
-    template struct A<int, int*>; // expected-error {{ambiguous}}
+    template<class T1, class T2, class... U> struct A<T1,T2*,U...> {}; // expected-error 0-1{{C++11}}
+    template<class T1, class T2> struct A<T1,T2>;
+    template struct A<int, int*>;
   }
 
   namespace temp_deduct_type_example3 {
-    // FIXME: This should select the first template, as in the case above.
-    template<class T, class... U> void f(T*, U...){} // expected-note {{candidate}} expected-error 0-1{{C++11}}
-    template<class T> void f(T){} // expected-note {{candidate}}
-    template void f(int*); // expected-error {{ambiguous}}
+    template<class T, class... U> void f(T*, U...){} // expected-error 0-1{{C++11}}
+    template<class T> void f(T){}
+    template void f(int*);
   }
 }
 
 namespace dr696 { // dr696: yes
   void f(const int*);
   void g() {
     const int N = 10; // expected-note 1+{{here}}
     struct A {

clang/test/CodeGen/partial-order-variadic.cpp

+// RUN: %clang_cc1 -triple x86_64-unknown-unknown -fclang-abi-compat=14 -DCLANG_ABI_COMPAT=14 %s -emit-llvm -disable-llvm-passes -o - | FileCheck %s --check-prefix=CHECK-14
+// RUN: %clang_cc1 -triple x86_64-unknown-unknown %s -emit-llvm -disable-llvm-passes -o - | FileCheck %s
+
+#if defined(CLANG_ABI_COMPAT) && CLANG_ABI_COMPAT <= 14
+
+// CHECK-14: define dso_local void @_ZN24temp_func_order_example31hEi(i32 noundef %i)
+// CHECK-14-NEXT: entry:
+// CHECK-14-NEXT:   %i.addr = alloca i32, align 4
+// CHECK-14-NEXT:   %r = alloca ptr, align 8
+// CHECK-14-NEXT:   store i32 %i, ptr %i.addr, align 4
+// CHECK-14-NEXT:   %call = call noundef nonnull align 4 dereferenceable(4) ptr @_ZN24temp_func_order_example31gIiJEEERiPT_DpT0_(ptr noundef %i.addr)
+// CHECK-14-NEXT:   store ptr %call, ptr %r, align 8
+// CHECK-14-NEXT:   ret void
+
+namespace temp_func_order_example3 {
+  template <typename T, typename... U> int &g(T *, U...);
+  template <typename T> void g(T);
+  void h(int i) {
+    int &r = g(&i);
+  }
+}
+
+#else
+
+// CHECK: %"struct.temp_deduct_type_example1::A" = type { i8 }
+
+// CHECK: $_ZN25temp_deduct_type_example31fIiJEEEvPT_DpT0_ = comdat any
+
+// CHECK: define dso_local void @_ZN25temp_deduct_type_example11fEv()
+// CHECK-NEXT: entry:
+// CHECK-NEXT:   %a = alloca %"struct.temp_deduct_type_example1::A", align 1
+// CHECK-NEXT:   ret void
+
+// CHECK: define weak_odr void @_ZN25temp_deduct_type_example31fIiJEEEvPT_DpT0_(ptr noundef %0)
+// CHECK-NEXT: entry:
+// CHECK-NEXT:   %.addr = alloca ptr, align 8
+// CHECK-NEXT:   store ptr %0, ptr %.addr, align 8
+// CHECK-NEXT:   ret void
+
+namespace temp_deduct_type_example1 {
+  template<class T, class... U> struct A;
+  template<class T1, class T2, class... U> struct A<T1,T2*,U...> {};
+  template<class T1, class T2> struct A<T1,T2>;
+  template struct A<int, int*>;
+  void f() { A<int, int*> a; }
+}
+
+namespace temp_deduct_type_example3 {
+  template<class T, class... U> void f(T*, U...){}
+  template<class T> void f(T){}
+  template void f(int*);
+}
+
+#endif

clang/www/cxx_dr_status.html

<tr id="691">
     <td>C++11</td>
     <td>Template parameter packs in class template partial specializations</td>
     <td class="none" align="center">Unknown</td>
   </tr>
   <tr id="692">
     <td><a href="https://wg21.link/cwg692">692</a></td>
     <td>C++11</td>
     <td>Partial ordering of variadic class template partial specializations</td>
-    <td class="none" align="center">No</td>
+    <td class="unreleased" align="center">Clang 16</td>
   </tr>
   <tr id="693">
     <td><a href="https://wg21.link/cwg693">693</a></td>
     <td>CD2</td>
     <td>New string types and deprecated conversion</td>
     <td class="none" align="center">Unknown</td>
   </tr>
   <tr id="694">
<tr id="1394">
     <td>CD3</td>
     <td>Incomplete types as parameters of deleted functions</td>
     <td class="unreleased" align="center">Clang 15</td>
   </tr>
   <tr id="1395">
     <td><a href="https://wg21.link/cwg1395">1395</a></td>
     <td>C++17</td>
     <td>Partial ordering of variadic templates reconsidered</td>
-    <td class="none" align="center">Unknown</td>
+    <td class="unreleased" align="center">Clang 16</td>
   </tr>
   <tr class="open" id="1396">
     <td><a href="https://wg21.link/cwg1396">1396</a></td>
     <td>open</td>
     <td>Deferred instantiation and checking of non-static data member initializers</td>
     <td align="center">Not resolved</td>
   </tr>
   <tr id="1397">