Improve access control diagnostics. Perform access control on member-pointer

conversions.  Fix an access-control bug where privileges were not considered
at intermediate points along the inheritance path.  Prepare for friends.

llvm-svn: 95775

8 files changed, 555 insertions, 293 deletions

diff --git a/clang/lib/Sema/Lookup.h b/clang/lib/Sema/Lookup.h
index fa6f0370264..6b2694591f5 100644
--- a/clang/lib/Sema/Lookup.h
+++ b/clang/lib/Sema/Lookup.h
@@ -503,7 +503,7 @@ private:
     if (isAmbiguous())
       SemaRef.DiagnoseAmbiguousLookup(*this);
     else if (isClassLookup() && SemaRef.getLangOptions().AccessControl)
-      SemaRef.CheckAccess(*this);
+      SemaRef.CheckLookupAccess(*this);
   }
 
   void setAmbiguous(AmbiguityKind AK) {
diff --git a/clang/lib/Sema/Sema.h b/clang/lib/Sema/Sema.h
index 8b3b4650387..a835c477e28 100644
--- a/clang/lib/Sema/Sema.h
+++ b/clang/lib/Sema/Sema.h
@@ -96,6 +96,7 @@ namespace clang {
   class ObjCPropertyDecl;
   class ObjCContainerDecl;
   class FunctionProtoType;
+  class CXXBasePath;
   class CXXBasePaths;
   class CXXTemporary;
   class LookupResult;
@@ -275,6 +276,77 @@ public:
   /// \brief All the tentative definitions encountered in the TU.
   std::vector<VarDecl *> TentativeDefinitions;
 
+  /// 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;
+    }
+
+    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;
+    }
+
+    Kind getKind() const { return Kind(K); }
+    AccessSpecifier getAccess() const { return AccessSpecifier(Access); }
+
+    // These apply to member decls...
+    NamedDecl *getTargetDecl() const { return Target; }
+    CXXRecordDecl *getNamingClass() const { return NamingClass; }
+
+    // ...and these apply to hierarchy conversions.
+    CXXRecordDecl *getBaseClass() const { return cast<CXXRecordDecl>(Target); }
+    CXXRecordDecl *getDerivedClass() const { return NamingClass; }
+
+  private:
+    unsigned K : 2;
+    unsigned Access : 2;
+    NamedDecl *Target;
+    CXXRecordDecl *NamingClass;    
+  };
+
   struct DelayedDiagnostic {
     enum DDKind { Deprecation, Access };
 
@@ -288,11 +360,7 @@ public:
       struct { NamedDecl *Decl; } DeprecationData;
 
       /// Access control.
-      struct {
-        NamedDecl *Decl;
-        AccessSpecifier Access; 
-        CXXRecordDecl *NamingClass;
-      } AccessData;
+      AccessedEntity AccessData;
     };
 
     static DelayedDiagnostic makeDeprecation(SourceLocation Loc,
@@ -306,16 +374,12 @@ public:
     }
 
     static DelayedDiagnostic makeAccess(SourceLocation Loc,
-                                        NamedDecl *Decl,
-                                        AccessSpecifier AS,
-                                        CXXRecordDecl *NamingClass) {
+                                        const AccessedEntity &Entity) {
       DelayedDiagnostic DD;
       DD.Kind = Access;
       DD.Triggered = false;
       DD.Loc = Loc;
-      DD.AccessData.Decl = Decl;
-      DD.AccessData.Access = AS;
-      DD.AccessData.NamingClass = NamingClass;
+      DD.AccessData = Entity;
       return DD;
     }
 
@@ -2385,7 +2449,7 @@ public:
                                     SourceLocation Loc, SourceRange Range,
                                     bool IgnoreAccess = false);
   bool CheckDerivedToBaseConversion(QualType Derived, QualType Base,
-                                    unsigned InaccessibleBaseID,
+                                    AccessDiagnosticsKind ADK,
                                     unsigned AmbigiousBaseConvID,
                                     SourceLocation Loc, SourceRange Range,
                                     DeclarationName Name);
@@ -2412,38 +2476,43 @@ public:
   // C++ Access Control
   //
 
+  enum AccessResult {
+    AR_accessible,
+    AR_inaccessible,
+    AR_dependent,
+    AR_delayed
+  };
+
   bool SetMemberAccessSpecifier(NamedDecl *MemberDecl,
                                 NamedDecl *PrevMemberDecl,
                                 AccessSpecifier LexicalAS);
 
