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

_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Reply via email to