Perform access control for the implicit base and member destructor calls

required when emitting a destructor definition.



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

11 files changed, 273 insertions, 182 deletions

diff --git a/include/clang/Basic/DiagnosticSemaKinds.td b/include/clang/Basic/DiagnosticSemaKinds.td
index 3519899c77..749b0467a6 100644
--- a/include/clang/Basic/DiagnosticSemaKinds.td
+++ b/include/clang/Basic/DiagnosticSemaKinds.td
@@ -428,11 +428,25 @@ def err_deep_exception_specs_differ : Error<
 // C++ access checking
 def err_class_redeclared_with_different_access : Error<
   "%0 redeclared with '%1' access">;
-def err_access_private : Error<"%0 is a private member of %1">;
-def err_access_ctor_private : Error<"calling a private constructor of %0">;
-// Say something about the context for these?
-def err_access_protected : Error<"%0 is a protected member of %1">;
-def err_access_ctor_protected : Error<"calling a protected constructor of %0">;
+def err_access :
+    Error<"%1 is a %select{private|protected}0 member of %3">,
+    NoSFINAE;
+def err_access_ctor :
+    Error<"calling a %select{private|protected}0 constructor of class %2">,
+    NoSFINAE;
+def err_access_dtor_base :
+    Error<"base class %0 has %select{private|protected}1 destructor">,
+    NoSFINAE;
+def err_access_dtor_vbase :
+    Error<"inherited virtual base class %0 has "
+    "%select{private|protected}1 destructor">,
+    NoSFINAE;
+def err_access_dtor_field :
+    Error<"field of type %1 has %select{private|protected}2 destructor">,
+    NoSFINAE;
+def err_access_dtor_var :
+    Error<"variable of type %1 has %select{private|protected}2 destructor">,
+    NoSFINAE;
 def note_previous_access_declaration : Note<
   "previously declared '%1' here">;
 def err_access_outside_class : Error<
diff --git a/include/clang/Basic/PartialDiagnostic.h b/include/clang/Basic/PartialDiagnostic.h
index 873aaeecb6..fb861dcf93 100644
--- a/include/clang/Basic/PartialDiagnostic.h
+++ b/include/clang/Basic/PartialDiagnostic.h
@@ -69,6 +69,10 @@ class PartialDiagnostic {
     CodeModificationHint CodeModificationHints[MaxCodeModificationHints];    
   };
 
+  // NOTE: Sema assumes that PartialDiagnostic is location-invariant
+  // in the sense that its bits can be safely memcpy'ed and destructed
+  // in the new location.
+
   /// DiagID - The diagnostic ID.
   mutable unsigned DiagID;
   
diff --git a/lib/Sema/Sema.h b/lib/Sema/Sema.h
index c5b719b1d8..2675734eca 100644
--- a/lib/Sema/Sema.h
+++ b/lib/Sema/Sema.h
@@ -300,60 +300,38 @@ public:
   /// \brief The set of static functions seen so far that have not been used.
   std::vector<FunctionDecl*> UnusedStaticFuncs;
   
