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
diff --git a/include/clang/Sema/Sema.h b/include/clang/Sema/Sema.h
index d8156ff..4518fe9 100644
--- a/include/clang/Sema/Sema.h
+++ b/include/clang/Sema/Sema.h
@@ -2592,6 +2592,8 @@
     DelayedDiagnostics.popUndelayed(state);
   }
 
+  void redelayDiagnostics(sema::DelayedDiagnosticPool &pool);
+
   void EmitDeprecationWarning(NamedDecl *D, StringRef Message,
                               SourceLocation Loc,
                               const ObjCInterfaceDecl *UnknownObjCClass=0);
@@ -4395,45 +4397,10 @@
 
   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,
diff --git a/lib/Parse/ParseDecl.cpp b/lib/Parse/ParseDecl.cpp
index dbf35b1..7cb6474 100644
--- a/lib/Parse/ParseDecl.cpp
+++ b/lib/Parse/ParseDecl.cpp
@@ -2773,12 +2773,16 @@
     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 @@
     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 @@
   // 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 @@
   }
 
   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,
diff --git a/lib/Parse/ParseDeclCXX.cpp b/lib/Parse/ParseDeclCXX.cpp
index 2a64141..730c345 100644
--- a/lib/Parse/ParseDeclCXX.cpp
+++ b/lib/Parse/ParseDeclCXX.cpp
@@ -968,9 +968,13 @@
   // 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 @@
     }
   }
 
-  // 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 @@
   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) {
diff --git a/lib/Parse/RAIIObjectsForParser.h b/lib/Parse/RAIIObjectsForParser.h
index f5a6f8f..6443127 100644
--- a/lib/Parse/RAIIObjectsForParser.h
+++ b/lib/Parse/RAIIObjectsForParser.h
@@ -24,6 +24,59 @@
   // 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 @@
       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;
diff --git a/lib/Sema/Sema.cpp b/lib/Sema/Sema.cpp
index ca3e233..5017c2c 100644
--- a/lib/Sema/Sema.cpp
+++ b/lib/Sema/Sema.cpp
@@ -99,7 +99,7 @@
     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),
diff --git a/lib/Sema/SemaAccess.cpp b/lib/Sema/SemaAccess.cpp
index 13bb6dc..49e5900 100644
--- a/lib/Sema/SemaAccess.cpp
+++ b/lib/Sema/SemaAccess.cpp
@@ -1391,9 +1391,6 @@
   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.
diff --git a/lib/Sema/SemaDeclAttr.cpp b/lib/Sema/SemaDeclAttr.cpp
index e3a2169..f2d1d19 100644
--- a/lib/Sema/SemaDeclAttr.cpp
+++ b/lib/Sema/SemaDeclAttr.cpp
@@ -4200,8 +4200,7 @@
 
 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 @@
   // 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 @@
   } 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())
diff --git a/test/CXX/temp/temp.spec/temp.explicit/p12.cpp b/test/CXX/temp/temp.spec/temp.explicit/p12.cpp
index c756486..9518a0b 100644
--- a/test/CXX/temp/temp.spec/temp.explicit/p12.cpp
+++ b/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);
+  }
+}