When parsing an out-of-line member function declaration, we must delay

access-control diagnostics which arise from the portion of the declarator
following the scope specifier, just in case access is granted by
friending the individual method.  This can also happen with in-line
member function declarations of class templates due to templated-scope
friend declarations.

We were really playing fast-and-loose before with this sort of thing,
and it turned out to work because *most* friend functions are in file
scope.  Making us delay regardless of context exposed several bugs with
how we were manipulating delay.  I ended up needing a concept of a
context that's independent of the declarations in which it appears,
and then I actually had to make some things save contexts correctly,
but delay should be much cleaner now.

I also encapsulated all the delayed-diagnostics machinery in a single
subobject of Sema;  this is a pattern we might want to consider rolling
out to other components of Sema.

llvm-svn: 125485

8 files changed, 79 insertions, 49 deletions

diff --git a/clang/lib/Parse/ParseCXXInlineMethods.cpp b/clang/lib/Parse/ParseCXXInlineMethods.cpp
index d62e71836fa..399473840a9 100644
--- a/clang/lib/Parse/ParseCXXInlineMethods.cpp
+++ b/clang/lib/Parse/ParseCXXInlineMethods.cpp
@@ -20,7 +20,7 @@ using namespace clang;
 /// ParseCXXInlineMethodDef - We parsed and verified that the specified
 /// Declarator is a well formed C++ inline method definition. Now lex its body
 /// and store its tokens for parsing after the C++ class is complete.
