DR458: Search template parameter scopes in the right order.

C++ unqualified name lookup searches template parameter scopes
immediately after finishing searching the entity the parameters belong
to. (Eg, for a class template, you search the template parameter scope
after looking in that class template and its base classes and before
looking in the scope containing the class template.) This is complicated
by the fact that scope lookup within a template parameter scope looks in
a different sequence of places prior to reaching the end of the
declarator-id in the template declaration.

We used to approximate the proper lookup rule with a hack in the scope /
decl context walk inside name lookup. Now we instead compute the lookup
parent for each template parameter scope.

In order to get this right, we now make sure to enter a distinct Scope
for each template parameter scope, and make sure to re-enter the
enclosing class scopes properly when handling delay-parsed regions
within a class.

Author: zygoloid

clang/include/clang/Parse/Parser.h

@@ -1088,12 +1088,40 @@ class Parser : public CodeCompletionHandler {
     }
   };
 
+  /// Introduces zero or more scopes for parsing. The scopes will all be exited
+  /// when the object is destroyed.
+  class MultiParseScope {
+    Parser &Self;
+    unsigned NumScopes = 0;
+
+    MultiParseScope(const MultiParseScope&) = delete;
+
+  public:
+    MultiParseScope(Parser &Self) : Self(Self) {}
+    void Enter(unsigned ScopeFlags) {
+      Self.EnterScope(ScopeFlags);
+      ++NumScopes;
+    }
+    void Exit() {
+      while (NumScopes) {
+        Self.ExitScope();
+        --NumScopes;
+      }
+    }
+    ~MultiParseScope() {
+      Exit();
+    }
+  };
+
   /// EnterScope - Start a new scope.
   void EnterScope(unsigned ScopeFlags);
 
   /// ExitScope - Pop a scope off the scope stack.
   void ExitScope();
 
