[PATCH] D147722: [Concepts] Fix Function Template Concepts comparisons

[Richard Smith - zygoloid via Phabricator via cfe-commits]

rsmith added inline comments.


================
Comment at: clang/lib/Sema/SemaTemplateDeduction.cpp:2854-2858
-template <>
-bool DeducedArgsNeedReplacement<ClassTemplatePartialSpecializationDecl>(
-    ClassTemplatePartialSpecializationDecl *Spec) {
-  return !Spec->isClassScopeExplicitSpecialization();
-}
----------------
It's not obvious to me what this was doing, so I'm not sure whether it's correct to remove it. Can you explain? It makes me uncomfortable that we would treat class template partial specializations and variable template partial specializations differently, at least without a good explanation for why that's appropriate -- perhaps we should apply this same set of changes to variable template partial specializations too, and remove this mechanism entirely?


================
Comment at: clang/lib/Sema/SemaTemplateInstantiate.cpp:318
+      // specialized template anyway, so any substitution we would do with these
+      // partially specialized arguments would 'wrong' and confuse constraint
+      // instantiation. We only do this in the case of a constraint check, since
----------------



================
Comment at: clang/lib/Sema/SemaTemplateInstantiate.cpp:322
+      // they mean.
+      R = Response::UseNextDecl(PartialClassTemplSpec);
     } else if (const auto *ClassTemplSpec =
----------------
Can we produce a "done" response instead here? It doesn't seem right to carry on to the next level here without adding a level of template arguments -- if any further arguments were somehow collected, they would be collected at  the wrong template depth. But, as far as I can determine, we should never encounter a partial specialization declaration except in cases where there is nothing remaining to be substituted (all outer substitutions would be identity substitutions). I think the same thing applies when we reach a class template declaration (the first branch in `HandleRecordDecl`) -- we should just be able to stop in that case too, rather than collecting some identity template arguments and carrying on to an outer level.

If we can't produce a "done" response here for some reason, then we should collect a level of identity template arguments instead.


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

https://reviews.llvm.org/D147722