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

ychen added a comment. 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 ,

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

aaron.ballman added a comment. 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)

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

rjmccall added a comment. 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? Repository: rG LLVM Github Monorepo CHANGES SINCE LAST

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

ychen added a comment. 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

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

hubert.reinterpretcast added a comment. 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)

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

rjmccall added a comment. 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

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

ychen added a comment. 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

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

tahonermann added a comment. 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,

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

ychen added a comment. 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

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

hubert.reinterpretcast added a comment. 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".

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

ychen added a comment. > 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

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

hubert.reinterpretcast added a comment. 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

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

royjacobson added a comment. 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

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

erichkeane added a comment. 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

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

aaron.ballman added a reviewer: hubert.reinterpretcast. aaron.ballman added a subscriber: hubert.reinterpretcast. aaron.ballman added a comment. In D128745#3832598 , @ychen wrote: > In D128745#3832432 , @rjmccall

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

ychen added a comment. 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

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

rjmccall added a comment. 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

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

tahonermann added a comment. 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

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

ychen added a comment. In D128745#3752593 , @ychen wrote: > In D128745#3752148 , @ychen wrote: > >> In D128745#3751471 , @joanahalili >> wrote: >> >>> We have some

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

ychen added a comment. 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

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

ychen added a comment. 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

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

joanahalili added a comment. 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 void F(T value) {} template void F(const T& value){}

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

ychen added a comment. 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. Repository: rG LLVM Github Monorepo

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

ychen added a comment. 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 >

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

royjacobson added a comment. Stopping by - I assume this targets Clang 16 now, so the abi compat level should be adjusted to 15. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D128745/new/ https://reviews.llvm.org/D128745

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

michele.scandale added a comment. 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

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

ychen added a comment. In D128745#3727770 , @abrachet wrote: > In D128745#3727715 , @ychen wrote: > >> In D128745#3727697 , @abrachet >> wrote: >> >>> This is breaking

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

abrachet added a comment. In D128745#3727715 , @ychen wrote: > In D128745#3727697 , @abrachet > wrote: > >> This is breaking us. >> >> template struct S; // #1 >> >> template struct S {}; // #2 >> >>

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

ychen added a comment. In D128745#3727715 , @ychen wrote: > In D128745#3727697 , @abrachet > wrote: > >> This is breaking us. >> >> template struct S; // #1 >> >> template struct S {}; // #2 >> >> The

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

ychen added a comment. In D128745#3727697 , @abrachet wrote: > This is breaking us. > > template struct S; // #1 > > template struct S {}; // #2 > > The following compiled before but is now broken, (we use > `-fclang-abi-compat=13.0`). We get a

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

abrachet added subscribers: mcgrathr, abrachet. abrachet added a comment. This is breaking us. template struct S; // #1 template struct S {}; // #2 The following compiled before but is now broken, (we use `-fclang-abi-compat=13.0`). We get a warning from

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

This revision was landed with ongoing or failed builds. This revision was automatically updated to reflect the committed changes. Closed by commit rG6afcc4a459ea: [c++] implements DR692, DR1395 and tentatively DR1432, about partial ordering… (authored by ychen). Repository: rG LLVM Github

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

ychen updated this revision to Diff 452564. ychen added a comment. - rebase Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D128745/new/ https://reviews.llvm.org/D128745 Files: clang/docs/ReleaseNotes.rst clang/lib/Sema/SemaTemplateDeduction.cpp

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

aaron.ballman accepted this revision. aaron.ballman added a comment. LGTM! Comment at: clang/lib/Sema/SemaTemplateDeduction.cpp:5444 + if (Better1 && Better2) { +bool ClangABICompat14 = S.Context.getLangOpts().getClangABICompat() <= +

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

ychen added inline comments. Comment at: clang/lib/Sema/SemaTemplateDeduction.cpp:5444 + if (Better1 && Better2) { +bool ClangABICompat14 = S.Context.getLangOpts().getClangABICompat() <= +LangOptions::ClangABI::Ver14;

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

ychen updated this revision to Diff 444363. ychen added a comment. - add CodeGen tests Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D128745/new/ https://reviews.llvm.org/D128745 Files: clang/docs/ReleaseNotes.rst

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

erichkeane accepted this revision. erichkeane added a comment. Nothing sticks out to me, LGTM too. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D128745/new/ https://reviews.llvm.org/D128745 ___

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

aaron.ballman accepted this revision. aaron.ballman added a comment. This revision is now accepted and ready to land. 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

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

ychen added inline comments. Comment at: clang/lib/Sema/SemaTemplateDeduction.cpp:5423-5424 +template ::value> +static TemplateLikeDecl * aaron.ballman wrote: > Can't this be a local constexpr variable instead of an NTTP, or are there > reasons you want to

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

ychen updated this revision to Diff 444099. ychen marked 4 inline comments as done. ychen added a comment. - add Clang ABI check - replace one NTTP with a local constexpr variable - style changes Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION

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

cor3ntin added inline comments. Comment at: clang/lib/Sema/SemaTemplateDeduction.cpp:5444 +if (TST1->getNumArgs()) { + const TemplateArgument = TST1->template_arguments().back(); + if (TA1.getKind() == TemplateArgument::Pack) { aaron.ballman

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

ychen added a comment. 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:

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

erichkeane added inline comments. Comment at: clang/lib/Sema/SemaTemplateDeduction.cpp:1110 + // Ai is ignored; + if (PartialOrdering && ArgIdx + 1 == NumArgs && + isa(Args[ArgIdx])) aaron.ballman wrote: > Why are you checking `ArgIdx + 1 == NumArgs`

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

aaron.ballman added a comment. 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). Comment at:

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

ychen added inline comments. Comment at: clang/lib/Sema/SemaTemplateDeduction.cpp:5184 - // 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, ychen wrote: >

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

ychen added inline comments. Comment at: clang/lib/Sema/SemaTemplateDeduction.cpp:2488 +bool XHasMoreArg = X.pack_size() > Y.pack_size(); +if (!(XHasMoreArg && X.pack_elements().back().isPackExpansion()) && +!(!XHasMoreArg &&