[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]

No objection to the current patch other than the `dyn_cast<Decl>` thing, but some thoughts.

================
Comment at: include/clang/Sema/Sema.h:4887-4888
@@ -4886,4 +4886,4 @@
   void ActOnReenterCXXMethodParameter(Scope *S, ParmVarDecl *Param);
-  void ActOnReenterTemplateScope(Scope *S, Decl *Template);
-  void ActOnReenterDeclaratorTemplateScope(Scope *S, DeclaratorDecl *D);
+  void ActOnReenterTemplateScope(Scope *S, Decl *Template,
+                                 unsigned *NumParamLists = 0);
   void ActOnStartDelayedMemberDeclarations(Scope *S, Decl *Record);
----------------
It's unusual to have an out parameter and a `void` return type. Have you considered returning the number of template parameter lists that were entered?

================
Comment at: lib/Parse/ParseTemplate.cpp:1232-1233
@@ -1231,4 +1231,4 @@
   FunctionDecl *FunD = LPT.D->getAsFunction();
   // Track template parameter depth.
   TemplateParameterDepthRAII CurTemplateDepthTracker(TemplateParameterDepth);
 
----------------
(Not really related to your patch, but...) Shouldn't we be resetting `TemplateParameterDepth` to 0 somewhere in here? If we decide to lazily parse a templated function definition while we're half-way through parsing another template, our depths will get messed up. (IIRC, we've turned off delayed parsing for constexpr functions and functions with deduced return types, perhaps due to this bug...)

================
Comment at: lib/Parse/ParseTemplate.cpp:1252
@@ -1251,16 +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;
+    if (Decl *D = dyn_cast<Decl>(*II)) {
+      TemplateParamScopeStack.push_back(new ParseScope(this,
----------------
No need for the `dyn_cast` here. Every `DeclContext` is a `Decl`.

================
Comment at: lib/Parse/ParseTemplate.cpp:1259-1267
@@ -1267,1 +1258,11 @@
+
+      if (CXXRecordDecl *RD = dyn_cast<CXXRecordDecl>(D)) {
+        if (ClassTemplateDecl *CTD = RD->getDescribedClassTemplate()) {
+          TemplateParamScopeStack.push_back(new ParseScope(this,
+                Scope::TemplateParamScope));
+          unsigned NumParamLists = 0;
+          Actions.ActOnReenterTemplateScope(getCurScope(), CTD, &NumParamLists);
+          CurTemplateDepthTracker.addDepth(NumParamLists);
+        }
+      }
     }
----------------
Could `ActOnReenterTemplateScope` do this itself? (That is, entering a templated decl could also enter its template, and thus enter all the template parameter lists on the same logical declaration.)

http://reviews.llvm.org/D3555