[PATCH] D145034: [Clang][Sema] Start fixing handling of out-of-line definitions of constrained templates

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

rsmith accepted this revision.
rsmith added a comment.
This revision is now accepted and ready to land.

Looks good to me. This is fixing an important bug, and while there's more cleanup I'd like for us to do, it seems important to get this fix landed first.

My understanding is that the release notes should be updated for bug fixes such as this; please can you add a bullet point to the list of bug fixes in docs/ReleaseNotes.rst?



================
Comment at: clang/lib/Sema/SemaCXXScopeSpec.cpp:141
+              ClassTemplate->getInjectedClassNameSpecialization();
+          if (Context.hasSameType(Injected, ContextType))
+            return ClassTemplate->getTemplatedDecl();
----------------
alexander-shaposhnikov wrote:
> rsmith wrote:
> > 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.)
> The problem is that currently the template parameter list (the one which we are supposed to pick above) is not always present,
> thus the fallback and this check are both still necessary. It happens e.g. for invalid code and if i remove the fallback it triggers more changes to the diagnostic output, other than that there might be more cases where we currently don't set TemplateParamLists for CXXScopeSpec, i kinda wanted to move incrementally.
OK. Can we do the check in the case where we do have a list of template parameter lists, at least, or does that also cause more diagnostic disruption than you'd like to deal with in this change?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D145034