-  /// An enum describing the kind of diagnostics to use when checking
-  /// access.
-  enum AccessDiagnosticsKind {
-    /// Suppress diagnostics.
-    ADK_quiet,
-
-    /// Use the normal diagnostics.
-    ADK_normal,
-
-    /// Use the diagnostics appropriate for checking a covariant
-    /// return type.
-    ADK_covariance
-  };
-
   class AccessedEntity {
   public:
-    enum Kind {
-      /// A member declaration found through lookup.  The target is the
-      /// member.
-      Member,
-
-      /// A base-to-derived conversion.  The target is the base class.
-      BaseToDerivedConversion,
-
-      /// A derived-to-base conversion.  The target is the base class.
-      DerivedToBaseConversion
-    };
-
-    bool isMemberAccess() const { return K == Member; }
-
-    static AccessedEntity makeMember(CXXRecordDecl *NamingClass,
-                                     AccessSpecifier Access,
-                                     NamedDecl *Target) {
-      AccessedEntity E;
-      E.K = Member;
-      E.Access = Access;
-      E.Target = Target;
-      E.NamingClass = NamingClass;
-      return E;
+    /// A member declaration found through lookup.  The target is the
+    /// member.
+    enum MemberNonce { Member };
+
+    /// A hierarchy (base-to-derived or derived-to-base) conversion.
+    /// The target is the base class.
+    enum BaseNonce { Base };
+
+    bool isMemberAccess() const { return IsMember; }
+
+    AccessedEntity(MemberNonce _,
+                   CXXRecordDecl *NamingClass,
+                   AccessSpecifier Access,
+                   NamedDecl *Target)
+      : Access(Access), IsMember(true), 
+        Target(Target), NamingClass(NamingClass),
+        Diag(0) {
     }
 
-    static AccessedEntity makeBaseClass(bool BaseToDerived,
-                                        CXXRecordDecl *BaseClass,
-                                        CXXRecordDecl *DerivedClass,
-                                        AccessSpecifier Access) {
-      AccessedEntity E;
-      E.K = BaseToDerived ? BaseToDerivedConversion : DerivedToBaseConversion;
-      E.Access = Access;
-      E.Target = BaseClass;
-      E.NamingClass = DerivedClass;
-      return E;
+    AccessedEntity(BaseNonce _,
+                   CXXRecordDecl *BaseClass,
+                   CXXRecordDecl *DerivedClass,
+                   AccessSpecifier Access)
+      : Access(Access), IsMember(false),
+        Target(BaseClass), NamingClass(DerivedClass),
+        Diag(0) {
     }
 
-    Kind getKind() const { return Kind(K); }
+    bool isQuiet() const { return Diag.getDiagID() == 0; }
+
     AccessSpecifier getAccess() const { return AccessSpecifier(Access); }
 
     // These apply to member decls...
@@ -364,11 +342,32 @@ public:
     CXXRecordDecl *getBaseClass() const { return cast<CXXRecordDecl>(Target); }
     CXXRecordDecl *getDerivedClass() const { return NamingClass; }
 
+    /// Sets a diagnostic to be performed.  The diagnostic is given
+    /// four (additional) arguments:
+    ///   %0 - 0 if the entity was private, 1 if protected
+    ///   %1 - the DeclarationName of the entity
+    ///   %2 - the TypeDecl type of the naming class
+    ///   %3 - the TypeDecl type of the declaring class
+    void setDiag(const PartialDiagnostic &PDiag) {
+      assert(isQuiet() && "partial diagnostic already defined");
+      Diag = PDiag;
+    }
+    PartialDiagnostic &setDiag(unsigned DiagID) {
+      assert(isQuiet() && "partial diagnostic already defined");
+      assert(DiagID && "creating null diagnostic");
+      Diag = PartialDiagnostic(DiagID);
+      return Diag;
+    }
+    const PartialDiagnostic &getDiag() const {
+      return Diag;
+    }
+
   private:
-    unsigned K : 2;
     unsigned Access : 2;
+    bool IsMember;
     NamedDecl *Target;
     CXXRecordDecl *NamingClass;    
+    PartialDiagnostic Diag;
   };
 
   struct DelayedDiagnostic {
@@ -384,9 +383,16 @@ public:
       struct { NamedDecl *Decl; } DeprecationData;
 
       /// Access control.
-      AccessedEntity AccessData;
+      char AccessData[sizeof(AccessedEntity)];
     };
 
+    void destroy() {
+      switch (Kind) {
+      case Access: getAccessData().~AccessedEntity(); break;
+      case Deprecation: break;
+      }
+    }
+
     static DelayedDiagnostic makeDeprecation(SourceLocation Loc,
                                              NamedDecl *D) {
       DelayedDiagnostic DD;
@@ -403,10 +409,16 @@ public:
       DD.Kind = Access;
       DD.Triggered = false;
       DD.Loc = Loc;
-      DD.AccessData = Entity;
+      new (&DD.getAccessData()) AccessedEntity(Entity);
       return DD;
     }
 
+    AccessedEntity &getAccessData() {
+      return *reinterpret_cast<AccessedEntity*>(AccessData);
+    }
+    const AccessedEntity &getAccessData() const {
+      return *reinterpret_cast<const AccessedEntity*>(AccessData);
+    }
   };
 
   /// \brief The stack of diagnostics that were delayed due to being
@@ -2566,7 +2578,7 @@ public:
                                     SourceLocation Loc, SourceRange Range,
                                     bool IgnoreAccess = false);
   bool CheckDerivedToBaseConversion(QualType Derived, QualType Base,
-                                    AccessDiagnosticsKind ADK,
+                                    unsigned InaccessibleBaseID,
                                     unsigned AmbigiousBaseConvID,
                                     SourceLocation Loc, SourceRange Range,
                                     DeclarationName Name);
@@ -2614,18 +2626,19 @@ public:
                                       CXXConstructorDecl *D,
                                       AccessSpecifier Access);
   AccessResult CheckDestructorAccess(SourceLocation Loc,
-                                     const RecordType *Record);
+                                     CXXDestructorDecl *Dtor,
+                                     const PartialDiagnostic &PDiag);
   AccessResult CheckMemberOperatorAccess(SourceLocation Loc,
                                          Expr *ObjectExpr,
+                                         Expr *ArgExpr,
                                          NamedDecl *D,
                                          AccessSpecifier Access);
   AccessResult CheckBaseClassAccess(SourceLocation AccessLoc,
-                                    bool IsBaseToDerived,
                                     QualType Base, QualType Derived,
                                     const CXXBasePath &Path,
+                                    unsigned DiagID,
                                     bool ForceCheck = false,
-                                    bool ForceUnprivileged = false,
-                                    AccessDiagnosticsKind ADK = ADK_normal);
+                                    bool ForceUnprivileged = false);
                             
   void CheckLookupAccess(const LookupResult &R);
 
