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

[Aaron Ballman via Phabricator via cfe-commits]

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 had a chance to look again though.



================
Comment at: clang/lib/Sema/SemaTemplateDeduction.cpp:1110
+  //   Ai is ignored;
+  bool ClangABICompat14 = S.Context.getLangOpts().getClangABICompat() <=
+                          LangOptions::ClangABI::Ver14;
----------------
Can you add CodeGen test coverage for both ABI modes (new RUN line with `-fclang-abi-compat=14`)


================
Comment at: clang/lib/Sema/SemaTemplateDeduction.cpp:5444
+  if (Better1 && Better2) {
+    bool ClangABICompat14 = S.Context.getLangOpts().getClangABICompat() <=
+                            LangOptions::ClangABI::Ver14;
----------------
Can you add CodeGen test coverage for both ABI modes (new RUN line with `-fclang-abi-compat=14`)


================
Comment at: clang/lib/Sema/SemaTemplateDeduction.cpp:5426-5427
+static TemplateLikeDecl *
+getMoreSpecialized(Sema &S, QualType T1, QualType T2, TemplateLikeDecl *P1,
+                   PrimaryDel *P2, TemplateDeductionInfo &Info) {
+  bool Better1 = isAtLeastAsSpecializedAs(S, T1, T2, P2, Info);
----------------
ychen wrote:
> aaron.ballman wrote:
> > Curiosity question -- can you make `P1` and `P2` be pointers to `const` without many other viral changes to existing code?
> `isAtLeastAsSpecializedAs` uses template instantiations which modify P1/P2.
Good to know, then no changes needed here, thanks!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D128745