[PATCH] -fdelayed-template-parsing: handle cases where a late-parsed function is not a direct member of a template, but rather nested inside a class that's a member of a template (PR19613)

[Richard Smith]

I'm well into nitpicking mode now; please go ahead and commit after this round.

... and then we can talk about function definitions inside lambdas inside variable templates. I'm sure that'll make your day =)

================
Comment at: lib/Parse/ParseTemplate.cpp:1266-1270
@@ -1273,10 +1265,7 @@
 
-  DeclaratorDecl *Declarator = dyn_cast<DeclaratorDecl>(FunD);
-  const unsigned DeclaratorNumTemplateParameterLists = 
-      (Declarator ? Declarator->getNumTemplateParameterLists() : 0);
-  if (Declarator && DeclaratorNumTemplateParameterLists != 0) {
-    Actions.ActOnReenterDeclaratorTemplateScope(getCurScope(), Declarator);
-    CurTemplateDepthTracker.addDepth(DeclaratorNumTemplateParameterLists);
+  if (unsigned NumLists = FunD->getNumTemplateParameterLists()) {
+    Actions.ActOnReenterTemplateScope(getCurScope(), FunD);
+    CurTemplateDepthTracker.addDepth(NumLists);
   }
   Actions.ActOnReenterTemplateScope(getCurScope(), LPT.D);
   ++CurTemplateDepthTracker;
----------------
Hans Wennborg wrote:
> Richard Smith wrote:
> > Can you push the function template case into `ActOnReenterTemplateScope` too?
> I can make Actions.ActOnReenterTemplateScope(getCurScope(), FunD); be covered by the loop above if I make sure not to call Actions.PushDeclContext() on it.
> 
> I have no idea why we then do an extra Actions.ActOnReenterTemplateScope(getCurScope(), LPT.D); since that should have the same effect. But if I remove that, we start failing tests, so I guess it needs to be there.
I meant, can you make `ActOnReenterTemplateScope` "do the right thing" when given the pattern declaration of a function template? That is, call `getDescribedFunctionTemplate` on it, and handle the template parameter list from the function template too. (That's why you still need this extra line -- `LPT.D` here is the template, `FunD` is the pattern within the template.)

================
Comment at: lib/Parse/ParseTemplate.cpp:1252
@@ -1251,17 +1251,3 @@
   for (; II != DeclContextsToReenter.rend(); ++II) {
-    if (ClassTemplatePartialSpecializationDecl *MD =
-            dyn_cast_or_null<ClassTemplatePartialSpecializationDecl>(*II)) {
-      TemplateParamScopeStack.push_back(
-          new ParseScope(this, Scope::TemplateParamScope));
-      Actions.ActOnReenterTemplateScope(getCurScope(), MD);
-      ++CurTemplateDepthTracker;
-    } else if (CXXRecordDecl *MD = dyn_cast_or_null<CXXRecordDecl>(*II)) {
-      bool IsClassTemplate = MD->getDescribedClassTemplate() != 0;
-      TemplateParamScopeStack.push_back(
-          new ParseScope(this, Scope::TemplateParamScope, 
-                        /*ManageScope*/IsClassTemplate));
-      Actions.ActOnReenterTemplateScope(getCurScope(),
-                                        MD->getDescribedClassTemplate());
-      if (IsClassTemplate) 
-        ++CurTemplateDepthTracker;
-    }
+    Decl *D = cast<Decl>(*II);
+
----------------
Maybe sink this into its only use.

================
Comment at: lib/Sema/SemaDeclCXX.cpp:5987
@@ -5986,13 +5986,3 @@
 
-  int NumParamList = D->getNumTemplateParameterLists();
-  for (int i = 0; i < NumParamList; i++) {
-    TemplateParameterList* Params = D->getTemplateParameterList(i);
-    for (TemplateParameterList::iterator Param = Params->begin(),
-                                      ParamEnd = Params->end();
-          Param != ParamEnd; ++Param) {
-      NamedDecl *Named = cast<NamedDecl>(*Param);
-      if (Named->getDeclName()) {
-        S->AddDecl(Named);
-        IdResolver.AddDecl(Named);
-      }
-    }
+  SmallVector<TemplateParameterList *, 4> ParameterLists;
+
----------------
Please add a comment saying that it doesn't matter what order we collect the template parameter lists in, so future readers don't worry that they're in the wrong order like I just did =)

================
Comment at: lib/Sema/SemaDeclCXX.cpp:5989-5990
@@ +5988,4 @@
+
+  if (TemplateDecl *TD = dyn_cast<TemplateDecl>(D))
+    ParameterLists.push_back(TD->getTemplateParameters());
+
----------------
Maybe just assert in this case? People shouldn't be passing `TemplateDecl`s in here.

http://reviews.llvm.org/D3555