diff --git a/lib/Sema/SemaAccess.cpp b/lib/Sema/SemaAccess.cpp
index 85c4662846..818d369f74 100644
--- a/lib/Sema/SemaAccess.cpp
+++ b/lib/Sema/SemaAccess.cpp
@@ -255,18 +255,11 @@ static void DiagnoseInaccessibleMember(Sema &S, SourceLocation Loc,
   NamedDecl *D = Entity.getTargetDecl();
   CXXRecordDecl *DeclaringClass = FindDeclaringClass(D);
 
-  if (isa<CXXConstructorDecl>(D)) {
-    unsigned DiagID = (Access == AS_protected ? diag::err_access_ctor_protected
-                                              : diag::err_access_ctor_private);
-    S.Diag(Loc, DiagID)
-      << S.Context.getTypeDeclType(DeclaringClass);
-  } else {
-    unsigned DiagID = (Access == AS_protected ? diag::err_access_protected
-                                              : diag::err_access_private);
-    S.Diag(Loc, DiagID)
-      << D->getDeclName()
-      << S.Context.getTypeDeclType(DeclaringClass);
-  }
+  S.Diag(Loc, Entity.getDiag())
+    << (Access == AS_protected)
+    << D->getDeclName()
+    << S.Context.getTypeDeclType(NamingClass)
+    << S.Context.getTypeDeclType(DeclaringClass);
   DiagnoseAccessPath(S, EC, NamingClass, DeclaringClass, D, Access);
 }
 
