[PATCH] D133683: [c++] implements tentative DR1432 for partial ordering of function template

[Matheus Izvekov via Phabricator via cfe-commits]

mizvekov added inline comments.


================
Comment at: clang/lib/Sema/SemaTemplateDeduction.cpp:5195-5196
+    for (int i = 0, e = std::min(NumParams1, NumParams2); i < e; ++i) {
+      QualType T1 = FD1->getParamDecl(i)->getType().getDesugaredType(Context);
+      QualType T2 = FD2->getParamDecl(i)->getType().getDesugaredType(Context);
+      auto *TST1 = dyn_cast<TemplateSpecializationType>(T1);
----------------
This should work, as all the properties you are testing below are present on the canonical type.


================
Comment at: clang/lib/Sema/SemaTemplateDeduction.cpp:5455-5456
     if (!ClangABICompat15) {
-      // Consider this a fix for CWG1432. Similar to the fix for CWG1395.
       auto *TST1 = T1->castAs<TemplateSpecializationType>();
       auto *TST2 = T2->castAs<TemplateSpecializationType>();
+      const TemplateArgument &TA1 = TST1->template_arguments().back();
----------------
This is a bug, T1 and T2 are not in general canonical types for this function, and this castAs can pick up an alias template, and these should never participate in deduction.

(Also, can you please add a test for that?)

You should not need to keep sugar here, everything being tested below is present on the canonical type, so this is simple to solve, as suggested.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D133683