+  /// Re-enter the template scopes for a declaration that might be a template.
+  unsigned ReenterTemplateScopes(MultiParseScope &S, Decl *D);
+
 private:
   /// RAII object used to modify the scope flags for the current scope.
   class ParseScopeFlags {
@@ -1278,13 +1306,7 @@ class Parser : public CodeCompletionHandler {
     Decl *D;
     CachedTokens Toks;
 
-    /// Whether this member function had an associated template
-    /// scope. When true, D is a template declaration.
-    /// otherwise, it is a member function declaration.
-    bool TemplateScope;
-
-    explicit LexedMethod(Parser* P, Decl *MD)
-      : Self(P), D(MD), TemplateScope(false) {}
+    explicit LexedMethod(Parser *P, Decl *MD) : Self(P), D(MD) {}
 
     void ParseLexedMethodDefs() override;
   };
@@ -1314,8 +1336,7 @@ class Parser : public CodeCompletionHandler {
   /// argument (C++ [class.mem]p2).
   struct LateParsedMethodDeclaration : public LateParsedDeclaration {
     explicit LateParsedMethodDeclaration(Parser *P, Decl *M)
-      : Self(P), Method(M), TemplateScope(false),
-        ExceptionSpecTokens(nullptr) {}
+        : Self(P), Method(M), ExceptionSpecTokens(nullptr) {}
 
     void ParseLexedMethodDeclarations() override;
 
@@ -1324,11 +1345,6 @@ class Parser : public CodeCompletionHandler {
     /// Method - The method declaration.
     Decl *Method;
 
-    /// Whether this member function had an associated template
-    /// scope. When true, D is a template declaration.
-    /// otherwise, it is a member function declaration.
-    bool TemplateScope;
-
     /// DefaultArgs - Contains the parameters of the function and
     /// their default arguments. At least one of the parameters will
     /// have a default argument, but all of the parameters of the
@@ -1373,18 +1389,13 @@ class Parser : public CodeCompletionHandler {
   /// parsed after the corresponding top-level class is complete.
   struct ParsingClass {
     ParsingClass(Decl *TagOrTemplate, bool TopLevelClass, bool IsInterface)
-      : TopLevelClass(TopLevelClass), TemplateScope(false),
-        IsInterface(IsInterface), TagOrTemplate(TagOrTemplate) { }
+        : TopLevelClass(TopLevelClass), IsInterface(IsInterface),
+          TagOrTemplate(TagOrTemplate) {}
 
     /// Whether this is a "top-level" class, meaning that it is
     /// not nested within another class.
     bool TopLevelClass : 1;
 
-    /// Whether this class had an associated template
-    /// scope. When true, TagOrTemplate is a template declaration;
-    /// otherwise, it is a tag declaration.
-    bool TemplateScope : 1;
-
     /// Whether this class is an __interface.
     bool IsInterface : 1;
 
@@ -1483,6 +1494,10 @@ class Parser : public CodeCompletionHandler {
     SourceRange getSourceRange() const LLVM_READONLY;
   };
 
+  // In ParseCXXInlineMethods.cpp.
+  struct ReenterTemplateScopeRAII;
+  struct ReenterClassScopeRAII;
+
   void LexTemplateFunctionForLateParsing(CachedTokens &Toks);
   void ParseLateTemplatedFuncDef(LateParsedTemplate &LPT);
 
@@ -3240,7 +3255,7 @@ class Parser : public CodeCompletionHandler {
       DeclaratorContext Context, const ParsedTemplateInfo &TemplateInfo,
       ParsingDeclRAIIObject &DiagsFromParams, SourceLocation &DeclEnd,
       ParsedAttributes &AccessAttrs, AccessSpecifier AS = AS_none);
-  bool ParseTemplateParameters(unsigned Depth,
+  bool ParseTemplateParameters(MultiParseScope &TemplateScopes, unsigned Depth,
                                SmallVectorImpl<NamedDecl *> &TemplateParams,
                                SourceLocation &LAngleLoc,
                                SourceLocation &RAngleLoc);

clang/include/clang/Sema/Scope.h

@@ -322,8 +322,21 @@ class Scope {
   /// declared in.
   bool isDeclScope(const Decl *D) const { return DeclsInScope.count(D) != 0; }
 
-  DeclContext *getEntity() const { return Entity; }
-  void setEntity(DeclContext *E) { Entity = E; }
+  /// Get the entity corresponding to this scope.
+  DeclContext *getEntity() const {
+    return isTemplateParamScope() ? nullptr : Entity;
+  }
+
+  /// Get the DeclContext in which to continue unqualified lookup after a
+  /// lookup in this scope.
+  DeclContext *getLookupEntity() const { return Entity; }
+
+  void setEntity(DeclContext *E) {
+    assert(!isTemplateParamScope() &&
+           "entity associated with template param scope");
+    Entity = E;
+  }
+  void setLookupEntity(DeclContext *E) { Entity = E; }
 
   /// Determine whether any unrecoverable errors have occurred within this
   /// scope. Note that this may return false even if the scope contains invalid

clang/include/clang/Sema/Sema.h

@@ -2866,8 +2866,6 @@ class Sema final {
                      Decl *EnumDecl, ArrayRef<Decl *> Elements, Scope *S,
                      const ParsedAttributesView &Attr);
 
-  DeclContext *getContainingDC(DeclContext *DC);
-
   /// Set the current declaration context until it gets popped.
   void PushDeclContext(Scope *S, DeclContext *DC);
   void PopDeclContext();
@@ -2877,6 +2875,11 @@ class Sema final {
   void EnterDeclaratorContext(Scope *S, DeclContext *DC);
   void ExitDeclaratorContext(Scope *S);
 
+  /// Enter a template parameter scope, after it's been associated with a particular
+  /// DeclContext. Causes lookup within the scope to chain through enclosing contexts
+  /// in the correct order.
+  void EnterTemplatedContext(Scope *S, DeclContext *DC);
+
   /// Push the parameters of D, which must be a function, into scope.
   void ActOnReenterFunctionContext(Scope* S, Decl* D);
   void ActOnExitFunctionContext();
@@ -6789,7 +6792,8 @@ class Sema final {
   void ActOnFinishCXXNonNestedClass();
 
   void ActOnReenterCXXMethodParameter(Scope *S, ParmVarDecl *Param);
-  unsigned ActOnReenterTemplateScope(Scope *S, Decl *Template);
+  unsigned ActOnReenterTemplateScope(Decl *Template,
+                                     llvm::function_ref<Scope *()> EnterScope);
   void ActOnStartDelayedMemberDeclarations(Scope *S, Decl *Record);
   void ActOnStartDelayedCXXMethodDeclaration(Scope *S, Decl *Method);
   void ActOnDelayedCXXMethodParameter(Scope *S, Decl *Param);

clang/lib/AST/DeclBase.cpp

@@ -251,7 +251,7 @@ const TemplateParameterList *Decl::getDescribedTemplateParams() const {
 }
 
 bool Decl::isTemplated() const {
-  // A declaration is dependent if it is a template or a template pattern, or
+  // A declaration is templated if it is a template or a template pattern, or
   // is within (lexcially for a friend, semantically otherwise) a dependent
   // context.
   // FIXME: Should local extern declarations be treated like friends?