@@ -274,39 +267,25 @@ static void DiagnoseInaccessibleMember(Sema &S, SourceLocation Loc,
 static void DiagnoseInaccessibleBase(Sema &S, SourceLocation Loc,
                                      const EffectiveContext &EC,
                                      AccessSpecifier Access,
-                                     const Sema::AccessedEntity &Entity,
-                                     Sema::AccessDiagnosticsKind ADK) {
-  if (ADK == Sema::ADK_covariance) {
-    S.Diag(Loc, diag::err_covariant_return_inaccessible_base)
-      << S.Context.getTypeDeclType(Entity.getDerivedClass())
-      << S.Context.getTypeDeclType(Entity.getBaseClass())
-      << (Access == AS_protected);
-  } else if (Entity.getKind() == Sema::AccessedEntity::BaseToDerivedConversion) {
-    S.Diag(Loc, diag::err_downcast_from_inaccessible_base)
-      << S.Context.getTypeDeclType(Entity.getDerivedClass())
-      << S.Context.getTypeDeclType(Entity.getBaseClass())
-      << (Access == AS_protected);
-  } else {
-    S.Diag(Loc, diag::err_upcast_to_inaccessible_base)
-      << S.Context.getTypeDeclType(Entity.getDerivedClass())
-      << S.Context.getTypeDeclType(Entity.getBaseClass())
-      << (Access == AS_protected);
-  }
+                                     const Sema::AccessedEntity &Entity) {
+  S.Diag(Loc, Entity.getDiag())
+    << (Access == AS_protected)
+    << DeclarationName()
+    << S.Context.getTypeDeclType(Entity.getDerivedClass())
+    << S.Context.getTypeDeclType(Entity.getBaseClass());
   DiagnoseAccessPath(S, EC, Entity.getDerivedClass(),
                      Entity.getBaseClass(), 0, Access);
 }
 
-static void DiagnoseBadAccess(Sema &S,
-                              SourceLocation Loc,
+static void DiagnoseBadAccess(Sema &S, SourceLocation Loc,
                               const EffectiveContext &EC,
                               CXXRecordDecl *NamingClass,
                               AccessSpecifier Access,
-                              const Sema::AccessedEntity &Entity,
-                              Sema::AccessDiagnosticsKind ADK) {
+                              const Sema::AccessedEntity &Entity) {
   if (Entity.isMemberAccess())
     DiagnoseInaccessibleMember(S, Loc, EC, NamingClass, Access, Entity);
   else
-    DiagnoseInaccessibleBase(S, Loc, EC, Access, Entity, ADK);
+    DiagnoseInaccessibleBase(S, Loc, EC, Access, Entity);
 }
 
 
@@ -369,8 +348,7 @@ static void TryElevateAccess(Sema &S,
 static Sema::AccessResult CheckEffectiveAccess(Sema &S,
                                                const EffectiveContext &EC,
                                                SourceLocation Loc,
-                                         Sema::AccessedEntity const &Entity,
-                                         Sema::AccessDiagnosticsKind ADK) {
+                                         Sema::AccessedEntity const &Entity) {
   AccessSpecifier Access = Entity.getAccess();
   assert(Access != AS_public);
 
@@ -378,14 +356,14 @@ static Sema::AccessResult CheckEffectiveAccess(Sema &S,
   while (NamingClass->isAnonymousStructOrUnion())
     // This should be guaranteed by the fact that the decl has
     // non-public access.  If not, we should make it guaranteed!
-    NamingClass = cast<CXXRecordDecl>(NamingClass);
+    NamingClass = cast<CXXRecordDecl>(NamingClass->getParent());
 
   if (!EC.Record) {
     TryElevateAccess(S, EC, Entity, Access);
     if (Access == AS_public) return Sema::AR_accessible;
 
-    if (ADK != Sema::ADK_quiet)
-      DiagnoseBadAccess(S, Loc, EC, NamingClass, Access, Entity, ADK);
+    if (!Entity.isQuiet())
+      DiagnoseBadAccess(S, Loc, EC, NamingClass, Access, Entity);
     return Sema::AR_inaccessible;
   }
 
@@ -418,15 +396,13 @@ static Sema::AccessResult CheckEffectiveAccess(Sema &S,
   }
     
   // Okay, that's it, reject it.
-  if (ADK != Sema::ADK_quiet)
-    DiagnoseBadAccess(S, Loc, EC, NamingClass, Access, Entity, ADK);
+  if (!Entity.isQuiet())
+    DiagnoseBadAccess(S, Loc, EC, NamingClass, Access, Entity);
   return Sema::AR_inaccessible;
 }
 
 static Sema::AccessResult CheckAccess(Sema &S, SourceLocation Loc,
-                                      const Sema::AccessedEntity &Entity,
-                                      Sema::AccessDiagnosticsKind ADK
-                                        = Sema::ADK_normal) {
+                                      const Sema::AccessedEntity &Entity) {
   // If the access path is public, it's accessible everywhere.
   if (Entity.getAccess() == AS_public)
     return Sema::AR_accessible;
@@ -436,14 +412,13 @@ static Sema::AccessResult CheckAccess(Sema &S, SourceLocation Loc,
   // can actually change our effective context for the purposes of
   // access control.
   if (S.CurContext->isFileContext() && S.ParsingDeclDepth) {
-    assert(ADK == Sema::ADK_normal && "delaying abnormal access check");
     S.DelayedDiagnostics.push_back(
         Sema::DelayedDiagnostic::makeAccess(Loc, Entity));
     return Sema::AR_delayed;
   }
 
   return CheckEffectiveAccess(S, EffectiveContext(S.CurContext),
-                              Loc, Entity, ADK);
+                              Loc, Entity);
 }
 
 void Sema::HandleDelayedAccessCheck(DelayedDiagnostic &DD, Decl *Ctx) {
@@ -451,18 +426,23 @@ void Sema::HandleDelayedAccessCheck(DelayedDiagnostic &DD, Decl *Ctx) {
   // declaration.
   EffectiveContext EC(Ctx->getDeclContext());
 
-  if (CheckEffectiveAccess(*this, EC, DD.Loc, DD.AccessData, ADK_normal))
+  if (CheckEffectiveAccess(*this, EC, DD.Loc, DD.getAccessData()))
     DD.Triggered = true;
 }
 
 Sema::AccessResult Sema::CheckUnresolvedLookupAccess(UnresolvedLookupExpr *E,
                                                      NamedDecl *D,
                                                      AccessSpecifier Access) {
-  if (!getLangOptions().AccessControl || !E->getNamingClass())
+  if (!getLangOptions().AccessControl ||
+      !E->getNamingClass() ||
+      Access == AS_public)
     return AR_accessible;
 
-  return CheckAccess(*this, E->getNameLoc(),
-                 AccessedEntity::makeMember(E->getNamingClass(), Access, D));
+  AccessedEntity Entity(AccessedEntity::Member,
+                        E->getNamingClass(), Access, D);
+  Entity.setDiag(diag::err_access) << E->getSourceRange();
+
+  return CheckAccess(*this, E->getNameLoc(), Entity);
 }
 
 /// Perform access-control checking on a previously-unresolved member
@@ -470,56 +450,73 @@ Sema::AccessResult Sema::CheckUnresolvedLookupAccess(UnresolvedLookupExpr *E,
 Sema::AccessResult Sema::CheckUnresolvedMemberAccess(UnresolvedMemberExpr *E,
                                                      NamedDecl *D,
                                                      AccessSpecifier Access) {
-  if (!getLangOptions().AccessControl)
+  if (!getLangOptions().AccessControl ||
+      Access == AS_public)
     return AR_accessible;
 
-  return CheckAccess(*this, E->getMemberLoc(),
-                 AccessedEntity::makeMember(E->getNamingClass(), Access, D));
+  AccessedEntity Entity(AccessedEntity::Member,
+                        E->getNamingClass(), Access, D);
+  Entity.setDiag(diag::err_access) << E->getSourceRange();
+
+  return CheckAccess(*this, E->getMemberLoc(), Entity);
 }
 
 Sema::AccessResult Sema::CheckDestructorAccess(SourceLocation Loc,
-                                               const RecordType *RT) {
+                                               CXXDestructorDecl *Dtor,
+                                               const PartialDiagnostic &PDiag) {
   if (!getLangOptions().AccessControl)
     return AR_accessible;
 
-  CXXRecordDecl *NamingClass = cast<CXXRecordDecl>(RT->getDecl());
-  CXXDestructorDecl *Dtor = NamingClass->getDestructor(Context);
-
+  // There's never a path involved when checking implicit destructor access.
   AccessSpecifier Access = Dtor->getAccess();
   if (Access == AS_public)
     return AR_accessible;
 
-  return CheckAccess(*this, Loc,
-                 AccessedEntity::makeMember(NamingClass, Access, Dtor));
+  CXXRecordDecl *NamingClass = Dtor->getParent();
+  AccessedEntity Entity(AccessedEntity::Member, NamingClass, Access, Dtor);
+  Entity.setDiag(PDiag); // TODO: avoid copy
+
+  return CheckAccess(*this, Loc, Entity);
 }
 
 /// Checks access to a constructor.
 Sema::AccessResult Sema::CheckConstructorAccess(SourceLocation UseLoc,
                                   CXXConstructorDecl *Constructor,
                                   AccessSpecifier Access) {
-  if (!getLangOptions().AccessControl)
+  if (!getLangOptions().AccessControl ||
+      Access == AS_public)
     return AR_accessible;
 
   CXXRecordDecl *NamingClass = Constructor->getParent();
-  return CheckAccess(*this, UseLoc,
-                 AccessedEntity::makeMember(NamingClass, Access, Constructor));
+  AccessedEntity Entity(AccessedEntity::Member,
+                        NamingClass, Access, Constructor);
+  Entity.setDiag(diag::err_access_ctor);
+
+  return CheckAccess(*this, UseLoc, Entity);
 }
 
 /// Checks access to an overloaded member operator, including
 /// conversion operators.
 Sema::AccessResult Sema::CheckMemberOperatorAccess(SourceLocation OpLoc,
                                                    Expr *ObjectExpr,
+                                                   Expr *ArgExpr,
                                                    NamedDecl *MemberOperator,
                                                    AccessSpecifier Access) {
-  if (!getLangOptions().AccessControl)
+  if (!getLangOptions().AccessControl ||
+      Access == AS_public)
     return AR_accessible;
 
   const RecordType *RT = ObjectExpr->getType()->getAs<RecordType>();
   assert(RT && "found member operator but object expr not of record type");
   CXXRecordDecl *NamingClass = cast<CXXRecordDecl>(RT->getDecl());
 
-  return CheckAccess(*this, OpLoc,
-            AccessedEntity::makeMember(NamingClass, Access, MemberOperator));
+  AccessedEntity Entity(AccessedEntity::Member,
+                        NamingClass, Access, MemberOperator);
+  Entity.setDiag(diag::err_access)
+    << ObjectExpr->getSourceRange()
+    << (ArgExpr ? ArgExpr->getSourceRange() : SourceRange());
+
+  return CheckAccess(*this, OpLoc, Entity);
 }
 
 /// Checks access for a hierarchy conversion.
@@ -532,31 +529,29 @@ Sema::AccessResult Sema::CheckMemberOperatorAccess(SourceLocation OpLoc,
 ///     context had no special privileges
 /// \param ADK controls the kind of diagnostics that are used
 Sema::AccessResult Sema::CheckBaseClassAccess(SourceLocation AccessLoc,
-                                              bool IsBaseToDerived,
                                               QualType Base,
                                               QualType Derived,
                                               const CXXBasePath &Path,
+                                              unsigned DiagID,
                                               bool ForceCheck,
-                                              bool ForceUnprivileged,
-                                              AccessDiagnosticsKind ADK) {
+                                              bool ForceUnprivileged) {
   if (!ForceCheck && !getLangOptions().AccessControl)
     return AR_accessible;
 
   if (Path.Access == AS_public)
     return AR_accessible;
 
-  // TODO: preserve the information about which types exactly were used.
   CXXRecordDecl *BaseD, *DerivedD;
   BaseD = cast<CXXRecordDecl>(Base->getAs<RecordType>()->getDecl());
   DerivedD = cast<CXXRecordDecl>(Derived->getAs<RecordType>()->getDecl());
-  AccessedEntity Entity = AccessedEntity::makeBaseClass(IsBaseToDerived,
-                                                        BaseD, DerivedD,
-                                                        Path.Access);
+
+  AccessedEntity Entity(AccessedEntity::Base, BaseD, DerivedD, Path.Access);
+  if (DiagID)
+    Entity.setDiag(DiagID) << Derived << Base;
 
   if (ForceUnprivileged)
-    return CheckEffectiveAccess(*this, EffectiveContext(),
-                                AccessLoc, Entity, ADK);
-  return CheckAccess(*this, AccessLoc, Entity, ADK);
+    return CheckEffectiveAccess(*this, EffectiveContext(), AccessLoc, Entity);
+  return CheckAccess(*this, AccessLoc, Entity);
 }
 
 /// Checks access to all the declarations in the given result set.
@@ -565,9 +560,13 @@ void Sema::CheckLookupAccess(const LookupResult &R) {
          && "performing access check without access control");
   assert(R.getNamingClass() && "performing access check without naming class");
 
-  for (LookupResult::iterator I = R.begin(), E = R.end(); I != E; ++I)
-    if (I.getAccess() != AS_public)
-      CheckAccess(*this, R.getNameLoc(),
-                  AccessedEntity::makeMember(R.getNamingClass(),
-                                             I.getAccess(), *I));
+  for (LookupResult::iterator I = R.begin(), E = R.end(); I != E; ++I) {
+    if (I.getAccess() != AS_public) {
+      AccessedEntity Entity(AccessedEntity::Member,
+                            R.getNamingClass(), I.getAccess(), *I);
+      Entity.setDiag(diag::err_access);
+
+      CheckAccess(*this, R.getNameLoc(), Entity);
+    }
+  }
 }
diff --git a/lib/Sema/SemaCXXCast.cpp b/lib/Sema/SemaCXXCast.cpp
index e04abd2aac..014cec2b65 100644
--- a/lib/Sema/SemaCXXCast.cpp
+++ b/lib/Sema/SemaCXXCast.cpp
@@ -780,9 +780,9 @@ TryStaticDowncast(Sema &Self, CanQualType SrcType, CanQualType DestType,
   }
 
   if (!CStyle && Self.CheckBaseClassAccess(OpRange.getBegin(),
-                                           /*IsBaseToDerived*/ true,
                                            SrcType, DestType,
-                                           Paths.front())) {
+                                           Paths.front(),
+                                diag::err_downcast_from_inaccessible_base)) {
     msg = 0;
     return TC_Failed;
   }
@@ -858,9 +858,9 @@ TryStaticMemberPointerUpcast(Sema &Self, Expr *&SrcExpr, QualType SrcType,
   }
 
   if (!CStyle && Self.CheckBaseClassAccess(OpRange.getBegin(),
-                                           /*IsBaseToDerived*/ false,
                                            DestType, SrcType,
-                                           Paths.front())) {
+                                           Paths.front(),
+                                     diag::err_upcast_to_inaccessible_base)) {
     msg = 0;
     return TC_Failed;
   }
diff --git a/lib/Sema/SemaDeclAttr.cpp b/lib/Sema/SemaDeclAttr.cpp
index d369b773e8..73a34f8107 100644
--- a/lib/Sema/SemaDeclAttr.cpp
+++ b/lib/Sema/SemaDeclAttr.cpp
@@ -2038,6 +2038,8 @@ void Sema::PopParsingDeclaration(ParsingDeclStackState S, DeclPtrTy Ctx) {
   assert(SavedIndex <= DelayedDiagnostics.size() &&
          "saved index is out of bounds");
 
+  unsigned E = DelayedDiagnostics.size();
+
   // We only want to actually emit delayed diagnostics when we
   // successfully parsed a decl.
   Decl *D = Ctx ? Ctx.getAs<Decl>() : 0;
@@ -2048,7 +2050,7 @@ void Sema::PopParsingDeclaration(ParsingDeclStackState S, DeclPtrTy Ctx) {
     // 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, E = DelayedDiagnostics.size(); I != E; ++I) {
+    for (unsigned I = 0; I != E; ++I) {
       if (DelayedDiagnostics[I].Triggered)
         continue;
 
@@ -2064,6 +2066,10 @@ void Sema::PopParsingDeclaration(ParsingDeclStackState S, DeclPtrTy Ctx) {
     }
   }
 
+  // Destroy all the delayed diagnostics we're about to pop off.
+  for (unsigned I = SavedIndex; I != E; ++I)
+    DelayedDiagnostics[I].destroy();
+
   DelayedDiagnostics.set_size(SavedIndex);
 }
 
diff --git a/lib/Sema/SemaDeclCXX.cpp b/lib/Sema/SemaDeclCXX.cpp
index db81e48f1d..1361318854 100644
--- a/lib/Sema/SemaDeclCXX.cpp
+++ b/lib/Sema/SemaDeclCXX.cpp
@@ -720,7 +720,7 @@ bool Sema::IsDerivedFrom(QualType Derived, QualType Base, CXXBasePaths &Paths) {
 /// if there is an error.
 bool
 Sema::CheckDerivedToBaseConversion(QualType Derived, QualType Base,
-                                   AccessDiagnosticsKind ADK,
+                                   unsigned InaccessibleBaseID,
                                    unsigned AmbigiousBaseConvID,
                                    SourceLocation Loc, SourceRange Range,
                                    DeclarationName Name) {
@@ -736,15 +736,12 @@ Sema::CheckDerivedToBaseConversion(QualType Derived, QualType Base,
   (void)DerivationOkay;
   
   if (!Paths.isAmbiguous(Context.getCanonicalType(Base).getUnqualifiedType())) {
-    if (ADK == ADK_quiet)
+    if (!InaccessibleBaseID)
       return false;
 
     // Check that the base class can be accessed.
-    switch (CheckBaseClassAccess(Loc, /*IsBaseToDerived*/ false,
-                                 Base, Derived, Paths.front(),
-                                 /*force*/ false,
-                                 /*unprivileged*/ false,
-                                 ADK)) {
+    switch (CheckBaseClassAccess(Loc, Base, Derived, Paths.front(),
+                                 InaccessibleBaseID)) {
     case AR_accessible: return false;
     case AR_inaccessible: return true;
     case AR_dependent: return false;
@@ -780,7 +777,8 @@ Sema::CheckDerivedToBaseConversion(QualType Derived, QualType Base,
                                    SourceLocation Loc, SourceRange Range,
                                    bool IgnoreAccess) {
   return CheckDerivedToBaseConversion(Derived, Base,
-                                      IgnoreAccess ? ADK_quiet : ADK_normal,
+                                      IgnoreAccess ? 0
+                                       : diag::err_upcast_to_inaccessible_base,
                                       diag::err_ambiguous_derived_to_base_conv,
                                       Loc, Range, DeclarationName());
 }
@@ -1854,6 +1852,11 @@ Sema::MarkBaseAndMemberDestructorsReferenced(CXXDestructorDecl *Destructor) {
   // Ignore dependent destructors.
   if (Destructor->isDependentContext())
     return;
+
+  // FIXME: all the access-control diagnostics are positioned on the
+  // field/base declaration.  That's probably good; that said, the
+  // user might reasonably want to know why the destructor is being
+  // emitted, and we currently don't say.
   
   CXXRecordDecl *ClassDecl = Destructor->getParent();
 
@@ -1872,25 +1875,41 @@ Sema::MarkBaseAndMemberDestructorsReferenced(CXXDestructorDecl *Destructor) {
     if (FieldClassDecl->hasTrivialDestructor())
       continue;
 
-    const CXXDestructorDecl *Dtor = FieldClassDecl->getDestructor(Context);
+    CXXDestructorDecl *Dtor = FieldClassDecl->getDestructor(Context);
+    CheckDestructorAccess(Field->getLocation(), Dtor,
+                          PartialDiagnostic(diag::err_access_dtor_field)
+                            << Field->getDeclName()
+                            << FieldType);
+
     MarkDeclarationReferenced(Destructor->getLocation(),
                               const_cast<CXXDestructorDecl*>(Dtor));
   }
 
+  llvm::SmallPtrSet<const RecordType *, 8> DirectVirtualBases;
+
   // Bases.
   for (CXXRecordDecl::base_class_iterator Base = ClassDecl->bases_begin(),
        E = ClassDecl->bases_end(); Base != E; ++Base) {
-    // Ignore virtual bases.
+    // Bases are always records in a well-formed non-dependent class.
+    const RecordType *RT = Base->getType()->getAs<RecordType>();
+
+    // Remember direct virtual bases.
     if (Base->isVirtual())
-      continue;
+      DirectVirtualBases.insert(RT);
 
     // Ignore trivial destructors.
-    CXXRecordDecl *BaseClassDecl
-      = cast<CXXRecordDecl>(Base->getType()->getAs<RecordType>()->getDecl());
+    CXXRecordDecl *BaseClassDecl = cast<CXXRecordDecl>(RT->getDecl());
     if (BaseClassDecl->hasTrivialDestructor())
       continue;
+
+    CXXDestructorDecl *Dtor = BaseClassDecl->getDestructor(Context);
+
+    // FIXME: caret should be on the start of the class name
+    CheckDestructorAccess(Base->getSourceRange().getBegin(), Dtor,
+                          PartialDiagnostic(diag::err_access_dtor_base)
+                            << Base->getType()
+                            << Base->getSourceRange());
     
-    const CXXDestructorDecl *Dtor = BaseClassDecl->getDestructor(Context);
     MarkDeclarationReferenced(Destructor->getLocation(),
                               const_cast<CXXDestructorDecl*>(Dtor));
   }
@@ -1898,13 +1917,24 @@ Sema::MarkBaseAndMemberDestructorsReferenced(CXXDestructorDecl *Destructor) {
   // Virtual bases.
   for (CXXRecordDecl::base_class_iterator VBase = ClassDecl->vbases_begin(),
        E = ClassDecl->vbases_end(); VBase != E; ++VBase) {
+
+    // Bases are always records in a well-formed non-dependent class.
+    const RecordType *RT = VBase->getType()->getAs<RecordType>();
+
+    // Ignore direct virtual bases.
+    if (DirectVirtualBases.count(RT))
+      continue;
+
     // Ignore trivial destructors.
-    CXXRecordDecl *BaseClassDecl
-      = cast<CXXRecordDecl>(VBase->getType()->getAs<RecordType>()->getDecl());
+    CXXRecordDecl *BaseClassDecl = cast<CXXRecordDecl>(RT->getDecl());
     if (BaseClassDecl->hasTrivialDestructor())
       continue;
-    
-    const CXXDestructorDecl *Dtor = BaseClassDecl->getDestructor(Context);
+
+    CXXDestructorDecl *Dtor = BaseClassDecl->getDestructor(Context);
+    CheckDestructorAccess(ClassDecl->getLocation(), Dtor,
+                          PartialDiagnostic(diag::err_access_dtor_vbase)
+                            << VBase->getType());
+
     MarkDeclarationReferenced(Destructor->getLocation(),
                               const_cast<CXXDestructorDecl*>(Dtor));
   }
@@ -4062,7 +4092,10 @@ void Sema::FinalizeVarWithDestructor(VarDecl *VD, const RecordType *Record) {
       !ClassDecl->hasTrivialDestructor()) {
     CXXDestructorDecl *Destructor = ClassDecl->getDestructor(Context);
     MarkDeclarationReferenced(VD->getLocation(), Destructor);
-    CheckDestructorAccess(VD->getLocation(), Record);
+    CheckDestructorAccess(VD->getLocation(), Destructor,
+                          PartialDiagnostic(diag::err_access_dtor_var)
+                            << VD->getDeclName()
+                            << VD->getType());
   }
 }
 
@@ -5725,7 +5758,8 @@ bool Sema::CheckOverridingFunctionReturnType(const CXXMethodDecl *New,
     }
 
     // Check if we the conversion from derived to base is valid.
-    if (CheckDerivedToBaseConversion(NewClassTy, OldClassTy, ADK_covariance,
+    if (CheckDerivedToBaseConversion(NewClassTy, OldClassTy,
+                      diag::err_covariant_return_inaccessible_base,
                       diag::err_covariant_return_ambiguous_derived_to_base_conv,
                       // FIXME: Should this point to the return type?
                       New->getLocation(), SourceRange(), New->getDeclName())) {
diff --git a/lib/Sema/SemaExceptionSpec.cpp b/lib/Sema/SemaExceptionSpec.cpp
index 9be411b552..4ce1ce9b01 100644
--- a/lib/Sema/SemaExceptionSpec.cpp
+++ b/lib/Sema/SemaExceptionSpec.cpp
@@ -294,12 +294,12 @@ bool Sema::CheckExceptionSpecSubset(
         continue;
 
       // Do this check from a context without privileges.
-      switch (CheckBaseClassAccess(SourceLocation(), false,
+      switch (CheckBaseClassAccess(SourceLocation(),
                                    CanonicalSuperT, CanonicalSubT,
                                    Paths.front(),
+                                   /*Diagnostic*/ 0,
                                    /*ForceCheck*/ true,
-                                   /*ForceUnprivileged*/ true,
-                                   ADK_quiet)) {
+                                   /*ForceUnprivileged*/ true)) {
       case AR_accessible: break;
       case AR_inaccessible: continue;
       case AR_dependent:
diff --git a/lib/Sema/SemaInit.cpp b/lib/Sema/SemaInit.cpp
index 51d2f15376..98a7eec232 100644
--- a/lib/Sema/SemaInit.cpp
+++ b/lib/Sema/SemaInit.cpp
@@ -3401,7 +3401,7 @@ InitializationSequence::Perform(Sema &S,
         // Build a call to the conversion function.
         CXXConversionDecl *Conversion = cast<CXXConversionDecl>(Fn);
 
-        S.CheckMemberOperatorAccess(Kind.getLocation(), CurInitExpr,
+        S.CheckMemberOperatorAccess(Kind.getLocation(), Cur