-  const CXXBaseSpecifier *FindInaccessibleBase(QualType Derived, QualType Base,
-                                               CXXBasePaths &Paths,
-                                               bool NoPrivileges = false);
-
-  bool CheckUnresolvedMemberAccess(UnresolvedMemberExpr *E,
-                                   NamedDecl *D,
-                                   AccessSpecifier Access);
-  bool CheckUnresolvedLookupAccess(UnresolvedLookupExpr *E,
-                                   NamedDecl *D,
-                                   AccessSpecifier Access);
-  bool CheckConstructorAccess(SourceLocation Loc, CXXConstructorDecl *D,
-                              AccessSpecifier Access);
-  bool CheckDestructorAccess(SourceLocation Loc, const RecordType *Record);
-  bool CheckMemberOperatorAccess(SourceLocation Loc, Expr *ObjectExpr,
-                                 NamedDecl *D, AccessSpecifier Access);
-  bool CheckAccess(const LookupResult &R, NamedDecl *D, AccessSpecifier Access);
-  void CheckAccess(const LookupResult &R);
+  AccessResult CheckUnresolvedMemberAccess(UnresolvedMemberExpr *E,
+                                           NamedDecl *D,
+                                           AccessSpecifier Access);
+  AccessResult CheckUnresolvedLookupAccess(UnresolvedLookupExpr *E,
+                                           NamedDecl *D,
+                                           AccessSpecifier Access);
+  AccessResult CheckConstructorAccess(SourceLocation Loc,
+                                      CXXConstructorDecl *D,
+                                      AccessSpecifier Access);
+  AccessResult CheckDestructorAccess(SourceLocation Loc,
+                                     const RecordType *Record);
+  AccessResult CheckMemberOperatorAccess(SourceLocation Loc,
+                                         Expr *ObjectExpr,
+                                         NamedDecl *D,
+                                         AccessSpecifier Access);
+  AccessResult CheckBaseClassAccess(SourceLocation AccessLoc,
+                                    bool IsBaseToDerived,
+                                    QualType Base, QualType Derived,
+                                    const CXXBasePath &Path,
+                                    bool ForceCheck = false,
+                                    bool ForceUnprivileged = false,
+                                    AccessDiagnosticsKind ADK = ADK_normal);
+                            
+  void CheckLookupAccess(const LookupResult &R);
 
   void HandleDelayedAccessCheck(DelayedDiagnostic &DD, Decl *Ctx);