-Decl *Parser::ParseCXXInlineMethodDef(AccessSpecifier AS, Declarator &D,
+Decl *Parser::ParseCXXInlineMethodDef(AccessSpecifier AS, ParsingDeclarator &D,
                                 const ParsedTemplateInfo &TemplateInfo,
                                 const VirtSpecifiers& VS) {
   assert(D.isFunctionDeclarator() && "This isn't a function declarator!");
@@ -51,6 +51,8 @@ Decl *Parser::ParseCXXInlineMethodDef(AccessSpecifier AS, Declarator &D,
 
   HandleMemberFunctionDefaultArgs(D, FnD);
 
+  D.complete(FnD);
+
   // Consume the tokens and store them for later parsing.
 
   LexedMethod* LM = new LexedMethod(this, FnD);
diff --git a/clang/lib/Parse/ParseDeclCXX.cpp b/clang/lib/Parse/ParseDeclCXX.cpp
index 9466ebc844b..b3ad25b024f 100644
--- a/clang/lib/Parse/ParseDeclCXX.cpp
+++ b/clang/lib/Parse/ParseDeclCXX.cpp
@@ -2087,10 +2087,12 @@ TypeResult Parser::ParseTrailingReturnType() {
 /// \brief We have just started parsing the definition of a new class,
 /// so push that class onto our stack of classes that is currently
 /// being parsed.
-void Parser::PushParsingClass(Decl *ClassDecl, bool NonNestedClass) {
+Sema::ParsingClassState
+Parser::PushParsingClass(Decl *ClassDecl, bool NonNestedClass) {
   assert((NonNestedClass || !ClassStack.empty()) &&
          "Nested class without outer class");
   ClassStack.push(new ParsingClass(ClassDecl, NonNestedClass));
+  return Actions.PushParsingClass();
 }
 
 /// \brief Deallocate the given parsed class and all of its nested
@@ -2110,9 +2112,11 @@ void Parser::DeallocateParsedClasses(Parser::ParsingClass *Class) {
 ///
 /// \returns true if the class we've popped is a top-level class,
 /// false otherwise.
-void Parser::PopParsingClass() {
+void Parser::PopParsingClass(Sema::ParsingClassState state) {
   assert(!ClassStack.empty() && "Mismatched push/pop for class parsing");
 
+  Actions.PopParsingClass(state);
+
   ParsingClass *Victim = ClassStack.top();
   ClassStack.pop();
   if (Victim->TopLevelClass) {
diff --git a/clang/lib/Parse/ParseTemplate.cpp b/clang/lib/Parse/ParseTemplate.cpp
index e64a933dec1..8387c881952 100644
--- a/clang/lib/Parse/ParseTemplate.cpp
+++ b/clang/lib/Parse/ParseTemplate.cpp
@@ -246,7 +246,7 @@ Parser::ParseSingleDeclarationAfterTemplate(
 
     // Eat the semi colon after the declaration.
     ExpectAndConsume(tok::semi, diag::err_expected_semi_declaration);
-    DS.complete(ThisDecl);
+    DeclaratorInfo.complete(ThisDecl);
     return ThisDecl;
   }
 
diff --git a/clang/lib/Sema/Sema.cpp b/clang/lib/Sema/Sema.cpp
index a1ad78418f4..8fbbeb85e3e 100644
--- a/clang/lib/Sema/Sema.cpp
+++ b/clang/lib/Sema/Sema.cpp
@@ -135,7 +135,7 @@ Sema::Sema(Preprocessor &pp, ASTContext &ctxt, ASTConsumer &consumer,
     LangOpts(pp.getLangOptions()), PP(pp), Context(ctxt), Consumer(consumer),
     Diags(PP.getDiagnostics()), SourceMgr(PP.getSourceManager()),
     ExternalSource(0), CodeCompleter(CodeCompleter), CurContext(0), 
-    PackContext(0), VisContext(0), ParsingDeclDepth(0),
+    PackContext(0), VisContext(0),
     IdResolver(pp.getLangOptions()), CXXTypeInfoDecl(0), MSVCGuidDecl(0),
     GlobalNewDeleteDeclared(false), 
     CompleteTranslationUnit(CompleteTranslationUnit),
diff --git a/clang/lib/Sema/SemaAccess.cpp b/clang/lib/Sema/SemaAccess.cpp
index 605baf9bada..3103255d200 100644
--- a/clang/lib/Sema/SemaAccess.cpp
+++ b/clang/lib/Sema/SemaAccess.cpp
@@ -1260,13 +1260,19 @@ static Sema::AccessResult CheckAccess(Sema &S, SourceLocation Loc,
   if (S.SuppressAccessChecking)
     return Sema::AR_accessible;
 
-  // If we're currently parsing a top-level declaration, delay
-  // diagnostics.  This is the only case where parsing a declaration
-  // can actually change our effective context for the purposes of
-  // access control.
-  if (S.CurContext->isFileContext() && S.ParsingDeclDepth) {
-    S.DelayedDiagnostics.push_back(
-        DelayedDiagnostic::makeAccess(Loc, Entity));
+  // If we're currently parsing a declaration, we may need to delay
+  // access control checking, because our effective context might be
+  // different based on what the declaration comes out as.
+  //
+  // For example, we might be parsing a declaration with a scope
+  // specifier, like this:
+  //   A::private_type A::foo() { ... }
+  //
+  // Or we might be parsing something that will turn out to be a friend:
+  //   void foo(A::private_type);
+  //   void B::foo(A::private_type);
+  if (S.DelayedDiagnostics.shouldDelayDiagnostics()) {
+    S.DelayedDiagnostics.add(DelayedDiagnostic::makeAccess(Loc, Entity));
     return Sema::AR_delayed;
   }
 
diff --git a/clang/lib/Sema/SemaDeclAttr.cpp b/clang/lib/Sema/SemaDeclAttr.cpp
index 5d5093f5fe4..b0636bcb96f 100644
--- a/clang/lib/Sema/SemaDeclAttr.cpp
+++ b/clang/lib/Sema/SemaDeclAttr.cpp
@@ -2923,59 +2923,77 @@ void Sema::ProcessDeclAttributes(Scope *S, Decl *D, const Declarator &PD,
     ProcessDeclAttributeList(S, D, Attrs, NonInheritable, Inheritable);
 }
 
-/// PushParsingDeclaration - Enter a new "scope" of deprecation
-/// warnings.
-///
-/// The state token we use is the start index of this scope
-/// on the warning stack.
-Sema::ParsingDeclStackState Sema::PushParsingDeclaration() {
-  ParsingDeclDepth++;
-  return (ParsingDeclStackState) DelayedDiagnostics.size();
+// This duplicates a vector push_back but hides the need to know the
+// size of the type.
+void Sema::DelayedDiagnostics::add(const DelayedDiagnostic &diag) {
+  assert(StackSize <= StackCapacity);
+
+  // Grow the stack if necessary.
+  if (StackSize == StackCapacity) {
+    unsigned newCapacity = 2 * StackCapacity + 2;
+    char *newBuffer = new char[newCapacity * sizeof(DelayedDiagnostic)];
+    const char *oldBuffer = (const char*) Stack;
+
+    if (StackCapacity)
+      memcpy(newBuffer, oldBuffer, StackCapacity * sizeof(DelayedDiagnostic));
+    
+    delete[] oldBuffer;
+    Stack = reinterpret_cast<sema::DelayedDiagnostic*>(newBuffer);
+    StackCapacity = newCapacity;
+  }
+
+  assert(StackSize < StackCapacity);
+  new (&Stack[StackSize++]) DelayedDiagnostic(diag);
 }
 
-void Sema::PopParsingDeclaration(ParsingDeclStackState S, Decl *D) {
-  assert(ParsingDeclDepth > 0 && "empty ParsingDeclaration stack");
-  ParsingDeclDepth--;
+void Sema::DelayedDiagnostics::popParsingDecl(Sema &S, ParsingDeclState state,
+                                              Decl *decl) {
+  DelayedDiagnostics &DD = S.DelayedDiagnostics;
 
-  if (DelayedDiagnostics.empty())
-    return;
+  // Check the invariants.
+  assert(DD.StackSize >= state.SavedStackSize);
+  assert(state.SavedStackSize >= DD.ActiveStackBase);
+  assert(DD.ParsingDepth > 0);
 
-  unsigned SavedIndex = (unsigned) S;
-  assert(SavedIndex <= DelayedDiagnostics.size() &&
-         "saved index is out of bounds");
+  // Drop the parsing depth.
+  DD.ParsingDepth--;
 
-  unsigned E = DelayedDiagnostics.size();
+  // If there are no active diagnostics, we're done.
+  if (DD.StackSize == DD.ActiveStackBase)
+    return;
 
   // We only want to actually emit delayed diagnostics when we
   // successfully parsed a decl.
-  if (D) {
-    // We really do want to start with 0 here.  We get one push for a
+  if (decl) {
+    // We emit all the active diagnostics, not just those starting
+    // from the saved state.  The idea is this:  we get one push for a
     // decl spec and another for each declarator;  in a decl group like:
     //   deprecated_typedef foo, *bar, baz();
     // only the declarator pops will be passed decls.  This is correct;
     // we really do need to consider delayed diagnostics from the decl spec
     // for each of the different declarations.
-    for (unsigned I = 0; I != E; ++I) {
-      if (DelayedDiagnostics[I].Triggered)
+    for (unsigned i = DD.ActiveStackBase, e = DD.StackSize; i != e; ++i) {
+      DelayedDiagnostic &diag = DD.Stack[i];
+      if (diag.Triggered)
         continue;
 
-      switch (DelayedDiagnostics[I].Kind) {
+      switch (diag.Kind) {
       case DelayedDiagnostic::Deprecation:
-        HandleDelayedDeprecationCheck(DelayedDiagnostics[I], D);
+        S.HandleDelayedDeprecationCheck(diag, decl);
         break;
 
       case DelayedDiagnostic::Access:
-        HandleDelayedAccessCheck(DelayedDiagnostics[I], D);
+        S.HandleDelayedAccessCheck(diag, decl);
         break;
       }
     }
   }
 
   // Destroy all the delayed diagnostics we're about to pop off.
-  for (unsigned I = SavedIndex; I != E; ++I)
-    DelayedDiagnostics[I].destroy();
+  for (unsigned i = state.SavedStackSize, e = DD.StackSize; i != e; ++i)
+    DD.Stack[i].destroy();
 
-  DelayedDiagnostics.set_size(SavedIndex);
+  DD.StackSize = state.SavedStackSize;
 }
 
 static bool isDeclDeprecated(Decl *D) {
@@ -3005,9 +3023,8 @@ void Sema::EmitDeprecationWarning(NamedDecl *D, llvm::StringRef Message,
                                   SourceLocation Loc,
                                   bool UnknownObjCClass) {
   // Delay if we're currently parsing a declaration.
-  if (ParsingDeclDepth) {
-    DelayedDiagnostics.push_back(DelayedDiagnostic::makeDeprecation(Loc, D, 
-                                                                    Message));
+  if (DelayedDiagnostics.shouldDelayDiagnostics()) {
+    DelayedDiagnostics.add(DelayedDiagnostic::makeDeprecation(Loc, D, Message));
     return;
   }
 
diff --git a/clang/lib/Sema/SemaDeclCXX.cpp b/clang/lib/Sema/SemaDeclCXX.cpp
index cc3a02fb2aa..370def568e2 100644
--- a/clang/lib/Sema/SemaDeclCXX.cpp
+++ b/clang/lib/Sema/SemaDeclCXX.cpp
@@ -4556,13 +4556,12 @@ namespace {
   /// to implicitly define the body of a C++ member function;
   class ImplicitlyDefinedFunctionScope {
     Sema &S;
-    DeclContext *PreviousContext;
+    Sema::ContextRAII SavedContext;
     
   public:
     ImplicitlyDefinedFunctionScope(Sema &S, CXXMethodDecl *Method)
-      : S(S), PreviousContext(S.CurContext) 
+      : S(S), SavedContext(S, Method) 
     {
-      S.CurContext = Method;
       S.PushFunctionScope();
       S.PushExpressionEvaluationContext(Sema::PotentiallyEvaluated);
     }
@@ -4570,7 +4569,6 @@ namespace {
     ~ImplicitlyDefinedFunctionScope() {
       S.PopExpressionEvaluationContext();
       S.PopFunctionOrBlockScope();
-      S.CurContext = PreviousContext;
     }
   };
 }
@@ -7281,6 +7279,10 @@ bool Sema::CheckOverridingFunctionReturnType(const CXXMethodDecl *New,
                     diag::err_covariant_return_ambiguous_derived_to_base_conv,
                     // FIXME: Should this point to the return type?
                     New->getLocation(), SourceRange(), New->getDeclName(), 0)) {
+      // FIXME: this note won't trigger for delayed access control
+      // diagnostics, and it's impossible to get an undelayed error
+      // here from access control during the original parse because
+      // the ParsingDeclSpec/ParsingDeclarator are still in scope.
       Diag(Old->getLocation(), diag::note_overridden_virtual_function);
       return true;
     }
diff --git a/clang/lib/Sema/SemaTemplateInstantiateDecl.cpp b/clang/lib/Sema/SemaTemplateInstantiateDecl.cpp
index 73b01271b21..3475cc142db 100644
--- a/clang/lib/Sema/SemaTemplateInstantiateDecl.cpp
+++ b/clang/lib/Sema/SemaTemplateInstantiateDecl.cpp
@@ -2310,8 +2310,7 @@ void Sema::InstantiateFunctionDefinition(SourceLocation PointOfInstantiation,
 
   // Enter the scope of this instantiation. We don't use
   // PushDeclContext because we don't have a scope.
-  DeclContext *PreviousContext = CurContext;
-  CurContext = Function;
+  Sema::ContextRAII savedContext(*this, Function);
 
   MultiLevelTemplateArgumentList TemplateArgs =
     getTemplateInstantiationArgs(Function, 0, false, PatternDecl);
@@ -2334,7 +2333,7 @@ void Sema::InstantiateFunctionDefinition(SourceLocation PointOfInstantiation,
 
   PerformDependentDiagnostics(PatternDecl, TemplateArgs);
 
-  CurContext = PreviousContext;
+  savedContext.pop();
 
   DeclGroupRef DG(Function);
   Consumer.HandleTopLevelDecl(DG);