Change how we suppress access control in explicit instantiations

so that we actually accumulate all the delayed diagnostics.  Do
this so that we can restore those diagnostics to good standing
if it turns out that we were wrong to suppress, e.g. if the
tag specifier is actually an elaborated type specifier and not
a declaration.

git-svn-id: https://llvm.org/svn/llvm-project/cfe/trunk@156291 91177308-0d34-0410-b5e6-96231b3b80d8

include/clang/Sema/Sema.h

@@ -2592,6 +2592,8 @@ public:
     DelayedDiagnostics.popUndelayed(state);
   }
 
+  void redelayDiagnostics(sema::DelayedDiagnosticPool &pool);
+
   void EmitDeprecationWarning(NamedDecl *D, StringRef Message,
                               SourceLocation Loc,
                               const ObjCInterfaceDecl *UnknownObjCClass=0);
@@ -4395,45 +4397,10 @@ public:
 
   void HandleDelayedAccessCheck(sema::DelayedDiagnostic &DD, Decl *Ctx);
 
-  /// A flag to suppress access checking.
-  bool SuppressAccessChecking;
-
   /// \brief When true, access checking violations are treated as SFINAE
   /// failures rather than hard errors.
   bool AccessCheckingSFINAE;
 
-  /// \brief RAII object used to temporarily suppress access checking.
-  class SuppressAccessChecksRAII {
-    Sema &S;
-    bool SuppressingAccess;
-
-  public:
-    SuppressAccessChecksRAII(Sema &S, bool Suppress)
-      : S(S), SuppressingAccess(Suppress) {
-      if (Suppress) S.ActOnStartSuppressingAccessChecks();
-    }
-    ~SuppressAccessChecksRAII() {
-      done();
-    }
-    void done() {
-      if (!SuppressingAccess) return;
-      S.ActOnStopSuppressingAccessChecks();
-      SuppressingAccess = false;
-    }
-  };
-
-  void ActOnStartSuppressingAccessChecks() {
-    assert(!SuppressAccessChecking &&
-           "Tried to start access check suppression when already started.");
-    SuppressAccessChecking = true;
-  }
-
-  void ActOnStopSuppressingAccessChecks() {
-    assert(SuppressAccessChecking &&
-           "Tried to stop access check suprression when already stopped.");
-    SuppressAccessChecking = false;
-  }
-
   enum AbstractDiagSelID {
     AbstractNone = -1,
     AbstractReturnType,

lib/Parse/ParseDecl.cpp

@@ -2773,12 +2773,16 @@ void Parser::ParseEnumSpecifier(SourceLocation StartLoc, DeclSpec &DS,
     ScopedEnumKWLoc = ConsumeToken();
   }
 
-  // C++11 [temp.explicit]p12: The usual access controls do not apply to names
-  // used to specify explicit instantiations. We extend this to also cover
-  // explicit specializations.
-  Sema::SuppressAccessChecksRAII SuppressAccess(Actions,
-    TemplateInfo.Kind == ParsedTemplateInfo::ExplicitInstantiation ||
-    TemplateInfo.Kind == ParsedTemplateInfo::ExplicitSpecialization);
+  // C++11 [temp.explicit]p12:
+  //   The usual access controls do not apply to names used to specify
+  //   explicit instantiations.
+  // We extend this to also cover explicit specializations.  Note that
+  // we don't suppress if this turns out to be an elaborated type
+  // specifier.
+  bool shouldDelayDiagsInTag =
+    (TemplateInfo.Kind == ParsedTemplateInfo::ExplicitInstantiation ||
+     TemplateInfo.Kind == ParsedTemplateInfo::ExplicitSpecialization);
+  SuppressAccessChecks diagsFromTag(*this, shouldDelayDiagsInTag);
 
   // If attributes exist after tag, parse them.
   ParsedAttributes attrs(AttrFactory);
@@ -2842,8 +2846,10 @@ void Parser::ParseEnumSpecifier(SourceLocation StartLoc, DeclSpec &DS,
     IsScopedUsingClassTag = false;
   }
 
-  // Stop suppressing access control now we've parsed the enum name.
-  SuppressAccess.done();
+  // Okay, end the suppression area.  We'll decide whether to emit the
+  // diagnostics in a second.
+  if (shouldDelayDiagsInTag)
+    diagsFromTag.done();
 
   TypeResult BaseType;
 
@@ -2925,16 +2931,29 @@ void Parser::ParseEnumSpecifier(SourceLocation StartLoc, DeclSpec &DS,
   // enum foo {..};  void bar() { enum foo x; }  <- use of old foo.
   //
   Sema::TagUseKind TUK;
-  if (DS.isFriendSpecified())
-    TUK = Sema::TUK_Friend;
-  else if (!AllowDeclaration)
+  if (!AllowDeclaration) {
     TUK = Sema::TUK_Reference;
-  else if (Tok.is(tok::l_brace))
-    TUK = Sema::TUK_Definition;
-  else if (Tok.is(tok::semi) && DSC != DSC_type_specifier)
-    TUK = Sema::TUK_Declaration;
-  else
+  } else if (Tok.is(tok::l_brace)) {
+    if (DS.isFriendSpecified()) {
+      Diag(Tok.getLocation(), diag::err_friend_decl_defines_type)
+        << SourceRange(DS.getFriendSpecLoc());
+      ConsumeBrace();
+      SkipUntil(tok::r_brace);
+      TUK = Sema::TUK_Friend;
+    } else {
+      TUK = Sema::TUK_Definition;
+    }
+  } else if (Tok.is(tok::semi) && DSC != DSC_type_specifier) {
+    TUK = (DS.isFriendSpecified() ? Sema::TUK_Friend : Sema::TUK_Declaration);
+  } else {
     TUK = Sema::TUK_Reference;
+  }
+
+  // If this is an elaborated type specifier, and we delayed
+  // diagnostics before, just merge them into the current pool.
+  if (TUK == Sema::TUK_Reference && shouldDelayDiagsInTag) {
+    diagsFromTag.redelay();
+  }
 
   MultiTemplateParamsArg TParams;
   if (TemplateInfo.Kind != ParsedTemplateInfo::NonTemplate &&
@@ -3014,14 +3033,7 @@ void Parser::ParseEnumSpecifier(SourceLocation StartLoc, DeclSpec &DS,
   }
 
   if (Tok.is(tok::l_brace) && TUK != Sema::TUK_Reference) {
-    if (TUK == Sema::TUK_Friend) {
-      Diag(Tok, diag::err_friend_decl_defines_type)
-        << SourceRange(DS.getFriendSpecLoc());
-      ConsumeBrace();
-      SkipUntil(tok::r_brace);
-    } else {
-      ParseEnumBody(StartLoc, TagDecl);
-    }
+    ParseEnumBody(StartLoc, TagDecl);
   }
 
   if (DS.SetTypeSpecType(DeclSpec::TST_enum, StartLoc,

lib/Parse/ParseDeclCXX.cpp

@@ -968,9 +968,13 @@ void Parser::ParseClassSpecifier(tok::TokenKind TagTokKind,
   // As an extension we do not perform access checking on the names used to
   // specify explicit specializations either. This is important to allow
   // specializing traits classes for private types.
-  Sema::SuppressAccessChecksRAII SuppressAccess(Actions,
-    TemplateInfo.Kind == ParsedTemplateInfo::ExplicitInstantiation ||
-    TemplateInfo.Kind == ParsedTemplateInfo::ExplicitSpecialization);
+  //
+  // Note that we don't suppress if this turns out to be an elaborated
+  // type specifier.
+  bool shouldDelayDiagsInTag =
+    (TemplateInfo.Kind == ParsedTemplateInfo::ExplicitInstantiation ||
+     TemplateInfo.Kind == ParsedTemplateInfo::ExplicitSpecialization);
+  SuppressAccessChecks diagsFromTag(*this, shouldDelayDiagsInTag);
 
   ParsedAttributes attrs(AttrFactory);
   // If attributes exist after tag, parse them.
@@ -1103,10 +1107,6 @@ void Parser::ParseClassSpecifier(tok::TokenKind TagTokKind,
     }
   }
 
-  // As soon as we're finished parsing the class's template-id, turn access
-  // checking back on.
-  SuppressAccess.done();
-
   // There are four options here.
   //  - If we are in a trailing return type, this is always just a reference,
   //    and we must not try to parse a definition. For instance,
@@ -1149,6 +1149,14 @@ void Parser::ParseClassSpecifier(tok::TokenKind TagTokKind,
   else
     TUK = Sema::TUK_Reference;
 
+  // If this is an elaborated type specifier, and we delayed
+  // diagnostics before, just merge them into the current pool.
+  if (shouldDelayDiagsInTag) {
+    diagsFromTag.done();
+    if (TUK == Sema::TUK_Reference)
+      diagsFromTag.redelay();
+  }
+
   if (!Name && !TemplateId && (DS.getTypeSpecType() == DeclSpec::TST_error ||
                                TUK != Sema::TUK_Definition)) {
     if (DS.getTypeSpecType() != DeclSpec::TST_error) {

lib/Parse/RAIIObjectsForParser.h

@@ -24,6 +24,59 @@ namespace clang {
   // TODO: move ParsingClassDefinition here.
   // TODO: move TentativeParsingAction here.
 
+  /// \brief A RAII object used to temporarily suppress access-like
+  /// checking.  Access-like checks are those associated with
+  /// controlling the use of a declaration, like C++ access control
+  /// errors and deprecation warnings.  They are contextually
+  /// dependent, in that they can only be resolved with full
+  /// information about what's being declared.  They are also
+  /// suppressed in certain contexts, like the template arguments of
+  /// an explicit instantiation.  However, those suppression contexts
+  /// cannot necessarily be fully determined in advance;  for
+  /// example, something starting like this:
+  ///   template <> class std::vector<A::PrivateType>
+  /// might be the entirety of an explicit instantiation:
+  ///   template <> class std::vector<A::PrivateType>;
+  /// or just an elaborated type specifier:
+  ///   template <> class std::vector<A::PrivateType> make_vector<>();
+  /// Therefore this class collects all the diagnostics and permits
+  /// them to be re-delayed in a new context.
+  class SuppressAccessChecks {
+    Sema &S;
+    sema::DelayedDiagnosticPool DiagnosticPool;
+    Sema::ParsingDeclState State;
+    bool Active;
+
+  public:
+    /// Begin suppressing access-like checks 
+    SuppressAccessChecks(Parser &P, bool activate = true)
+        : S(P.getActions()), DiagnosticPool(NULL) {
+      if (activate) {
+        State = S.PushParsingDeclaration(DiagnosticPool);
+        Active = true;
+      } else {
+        Active = false;
+      }
+    }
+
+    void done() {
+      assert(Active && "trying to end an inactive suppression");
+      S.PopParsingDeclaration(State, NULL);
+      Active = false;
+    }
+
+    void redelay() {
+      assert(!Active && "redelaying without having ended first");
+      if (!DiagnosticPool.pool_empty())
+        S.redelayDiagnostics(DiagnosticPool);
+      assert(DiagnosticPool.pool_empty());
+    }
+
+    ~SuppressAccessChecks() {
+      if (Active) done();
+    }
+  };
+
   /// \brief RAII object used to inform the actions that we're
   /// currently parsing a declaration.  This is active when parsing a
   /// variable's initializer, but not when parsing the body of a
@@ -93,14 +146,13 @@ namespace clang {
       pop(D);
     }
 
-  private:
-    void steal(ParsingDeclRAIIObject &Other) {
-      DiagnosticPool.steal(Other.DiagnosticPool);
-      State = Other.State;
-      Popped = Other.Popped;
-      Other.Popped = true;
+    /// Unregister this object from Sema, but remember all the
+    /// diagnostics that were emitted into it.
+    void abortAndRemember() {
+      pop(0);
     }
 
+  private:
     void push() {
       State = Actions.PushParsingDeclaration(DiagnosticPool);
       Popped = false;

lib/Sema/Sema.cpp

@@ -99,7 +99,7 @@ Sema::Sema(Preprocessor &pp, ASTContext &ctxt, ASTConsumer &consumer,
     ObjCShouldCallSuperDealloc(false),
     ObjCShouldCallSuperFinalize(false),
     TUKind(TUKind),
-    NumSFINAEErrors(0), InFunctionDeclarator(0), SuppressAccessChecking(false), 
+    NumSFINAEErrors(0), InFunctionDeclarator(0),
     AccessCheckingSFINAE(false), InNonInstantiationSFINAEContext(false),
     NonInstantiationEntries(0), ArgumentPackSubstitutionIndex(-1),
     CurrentInstantiationScope(0), TyposCorrected(0),

lib/Sema/SemaAccess.cpp

@@ -1391,9 +1391,6 @@ static Sema::AccessResult CheckAccess(Sema &S, SourceLocation Loc,
   if (Entity.getAccess() == AS_public)
     return Sema::AR_accessible;
 
-  if (S.SuppressAccessChecking)
-    return Sema::AR_accessible;
-
   // 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.

lib/Sema/SemaDeclAttr.cpp

@@ -4200,8 +4200,7 @@ static void handleDelayedForbiddenType(Sema &S, DelayedDiagnostic &diag,
 
 void Sema::PopParsingDeclaration(ParsingDeclState state, Decl *decl) {
   assert(DelayedDiagnostics.getCurrentPool());
-  sema::DelayedDiagnosticPool &poppedPool =
-    *DelayedDiagnostics.getCurrentPool();
+  DelayedDiagnosticPool &poppedPool = *DelayedDiagnostics.getCurrentPool();
   DelayedDiagnostics.popWithoutEmitting(state);
 
   // When delaying diagnostics to run in the context of a parsed
@@ -4216,9 +4215,9 @@ void Sema::PopParsingDeclaration(ParsingDeclState state, Decl *decl) {
   // 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.
-  const sema::DelayedDiagnosticPool *pool = &poppedPool;
+  const DelayedDiagnosticPool *pool = &poppedPool;
   do {
-    for (sema::DelayedDiagnosticPool::pool_iterator
+    for (DelayedDiagnosticPool::pool_iterator
            i = pool->pool_begin(), e = pool->pool_end(); i != e; ++i) {
       // This const_cast is a bit lame.  Really, Triggered should be mutable.
       DelayedDiagnostic &diag = const_cast<DelayedDiagnostic&>(*i);
@@ -4244,6 +4243,15 @@ void Sema::PopParsingDeclaration(ParsingDeclState state, Decl *decl) {
   } while ((pool = pool->getParent()));
 }
 
+/// Given a set of delayed diagnostics, re-emit them as if they had
+/// been delayed in the current context instead of in the given pool.
+/// Essentially, this just moves them to the current pool.
+void Sema::redelayDiagnostics(DelayedDiagnosticPool &pool) {
+  DelayedDiagnosticPool *curPool = DelayedDiagnostics.getCurrentPool();
+  assert(curPool && "re-emitting in undelayed context not supported");
+  curPool->steal(pool);
+}
+
 static bool isDeclDeprecated(Decl *D) {
   do {
     if (D->isDeprecated())

test/CXX/temp/temp.spec/temp.explicit/p12.cpp

@@ -1,6 +1,49 @@
 // RUN: %clang_cc1 -fsyntax-only -verify %s
 
-char* p = 0; 
-template<class T> T g(T x = &p) { return x; }
-template int g<int>(int);	// OK even though &p isn't an int.
+namespace test0 {
+  char* p = 0; 
+  template<class T> T g(T x = &p) { return x; }
+  template int g<int>(int);	// OK even though &p isn't an int.
+}
 
+// Don't impose access restrictions on explicit instantiations.
+namespace test1 {
+  class A {
+    class Private {};
+  public:
+    typedef Private Public;
+  };
+
+  template <class T> class Temp {
+    static Temp<A::Public> make() { return Temp<A::Public>(); }
+  };
+  template class Temp<A::Private>;
+
+  // FIXME: this ought to be an error, but it isn't because Sema is
+  // silently failing to create a declaration for the explicit
+  // instantiation.
+  template class Temp<A::Private> Temp<int>::make();
+}
+
+// Don't impose access restrictions on explicit specializations,
+// either.  This goes here because it's an extension of the rule for
+// explicit instantiations and doesn't have any independent support.
+namespace test2 {
+  class A {
+    class Private {}; // expected-note {{implicitly declared private here}}
+  public:
+    typedef Private Public;
+  };
+
+  template <class T> class Temp {
+    static Temp<A::Public> make();
+  };
+  template <> class Temp<A::Private> {
+  public:
+    Temp(int x) {}
+  };
+
+  template <> class Temp<A::Private> Temp<int>::make() { // expected-error {{'Private' is a private member of 'test2::A'}}
+    return Temp<A::Public>(0);
+  }
+}