-  bool CheckEffectiveAccess(DeclContext *EffectiveContext,
-                            const LookupResult &R, NamedDecl *D,
-                            AccessSpecifier Access);
-
-  bool CheckBaseClassAccess(QualType Derived, QualType Base,
-                            unsigned InaccessibleBaseID,
-                            CXXBasePaths& Paths, SourceLocation AccessLoc,
-                            DeclarationName Name);
-
 
   enum AbstractDiagSelID {
     AbstractNone = -1,
diff --git a/clang/lib/Sema/SemaAccess.cpp b/clang/lib/Sema/SemaAccess.cpp
index 9e1ab8cefd3..eca8bb4c2a9 100644
--- a/clang/lib/Sema/SemaAccess.cpp
+++ b/clang/lib/Sema/SemaAccess.cpp
@@ -49,326 +49,500 @@ bool Sema::SetMemberAccessSpecifier(NamedDecl *MemberDecl,
   return false;
 }
 
-/// Find a class on the derivation path between Derived and Base that is
-/// inaccessible. If @p NoPrivileges is true, special access rights (members
-/// and friends) are not considered.
-const CXXBaseSpecifier *Sema::FindInaccessibleBase(
-    QualType Derived, QualType Base, CXXBasePaths &Paths, bool NoPrivileges) {
-  Base = Context.getCanonicalType(Base).getUnqualifiedType();
-  assert(!Paths.isAmbiguous(Base) &&
-         "Can't check base class access if set of paths is ambiguous");
-  assert(Paths.isRecordingPaths() &&
-         "Can't check base class access without recorded paths");
-
-
-  const CXXBaseSpecifier *InaccessibleBase = 0;
-
-  const CXXRecordDecl *CurrentClassDecl = 0;
-  if (CXXMethodDecl *MD = dyn_cast_or_null<CXXMethodDecl>(getCurFunctionDecl()))
-    CurrentClassDecl = MD->getParent();
-
-  for (CXXBasePaths::paths_iterator Path = Paths.begin(), PathsEnd = Paths.end();
-      Path != PathsEnd; ++Path) {
-
-    bool FoundInaccessibleBase = false;
-
-    for (CXXBasePath::const_iterator Element = Path->begin(),
-         ElementEnd = Path->end(); Element != ElementEnd; ++Element) {
-      const CXXBaseSpecifier *Base = Element->Base;
-
-      switch (Base->getAccessSpecifier()) {
-      default:
-        assert(0 && "invalid access specifier");
-      case AS_public:
-        // Nothing to do.
-        break;
-      case AS_private:
-        // FIXME: Check if the current function/class is a friend.
-        if (NoPrivileges || CurrentClassDecl != Element->Class)
-          FoundInaccessibleBase = true;
-        break;
-      case AS_protected:
-        // FIXME: Implement
-        break;
-      }
+namespace {
+struct EffectiveContext {
+  EffectiveContext() : Record(0), Function(0) {}
+
+  explicit EffectiveContext(DeclContext *DC) {
+    if (isa<FunctionDecl>(DC)) {
+      Function = cast<FunctionDecl>(DC);
+      DC = Function->getDeclContext();
+    } else
+      Function = 0;
+    
+    if (isa<CXXRecordDecl>(DC))
+      Record = cast<CXXRecordDecl>(DC)->getCanonicalDecl();
+    else
+      Record = 0;
+  }
+
+  bool isClass(const CXXRecordDecl *R) const {
+    return R->getCanonicalDecl() == Record;
+  }
+
+  CXXRecordDecl *Record;
+  FunctionDecl *Function;
+};
+}
 
-      if (FoundInaccessibleBase) {
-        InaccessibleBase = Base;
-        break;
+static CXXRecordDecl *FindDeclaringClass(NamedDecl *D) {
+  CXXRecordDecl *DeclaringClass = cast<CXXRecordDecl>(D->getDeclContext());
+  while (DeclaringClass->isAnonymousStructOrUnion())
+    DeclaringClass = cast<CXXRecordDecl>(DeclaringClass->getDeclContext());
+  return DeclaringClass;
+}
+
+static Sema::AccessResult GetFriendKind(Sema &S,
+                                        const EffectiveContext &EC,
+                                        const CXXRecordDecl *Class) {
+  if (EC.isClass(Class))
+    return Sema::AR_accessible;
+
+  // FIXME: implement
+  return Sema::AR_inaccessible;
+}
+
+/// Finds the best path from the naming class to the declaring class,
+/// taking friend declarations into account.
+///
+/// \return null if friendship is dependent
+static CXXBasePath *FindBestPath(Sema &S,
+                                 const EffectiveContext &EC,
+                                 CXXRecordDecl *Derived,
+                                 CXXRecordDecl *Base,
+                                 CXXBasePaths &Paths) {
+  // Derive the paths to the desired base.
+  bool isDerived = Derived->isDerivedFrom(Base, Paths);
+  assert(isDerived && "derived class not actually derived from base");
+  (void) isDerived;
+
+  CXXBasePath *BestPath = 0;
+
+  // Derive the friend-modified access along each path.
+  for (CXXBasePaths::paths_iterator PI = Paths.begin(), PE = Paths.end();
+         PI != PE; ++PI) {
+
+    // Walk through the path backwards.
+    AccessSpecifier PathAccess = AS_public;
+    CXXBasePath::iterator I = PI->end(), E = PI->begin();
+    while (I != E) {
+      --I;
+
+      AccessSpecifier BaseAccess = I->Base->getAccessSpecifier();
+      if (BaseAccess != AS_public) {
+        switch (GetFriendKind(S, EC, I->Class)) {
+        case Sema::AR_inaccessible: break;
+        case Sema::AR_accessible: BaseAccess = AS_public; break;
+        case Sema::AR_dependent: return 0;
+        case Sema::AR_delayed:
+          llvm_unreachable("friend resolution is never delayed"); break;
+        }
       }
+
+      PathAccess = CXXRecordDecl::MergeAccess(BaseAccess, PathAccess);
     }
 
-    if (!FoundInaccessibleBase) {
-      // We found a path to the base, our work here is done.
-      return 0;
+    // Note that we modify the path's Access field to the
+    // friend-modified access.
+    if (BestPath == 0 || PathAccess < BestPath->Access) {
+      BestPath = &*PI;
+      BestPath->Access = PathAccess;
     }
   }
 
-  assert(InaccessibleBase && "no path found, but no inaccessible base");
-  return InaccessibleBase;
+  return BestPath;
 }
 
-/// CheckBaseClassAccess - Check that a derived class can access its base class
-/// and report an error if it can't. [class.access.base]
-bool Sema::CheckBaseClassAccess(QualType Derived, QualType Base,
-                                unsigned InaccessibleBaseID,
-                                CXXBasePaths &Paths, SourceLocation AccessLoc,
-                                DeclarationName Name) {
+/// Diagnose the path which caused the given declaration or base class
+/// to become inaccessible.
+static void DiagnoseAccessPath(Sema &S,
+                               const EffectiveContext &EC,
+                               CXXRecordDecl *NamingClass,
+                               CXXRecordDecl *DeclaringClass,
+                               NamedDecl *D, AccessSpecifier Access) {
+  // Easy case: the decl's natural access determined its path access.
+  // We have to check against AS_private here in case Access is AS_none,
+  // indicating a non-public member of a private base class.
+  //
+  // DependentFriend should be impossible here.
+  if (D && (Access == D->getAccess() || D->getAccess() == AS_private)) {
+    switch (GetFriendKind(S, EC, DeclaringClass)) {
+    case Sema::AR_inaccessible: {
+      S.Diag(D->getLocation(), diag::note_access_natural)
+        << (unsigned) (Access == AS_protected)
+        << /*FIXME: not implicitly*/ 0;
+      return;
+    }
 
-  if (!getLangOptions().AccessControl)
-    return false;
-  const CXXBaseSpecifier *InaccessibleBase = FindInaccessibleBase(
-                                               Derived, Base, Paths);
+    case Sema::AR_accessible: break;
 
-  if (InaccessibleBase) {
-    Diag(AccessLoc, InaccessibleBaseID)
-      << Derived << Base << Name;
+    case Sema::AR_dependent:
+    case Sema::AR_delayed:
+      llvm_unreachable("dependent/delayed not allowed");
+      return;
+    }
+  }
 
-    AccessSpecifier AS = InaccessibleBase->getAccessSpecifierAsWritten();
+  CXXBasePaths Paths;
+  CXXBasePath &Path = *FindBestPath(S, EC, NamingClass, DeclaringClass, Paths);
 
-    // If there's no written access specifier, then the inheritance specifier
-    // is implicitly private.
-    if (AS == AS_none)
-      Diag(InaccessibleBase->getSourceRange().getBegin(),
-           diag::note_inheritance_implicitly_private_here);
-    else
-      Diag(InaccessibleBase->getSourceRange().getBegin(),
-           diag::note_inheritance_specifier_here) << AS;
+  CXXBasePath::iterator I = Path.end(), E = Path.begin();
+  while (I != E) {
+    --I;
 
-    return true;
+    const CXXBaseSpecifier *BS = I->Base;
+    AccessSpecifier BaseAccess = BS->getAccessSpecifier();
+
+    // If this is public inheritance, or the derived class is a friend,
+    // skip this step.
+    if (BaseAccess == AS_public)
+      continue;
+
+    switch (GetFriendKind(S, EC, I->Class)) {
+    case Sema::AR_accessible: continue;
+    case Sema::AR_inaccessible: break;
+
+    case Sema::AR_dependent:
+    case Sema::AR_delayed:
+      llvm_unreachable("dependent friendship, should not be diagnosing");
+    }
+
+    // Check whether this base specifier is the tighest point
+    // constraining access.  We have to check against AS_private for
+    // the same reasons as above.
+    if (BaseAccess == AS_private || BaseAccess >= Access) {
+
+      // We're constrained by inheritance, but we want to say
+      // "declared private here" if we're diagnosing a hierarchy
+      // conversion and this is the final step.
+      unsigned diagnostic;
+      if (D) diagnostic = diag::note_access_constrained_by_path;
+      else if (I + 1 == Path.end()) diagnostic = diag::note_access_natural;
+      else diagnostic = diag::note_access_constrained_by_path;
+
+      S.Diag(BS->getSourceRange().getBegin(), diagnostic)
+        << BS->getSourceRange()
+        << (BaseAccess == AS_protected)
+        << (BS->getAccessSpecifierAsWritten() == AS_none);
+      return;
+    }
   }
 
-  return false;
+  llvm_unreachable("access not apparently constrained by path");
 }
 
-/// Diagnose the path which caused the given declaration to become
-/// inaccessible.
-static void DiagnoseAccessPath(Sema &S, const LookupResult &R, NamedDecl *D,
-                               AccessSpecifier Access) {
-  // Easy case: the decl's natural access determined its path access.
-  if (Access == D->getAccess() || D->getAccess() == AS_private) {
-    S.Diag(D->getLocation(), diag::note_access_natural)
-      << (unsigned) (Access == AS_protected);
-    return;
+/// Diagnose an inaccessible class member.
+static void DiagnoseInaccessibleMember(Sema &S, SourceLocation Loc,
+                                       const EffectiveContext &EC,
+                                       CXXRecordDecl *NamingClass,
+                                       AccessSpecifier Access,
+                                       const Sema::AccessedEntity &Entity) {
+  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);
   }
+  DiagnoseAccessPath(S, EC, NamingClass, DeclaringClass, D, Access);
+}
 
-  // TODO: flesh this out
-  S.Diag(D->getLocation(), diag::note_access_constrained_by_path)
-    << (unsigned) (Access == AS_protected);
+/// Diagnose an inaccessible hierarchy conversion.
+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);
+  }
+  DiagnoseAccessPath(S, EC, Entity.getDerivedClass(),
+                     Entity.getBaseClass(), 0, Access);
 }
 
-/// Checks access to the given declaration in the current context.
-///
-/// \param R the means via which the access was made; must have a naming
-///   class set
-/// \param D the declaration accessed
-/// \param Access the best access along any inheritance path from the
-///   naming class to the declaration.  AS_none means the path is impossible
-bool Sema::CheckAccess(const LookupResult &R, NamedDecl *D,
-                       AccessSpecifier Access) {
-  assert(R.getNamingClass() && "performing access check without naming class");
+static void DiagnoseBadAccess(Sema &S,
+                              SourceLocation Loc,
+                              const EffectiveContext &EC,
+                              CXXRecordDecl *NamingClass,
+                              AccessSpecifier Access,
+                              const Sema::AccessedEntity &Entity,
+                              Sema::AccessDiagnosticsKind ADK) {
+  if (Entity.isMemberAccess())
+    DiagnoseInaccessibleMember(S, Loc, EC, NamingClass, Access, Entity);
+  else
+    DiagnoseInaccessibleBase(S, Loc, EC, Access, Entity, ADK);
+}
 
-  // If the access path is public, it's accessible everywhere.
-  if (Access == AS_public)
-    return false;
 
-  // 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 (CurContext->isFileContext() && ParsingDeclDepth) {
-    DelayedDiagnostics.push_back(
-        DelayedDiagnostic::makeAccess(R.getNameLoc(), D, Access,
-                                      R.getNamingClass()));
-    return false;
+/// Try to elevate access using friend declarations.  This is
+/// potentially quite expensive.
+static void TryElevateAccess(Sema &S,
+                             const EffectiveContext &EC,
+                             const Sema::AccessedEntity &Entity,
+                             AccessSpecifier &Access) {
+  CXXRecordDecl *DeclaringClass;
+  if (Entity.isMemberAccess()) {
+    DeclaringClass = FindDeclaringClass(Entity.getTargetDecl());
+  } else {
+    DeclaringClass = Entity.getBaseClass();
   }
+  CXXRecordDecl *NamingClass = Entity.getNamingClass();
+
+  // Adjust the declaration of the referred entity.
+  AccessSpecifier DeclAccess = AS_none;
+  if (Entity.isMemberAccess()) {
+    NamedDecl *Target = Entity.getTargetDecl();
+
+    DeclAccess = Target->getAccess();
+    if (DeclAccess != AS_public) {
+      switch (GetFriendKind(S, EC, DeclaringClass)) {
+      case Sema::AR_accessible: DeclAccess = AS_public; break;
+      case Sema::AR_inaccessible: break;
+      case Sema::AR_dependent: /* FIXME: delay dependent friendship */ return;
+      case Sema::AR_delayed: llvm_unreachable("friend status is never delayed");
+      }
+    }
 
-  return CheckEffectiveAccess(CurContext, R, D, Access);
-}
+    if (DeclaringClass == NamingClass) {
+      Access = DeclAccess;
+      return;
+    }
+  }
 
-/// Checks access from the given effective context.
-bool Sema::CheckEffectiveAccess(DeclContext *EffectiveContext,
-                                const LookupResult &R,
-                                NamedDecl *D, AccessSpecifier Access) {
-  DeclContext *DC = EffectiveContext;
-  while (isa<CXXRecordDecl>(DC) &&
-         cast<CXXRecordDecl>(DC)->isAnonymousStructOrUnion())
-    DC = DC->getParent();
-
-  CXXRecordDecl *CurRecord;
-  if (isa<CXXRecordDecl>(DC))
-    CurRecord = cast<CXXRecordDecl>(DC);
-  else if (isa<CXXMethodDecl>(DC))
-    CurRecord = cast<CXXMethodDecl>(DC)->getParent();
-  else {
-    Diag(R.getNameLoc(), diag::err_access_outside_class)
-      << (Access == AS_protected);
-    DiagnoseAccessPath(*this, R, D, Access);
-    return true;
+  assert(DeclaringClass != NamingClass);
+
+  // Append the declaration's access if applicable.
+  CXXBasePaths Paths;
+  CXXBasePath *Path = FindBestPath(S, EC, Entity.getNamingClass(),
+                                   DeclaringClass, Paths);
+  if (!Path) {
+    // FIXME: delay dependent friendship
+    return;
   }
 
-  CXXRecordDecl *NamingClass = R.getNamingClass();
+  // Grab the access along the best path.
+  AccessSpecifier NewAccess = Path->Access;
+  if (Entity.isMemberAccess())
+    NewAccess = CXXRecordDecl::MergeAccess(NewAccess, DeclAccess);
+  
+  assert(NewAccess <= Access && "access along best path worse than direct?");
+  Access = NewAccess;
+}
+
+/// Checks access to an entity from the given effective context.
+static Sema::AccessResult CheckEffectiveAccess(Sema &S,
+                                               const EffectiveContext &EC,
+                                               SourceLocation Loc,
+                                         Sema::AccessedEntity const &Entity,
+                                         Sema::AccessDiagnosticsKind ADK) {
+  AccessSpecifier Access = Entity.getAccess();
+  assert(Access != AS_public);
+
+  CXXRecordDecl *NamingClass = Entity.getNamingClass();
   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);
 
+  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);
+    return Sema::AR_inaccessible;
+  }
+
   // White-list accesses from within the declaring class.
-  if (Access != AS_none &&
-      CurRecord->getCanonicalDecl() == NamingClass->getCanonicalDecl())
-    return false;
+  if (Access != AS_none && EC.isClass(NamingClass))
+    return Sema::AR_accessible;
+  
+  // If the access is worse than 'protected', try to promote to it using
+  // friend declarations.
+  bool TriedElevation = false;
+  if (Access != AS_protected) {
+    TryElevateAccess(S, EC, Entity, Access);
+    if (Access == AS_public) return Sema::AR_accessible;
+    TriedElevation = true;
+  }
 
   // Protected access.
   if (Access == AS_protected) {
     // FIXME: implement [class.protected]p1
-    if (CurRecord->isDerivedFrom(NamingClass))
-      return false;
+    if (EC.Record->isDerivedFrom(NamingClass))
+      return Sema::AR_accessible;
 
-    // FIXME: dependent classes
+    // FIXME: delay dependent classes
   }
 
-  // FIXME: friends
-
-  // Okay, it's a bad access, reject it.
+  // We're about to reject;  one last chance to promote access.
+  if (!TriedElevation) {
+    TryElevateAccess(S, EC, Entity, Access);
+    if (Access == AS_public) return Sema::AR_accessible;
+  }
+    
+  // Okay, that's it, reject it.
+  if (ADK != Sema::ADK_quiet)
+    DiagnoseBadAccess(S, Loc, EC, NamingClass, Access, Entity, ADK);
+  return Sema::AR_inaccessible;
+}
 
-  
-  CXXRecordDecl *DeclaringClass = cast<CXXRecordDecl>(D->getDeclContext());
+static Sema::AccessResult CheckAccess(Sema &S, SourceLocation Loc,
+                                      const Sema::AccessedEntity &Entity,
+                                      Sema::AccessDiagnosticsKind ADK
+                                        = Sema::ADK_normal) {
+  // If the access path is public, it's accessible everywhere.
+  if (Entity.getAccess() == AS_public)
+    return Sema::AR_accessible;
 
-  if (Access == AS_protected) {
-    Diag(R.getNameLoc(), diag::err_access_protected)
-      << Context.getTypeDeclType(DeclaringClass)
-      << Context.getTypeDeclType(CurRecord);
-    DiagnoseAccessPath(*this, R, D, Access);
-    return true;
+  // 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) {
+    assert(ADK == Sema::ADK_normal && "delaying abnormal access check");
+    S.DelayedDiagnostics.push_back(
+        Sema::DelayedDiagnostic::makeAccess(Loc, Entity));
+    return Sema::AR_delayed;
   }
 
-  assert(Access == AS_private || Access == AS_none);
-  Diag(R.getNameLoc(), diag::err_access_private)
-    << Context.getTypeDeclType(DeclaringClass)
-    << Context.getTypeDeclType(CurRecord);
-  DiagnoseAccessPath(*this, R, D, Access);
-  return true;
+  return CheckEffectiveAccess(S, EffectiveContext(S.CurContext),
+                              Loc, Entity, ADK);
 }
 
 void Sema::HandleDelayedAccessCheck(DelayedDiagnostic &DD, Decl *Ctx) {
-  NamedDecl *D = DD.AccessData.Decl;
-
-  // Fake up a lookup result.
-  LookupResult R(*this, D->getDeclName(), DD.Loc, LookupOrdinaryName);
-  R.suppressDiagnostics();
-  R.setNamingClass(DD.AccessData.NamingClass);
-
   // Pretend we did this from the context of the newly-parsed
   // declaration.
-  DeclContext *EffectiveContext = Ctx->getDeclContext();
+  EffectiveContext EC(Ctx->getDeclContext());
 
-  if (CheckEffectiveAccess(EffectiveContext, R, D, DD.AccessData.Access))
+  if (CheckEffectiveAccess(*this, EC, DD.Loc, DD.AccessData, ADK_normal))
     DD.Triggered = true;
 }
 
-bool Sema::CheckUnresolvedLookupAccess(UnresolvedLookupExpr *E,
-                                       NamedDecl *D, AccessSpecifier Access) {
+Sema::AccessResult Sema::CheckUnresolvedLookupAccess(UnresolvedLookupExpr *E,
+                                                     NamedDecl *D,
+                                                     AccessSpecifier Access) {
   if (!getLangOptions().AccessControl || !E->getNamingClass())
-    return false;
-
-  // Fake up a lookup result.
-  LookupResult R(*this, E->getName(), E->getNameLoc(), LookupOrdinaryName);
-  R.suppressDiagnostics();
-
-  R.setNamingClass(E->getNamingClass());
-  R.addDecl(D, Access);
-
-  // FIXME: protected check (triggers for member-address expressions)
+    return AR_accessible;
 
-  return CheckAccess(R, D, Access);
+  return CheckAccess(*this, E->getNameLoc(),
+                 AccessedEntity::makeMember(E->getNamingClass(), Access, D));
 }
 
 /// Perform access-control checking on a previously-unresolved member
 /// access which has now been resolved to a member.
-bool Sema::CheckUnresolvedMemberAccess(UnresolvedMemberExpr *E,
-                                       NamedDecl *D, AccessSpecifier Access) {
+Sema::AccessResult Sema::CheckUnresolvedMemberAccess(UnresolvedMemberExpr *E,
+                                                     NamedDecl *D,
+                                                     AccessSpecifier Access) {
   if (!getLangOptions().AccessControl)
-    return false;
-
-  // Fake up a lookup result.
-  LookupResult R(*this, E->getMemberName(), E->getMemberLoc(),
-                 LookupOrdinaryName);
-  R.suppressDiagnostics();
-
-  R.setNamingClass(E->getNamingClass());
-  R.addDecl(D, Access);
+    return AR_accessible;
 
-  if (CheckAccess(R, D, Access))
-    return true;
-
-  // FIXME: protected check
-
-  return false;
+  return CheckAccess(*this, E->getMemberLoc(),
+                 AccessedEntity::makeMember(E->getNamingClass(), Access, D));
 }
 
-bool Sema::CheckDestructorAccess(SourceLocation Loc, const RecordType *RT) {
+Sema::AccessResult Sema::CheckDestructorAccess(SourceLocation Loc,
+                                               const RecordType *RT) {
   if (!getLangOptions().AccessControl)
-    return false;
+    return AR_accessible;
 
   CXXRecordDecl *NamingClass = cast<CXXRecordDecl>(RT->getDecl());
   CXXDestructorDecl *Dtor = NamingClass->getDestructor(Context);
 
   AccessSpecifier Access = Dtor->getAccess();
   if (Access == AS_public)
-    return false;
-
-  LookupResult R(*this, Dtor->getDeclName(), Loc, LookupOrdinaryName);
-  R.suppressDiagnostics();
+    return AR_accessible;
 
-  R.setNamingClass(NamingClass);
-  return CheckAccess(R, Dtor, Access);
-
-  // FIXME: protected check
+  return CheckAccess(*this, Loc,
+                 AccessedEntity::makeMember(NamingClass, Access, Dtor));
 }
 
 /// Checks access to a constructor.
-bool Sema::CheckConstructorAccess(SourceLocation UseLoc,
+Sema::AccessResult Sema::CheckConstructorAccess(SourceLocation UseLoc,
                                   CXXConstructorDecl *Constructor,
                                   AccessSpecifier Access) {
   if (!getLangOptions().AccessControl)
-    return false;
-
-  CXXRecordDecl *NamingClass = cast<CXXRecordDecl>(Constructor->getParent());
-
-  LookupResult R(*this, Constructor->getDeclName(), UseLoc, LookupOrdinaryName);
-  R.suppressDiagnostics();
+    return AR_accessible;
 
-  R.setNamingClass(NamingClass);
-  return CheckAccess(R, Constructor, Access);
+  CXXRecordDecl *NamingClass = Constructor->getParent();
+  return CheckAccess(*this, UseLoc,
+                 AccessedEntity::makeMember(NamingClass, Access, Constructor));
 }
 
 /// Checks access to an overloaded member operator, including
 /// conversion operators.
-bool Sema::CheckMemberOperatorAccess(SourceLocation OpLoc,
-                                     Expr *ObjectExpr,
-                                     NamedDecl *MemberOperator,
-                                     AccessSpecifier Access) {
+Sema::AccessResult Sema::CheckMemberOperatorAccess(SourceLocation OpLoc,
+                                                   Expr *ObjectExpr,
+                                                   NamedDecl *MemberOperator,
+                                                   AccessSpecifier Access) {
   if (!getLangOptions().AccessControl)
-    return false;
+    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());
 
-  LookupResult R(*this, DeclarationName(), OpLoc, LookupOrdinaryName);
-  R.suppressDiagnostics();
-
-  R.setNamingClass(NamingClass);
-  if (CheckAccess(R, MemberOperator, Access))
-    return true;
-
-  // FIXME: protected check
+  return CheckAccess(*this, OpLoc,
+            AccessedEntity::makeMember(NamingClass, Access, MemberOperator));
+}
 
-  return false;
+/// Checks access for a hierarchy conversion.
+///
+/// \param IsBaseToDerived whether this is a base-to-derived conversion (true)
+///     or a derived-to-base conversion (false)
+/// \param ForceCheck true if this check should be performed even if access
+///     control is disabled;  some things rely on this for semantics
+/// \param ForceUnprivileged true if this check should proceed as if the
+///     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,
+                                              bool ForceCheck,
+                                              bool ForceUnprivileged,
+                                              AccessDiagnosticsKind ADK) {
+  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);
+
+  if (ForceUnprivileged)
+    return CheckEffectiveAccess(*this, EffectiveContext(),
+                                AccessLoc, Entity, ADK);
+  return CheckAccess(*this, AccessLoc, Entity, ADK);
 }
 
 /// Checks access to all the declarations in the given result set.
-void Sema::CheckAccess(const LookupResult &R) {
+void Sema::CheckLookupAccess(const LookupResult &R) {
+  assert(getLangOptions().AccessControl
+         && "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)
-    CheckAccess(R, *I, I.getAccess());
+    if (I.getAccess() != AS_public)
+      CheckAccess(*this, R.getNameLoc(),
+                  AccessedEntity::makeMember(R.getNamingClass(),
+                                             I.getAccess(), *I));
 }
diff --git a/clang/lib/Sema/SemaCXXCast.cpp b/clang/lib/Sema/SemaCXXCast.cpp
index 63823054d85..48258ff6ad6 100644
--- a/clang/lib/Sema/SemaCXXCast.cpp
+++ b/clang/lib/Sema/SemaCXXCast.cpp
@@ -778,9 +778,10 @@ TryStaticDowncast(Sema &Self, CanQualType SrcType, CanQualType DestType,
     return TC_Failed;
   }
 
-  if (!CStyle && Self.CheckBaseClassAccess(DestType, SrcType,
-                          diag::err_downcast_from_inaccessible_base, Paths,
-                          OpRange.getBegin(), DeclarationName())) {
+  if (!CStyle && Self.CheckBaseClassAccess(OpRange.getBegin(),
+                                           /*IsBaseToDerived*/ true,
+                                           SrcType, DestType,
+                                           Paths.front())) {
     msg = 0;
     return TC_Failed;
   }
@@ -844,9 +845,10 @@ TryStaticMemberPointerUpcast(Sema &Self, QualType SrcType, QualType DestType,
     return TC_Failed;
   }
 
-  if (!CStyle && Self.CheckBaseClassAccess(DestType, SrcType,
-                          diag::err_downcast_from_inaccessible_base, Paths,
-                          OpRange.getBegin(), DeclarationName())) {
+  if (!CStyle && Self.CheckBaseClassAccess(OpRange.getBegin(),
+                                           /*IsBaseToDerived*/ false,
+                                           DestType, SrcType,
+                                           Paths.front())) {
     msg = 0;
     return TC_Failed;
   }
diff --git a/clang/lib/Sema/SemaDeclCXX.cpp b/clang/lib/Sema/SemaDeclCXX.cpp
index 67c08225b4e..b42a27cdcb2 100644
--- a/clang/lib/Sema/SemaDeclCXX.cpp
+++ b/clang/lib/Sema/SemaDeclCXX.cpp
@@ -713,7 +713,7 @@ bool Sema::IsDerivedFrom(QualType Derived, QualType Base, CXXBasePaths &Paths) {
 /// if there is an error.
 bool
 Sema::CheckDerivedToBaseConversion(QualType Derived, QualType Base,
-                                   unsigned InaccessibleBaseID,
+                                   AccessDiagnosticsKind ADK,
                                    unsigned AmbigiousBaseConvID,
                                    SourceLocation Loc, SourceRange Range,
                                    DeclarationName Name) {
@@ -729,11 +729,20 @@ Sema::CheckDerivedToBaseConversion(QualType Derived, QualType Base,
   (void)DerivationOkay;
   
   if (!Paths.isAmbiguous(Context.getCanonicalType(Base).getUnqualifiedType())) {
-    if (InaccessibleBaseID == 0)
+    if (ADK == ADK_quiet)
       return false;
+
     // Check that the base class can be accessed.
-    return CheckBaseClassAccess(Derived, Base, InaccessibleBaseID, Paths, Loc,
-                                Name);
+    switch (CheckBaseClassAccess(Loc, /*IsBaseToDerived*/ false,
+                                 Base, Derived, Paths.front(),
+                                 /*force*/ false,
+                                 /*unprivileged*/ false,
+                                 ADK)) {
+    case AR_accessible: return false;
+    case AR_inaccessible: return true;
+    case AR_dependent: return false;
+    case AR_delayed: return false;
+    }
   }
   
   // We know that the derived-to-base conversion is ambiguous, and
@@ -764,8 +773,7 @@ Sema::CheckDerivedToBaseConversion(QualType Derived, QualType Base,
                                    SourceLocation Loc, SourceRange Range,
                                    bool IgnoreAccess) {
   return CheckDerivedToBaseConversion(Derived, Base,
-                                      IgnoreAccess ? 0 :
-                                        diag::err_conv_to_inaccessible_base,
+                                      IgnoreAccess ? ADK_quiet : ADK_normal,
                                       diag::err_ambiguous_derived_to_base_conv,
                                       Loc, Range, DeclarationName());
 }
@@ -5626,8 +5634,7 @@ bool Sema::CheckOverridingFunctionReturnType(const CXXMethodDecl *New,
     }
 
     // Check if we the conversion from derived to base is valid.
-    if (CheckDerivedToBaseConversion(NewClassTy, OldClassTy,
-                      diag::err_covariant_return_inaccessible_base,
+    if (CheckDerivedToBaseConversion(NewClassTy, OldClassTy, ADK_covariance,
                       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/clang/lib/Sema/SemaExceptionSpec.cpp b/clang/lib/Sema/SemaExceptionSpec.cpp
index 7e2a98d0bf1..d0718d020b2 100644
--- a/clang/lib/Sema/SemaExceptionSpec.cpp
+++ b/clang/lib/Sema/SemaExceptionSpec.cpp
@@ -232,8 +232,22 @@ bool Sema::CheckExceptionSpecSubset(
       if (Paths.isAmbiguous(CanonicalSuperT))
         continue;
 
-      if (FindInaccessibleBase(CanonicalSubT, CanonicalSuperT, Paths, true))
-        continue;
+      // Do this check from a context without privileges.
+      switch (CheckBaseClassAccess(SourceLocation(), false,
+                                   CanonicalSuperT, CanonicalSubT,
+                                   Paths.front(),
+                                   /*ForceCheck*/ true,
+                                   /*ForceUnprivileged*/ true,
+                                   ADK_quiet)) {
+      case AR_accessible: break;
+      case AR_inaccessible: continue;
+      case AR_dependent:
+        llvm_unreachable("access check dependent for unprivileged context");
+        break;
+      case AR_delayed:
+        llvm_unreachable("access check delayed in non-declaration");
+        break;
+      }
 
       Contained = true;
       break;
diff --git a/clang/lib/Sema/SemaLookup.cpp b/clang/lib/Sema/SemaLookup.cpp
index 8cbcf127a43..0d95c713b01 100644
--- a/clang/lib/Sema/SemaLookup.cpp
+++ b/clang/lib/Sema/SemaLookup.cpp
@@ -404,7 +404,7 @@ void LookupResult::resolveKind() {
 }
 
 void LookupResult::addDeclsFromBasePaths(const CXXBasePaths &P) {
-  CXXBasePaths::paths_iterator I, E;
+  CXXBasePaths::const_paths_iterator I, E;
   DeclContext::lookup_iterator DI, DE;
   for (I = P.begin(), E = P.end(); I != E; ++I)
     for (llvm::tie(DI,DE) = I->Decls; DI != DE; ++DI)
diff --git a/clang/lib/Sema/SemaOverload.cpp b/clang/lib/Sema/SemaOverload.cpp
index 452901cf225..b79b1cc9937 100644
--- a/clang/lib/Sema/SemaOverload.cpp
+++ b/clang/lib/Sema/SemaOverload.cpp
@@ -1355,14 +1355,13 @@ bool Sema::IsMemberPointerConversion(Expr *From, QualType FromType,
   
 /// CheckMemberPointerConversion - Check the member pointer conversion from the
 /// expression From to the type ToType. This routine checks for ambiguous or
-/// virtual (FIXME: or inaccessible) base-to-derived member pointer conversions
+/// virtual or inaccessible base-to-derived member pointer conversions
 /// for which IsMemberPointerConversion has already returned true. It returns
 /// true and produces a diagnostic if there was an error, or returns false
 /// otherwise.
 bool Sema::CheckMemberPointerConversion(Expr *From, QualType ToType,
                                         CastExpr::CastKind &Kind,
                                         bool IgnoreBaseAccess) {
-  (void)IgnoreBaseAccess;
   QualType FromType = From->getType();
   const MemberPointerType *FromPtrType = FromType->getAs<MemberPointerType>();
   if (!FromPtrType) {
@@ -1385,7 +1384,7 @@ bool Sema::CheckMemberPointerConversion(Expr *From, QualType ToType,
   assert(FromClass->isRecordType() && "Pointer into non-class.");
   assert(ToClass->isRecordType() && "Pointer into non-class.");
 
-  CXXBasePaths Paths(/*FindAmbiguities=*/true, /*RecordPaths=*/false,
+  CXXBasePaths Paths(/*FindAmbiguities=*/true, /*RecordPaths=*/ true,
                      /*DetectVirtual=*/true);
   bool DerivationOkay = IsDerivedFrom(ToClass, FromClass, Paths);
   assert(DerivationOkay &&
@@ -1394,13 +1393,6 @@ bool Sema::CheckMemberPointerConversion(Expr *From, QualType ToType,
 
   if (Paths.isAmbiguous(Context.getCanonicalType(FromClass).
                                   getUnqualifiedType())) {
-    // Derivation is ambiguous. Redo the check to find the exact paths.
-    Paths.clear();
-    Paths.setRecordingPaths(true);
-    bool StillOkay = IsDerivedFrom(ToClass, FromClass, Paths);
-    assert(StillOkay && "Derivation changed due to quantum fluctuation.");
-    (void)StillOkay;
-
     std::string PathDisplayStr = getAmbiguousPathsDisplayString(Paths);
     Diag(From->getExprLoc(), diag::err_ambiguous_memptr_conv)
       << 0 << FromClass << ToClass << PathDisplayStr << From->getSourceRange();
@@ -1414,6 +1406,10 @@ bool Sema::CheckMemberPointerConversion(Expr *From, QualType ToType,
     return true;
   }
 
+  if (!IgnoreBaseAccess)
+    CheckBaseClassAccess(From->getExprLoc(), /*BaseToDerived*/ true,
+                         FromClass, ToClass, Paths.front());
+
   // Must be a base to derived member conversion.
   Kind = CastExpr::CK_BaseToDerivedMemberPointer;
   return false;