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

[Hans Wennborg]

================
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);
----------------
Richard Smith wrote:
> 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?
Oops, yeah that makes a lot of sense. Done.

================
Comment at: lib/Parse/ParseTemplate.cpp:1232-1233
@@ -1231,4 +1231,4 @@
   FunctionDecl *FunD = LPT.D->getAsFunction();
   // Track template parameter depth.
   TemplateParameterDepthRAII CurTemplateDepthTracker(TemplateParameterDepth);
 
----------------
Richard Smith wrote:
> (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...)
I don't have a deep enough understanding of this to answer, but I do know that we don't have any tests that fail if I reset TemplateParameterDepth to 0 at the end of this function..

================
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,
----------------
Richard Smith wrote:
> No need for the `dyn_cast` here. Every `DeclContext` is a `Decl`.
Done. I got confused because DeclContext doesn't inherit from Decl, but I guess every subclass of DeclContext does.

================
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);
+        }
+      }
     }
----------------
Richard Smith wrote:
> 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.)
I failed some tests when I tried that before, but that seems to have been unrelated because it works now and makes things much nicer.

http://reviews.llvm.org/D3555