[PATCH] D145034: [Clang][Sema] Preparations to fix handling of out-of-line definitions of constrained templates

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

rsmith added inline comments.


================
Comment at: clang/lib/Sema/SemaCXXScopeSpec.cpp:113-114
+          if (TemplateParamLists.size() == 1) {
+            // FIXME: pick the correct template parameter list based on NNS, SS
+            // and getCurScope().
+            TemplateParameterList *L = TemplateParamLists[0];
----------------
I think this can be done fairly straightforwardly: we should pick out the template parameter list whose depth equals the depth of `ClassTemplate->getTemplateParameters()`.


================
Comment at: clang/lib/Sema/SemaCXXScopeSpec.cpp:141
+              ClassTemplate->getInjectedClassNameSpecialization();
+          if (Context.hasSameType(Injected, ContextType))
+            return ClassTemplate->getTemplatedDecl();
----------------
This should also be guarded by a check that `TemplateParameterListsAreEqual` between `ClassTemplate->getTemplateParameters()` and the template parameter list we picked out of the given array of template parameter lists.

(With that check in place, we can move this back before the search for partial specializations.)


================
Comment at: clang/test/CXX/temp/temp.decls/temp.class.spec/temp.class.spec.mfunc/p1-neg.cpp:2-4
+// XFAIL: *
+// NOTE: This test is marked XFAIL until the diagnostics of
+// too many template parameters is fixed.
----------------
alexander-shaposhnikov wrote:
> rsmith wrote:
> > What is the new diagnostic output? (Presumably we no longer match the partial specialization when trying to match the declaration on line 25, because the template parameter list doesn't match.)
> the new diagnostic output:
>   error: nested name specifier 'A<T *, 2>::' for declaration does not refer into a class, class template or class template partial specialization 
> 
> ```
> template<typename T, int N>
> void A<T*, 2>::f0() { }
> ```
> 
> 
Hm, it'd definitely be nice to produce some notes pointing out the non-matching candidates, and ideally, why they don't match. But I don't think this is so terrible that we need to block the patch on it. I think for now you should update the test expectations and update the FIXME below to say what we want here, and remove the XFAIL.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D145034