1 files changed, 202 insertions, 16 deletions
diff --git a/lib/Sema/SemaAccess.cpp b/lib/Sema/SemaAccess.cpp
index e110e3d..e629f0f 100644
--- a/lib/Sema/SemaAccess.cpp
+++ b/lib/Sema/SemaAccess.cpp
@@ -11,9 +11,10 @@
 //
 //===----------------------------------------------------------------------===//
 
-#include "Sema.h"
-#include "SemaInit.h"
-#include "Lookup.h"
+#include "clang/Sema/SemaInternal.h"
+#include "clang/Sema/DelayedDiagnostic.h"
+#include "clang/Sema/Initialization.h"
+#include "clang/Sema/Lookup.h"
 #include "clang/AST/ASTContext.h"
 #include "clang/AST/CXXInheritance.h"
 #include "clang/AST/DeclCXX.h"
@@ -22,6 +23,7 @@
 #include "clang/AST/ExprCXX.h"
 
 using namespace clang;
+using namespace sema;
 
 /// A copy of Sema's enum without AR_delayed.
 enum AccessResult {
@@ -132,10 +134,10 @@ struct EffectiveContext {
   bool Dependent;
 };
 
-/// Like Sema's AccessedEntity, but kindly lets us scribble all over
+/// Like sema:;AccessedEntity, but kindly lets us scribble all over
 /// it.
-struct AccessTarget : public Sema::AccessedEntity {
-  AccessTarget(const Sema::AccessedEntity &Entity)
+struct AccessTarget : public AccessedEntity {
+  AccessTarget(const AccessedEntity &Entity)
     : AccessedEntity(Entity) {
     initialize();
   }
@@ -562,6 +564,130 @@ static AccessResult GetFriendKind(Sema &S,
   return OnFailure;
 }
 
+namespace {
+
+/// A helper class for checking for a friend which will grant access
+/// to a protected instance member.
+struct ProtectedFriendContext {
+  Sema &S;
+  const EffectiveContext &EC;
+  const CXXRecordDecl *NamingClass;
+  bool CheckDependent;
+  bool EverDependent;
+
+  /// The path down to the current base class.
+  llvm::SmallVector<const CXXRecordDecl*, 20> CurPath;
+
+  ProtectedFriendContext(Sema &S, const EffectiveContext &EC,
+                         const CXXRecordDecl *InstanceContext,
+                         const CXXRecordDecl *NamingClass)
+    : S(S), EC(EC), NamingClass(NamingClass),
+      CheckDependent(InstanceContext->isDependentContext() ||
+                     NamingClass->isDependentContext()),
+      EverDependent(false) {}
+
+  /// Check classes in the current path for friendship, starting at
+  /// the given index.
+  bool checkFriendshipAlongPath(unsigned I) {
+    assert(I < CurPath.size());
+    for (unsigned E = CurPath.size(); I != E; ++I) {
+      switch (GetFriendKind(S, EC, CurPath[I])) {
+      case AR_accessible:   return true;
+      case AR_inaccessible: continue;
+      case AR_dependent:    EverDependent = true; continue;
+      }
+    }
+    return false;
+  }
+
+  /// Perform a search starting at the given class.
+  ///
+  /// PrivateDepth is the index of the last (least derived) class
+  /// along the current path such that a notional public member of
+  /// the final class in the path would have access in that class.
+  bool findFriendship(const CXXRecordDecl *Cur, unsigned PrivateDepth) {
+    // If we ever reach the naming class, check the current path for
+    // friendship.  We can also stop recursing because we obviously
+    // won't find the naming class there again.
+    if (Cur == NamingClass)
+      return checkFriendshipAlongPath(PrivateDepth);
+
+    if (CheckDependent && MightInstantiateTo(Cur, NamingClass))
+      EverDependent = true;
+
+    // Recurse into the base classes.
+    for (CXXRecordDecl::base_class_const_iterator
+           I = Cur->bases_begin(), E = Cur->bases_end(); I != E; ++I) {
+
+      // If this is private inheritance, then a public member of the
+      // base will not have any access in classes derived from Cur.
+      unsigned BasePrivateDepth = PrivateDepth;
+      if (I->getAccessSpecifier() == AS_private)
+        BasePrivateDepth = CurPath.size() - 1;
+
+      const CXXRecordDecl *RD;
+
+      QualType T = I->getType();
+      if (const RecordType *RT = T->getAs<RecordType>()) {
+        RD = cast<CXXRecordDecl>(RT->getDecl());
+      } else if (const InjectedClassNameType *IT
+                   = T->getAs<InjectedClassNameType>()) {
+        RD = IT->getDecl();
+      } else {
+        assert(T->isDependentType() && "non-dependent base wasn't a record?");
+        EverDependent = true;
+        continue;
+      }
+
+      // Recurse.  We don't need to clean up if this returns true.
+      CurPath.push_back(RD);
+      if (findFriendship(RD->getCanonicalDecl(), BasePrivateDepth))
+        return true;
+      CurPath.pop_back();
+    }
+
+    return false;
+  }
+
+  bool findFriendship(const CXXRecordDecl *Cur) {
+    assert(CurPath.empty());
+    CurPath.push_back(Cur);
+    return findFriendship(Cur, 0);
+  }
+};
+}
+
+/// Search for a class P that EC is a friend of, under the constraint
+///   InstanceContext <= P <= NamingClass
+/// and with the additional restriction that a protected member of
+/// NamingClass would have some natural access in P.
+///
+/// That second condition isn't actually quite right: the condition in
+/// the standard is whether the target would have some natural access
+/// in P.  The difference is that the target might be more accessible
+/// along some path not passing through NamingClass.  Allowing that
+/// introduces two problems:
+///   - It breaks encapsulation because you can suddenly access a
+///     forbidden base class's members by subclassing it elsewhere.
+///   - It makes access substantially harder to compute because it
+///     breaks the hill-climbing algorithm: knowing that the target is
+///     accessible in some base class would no longer let you change
+///     the question solely to whether the base class is accessible,
+///     because the original target might have been more accessible
+///     because of crazy subclassing.
+/// So we don't implement that.
+static AccessResult GetProtectedFriendKind(Sema &S, const EffectiveContext &EC,
+                                           const CXXRecordDecl *InstanceContext,
+                                           const CXXRecordDecl *NamingClass) {
+  assert(InstanceContext->getCanonicalDecl() == InstanceContext);
+  assert(NamingClass->getCanonicalDecl() == NamingClass);
+
+  ProtectedFriendContext PRC(S, EC, InstanceContext, NamingClass);
+  if (PRC.findFriendship(InstanceContext)) return AR_accessible;
+  if (PRC.EverDependent) return AR_dependent;
+  return AR_inaccessible;
+}
+
 static AccessResult HasAccess(Sema &S,
                               const EffectiveContext &EC,
                               const CXXRecordDecl *NamingClass,
@@ -629,20 +755,25 @@ static AccessResult HasAccess(Sema &S,
     }
   }
 
-  if (!NamingClass->hasFriends())
-    return OnFailure;
-
-  // Don't consider friends if we're under the [class.protected]
-  // restriction, above.
+  // [M3] and [B3] say that, if the target is protected in N, we grant
+  // access if the access occurs in a friend or member of some class P
+  // that's a subclass of N and where the target has some natural
+  // access in P.  The 'member' aspect is easy to handle because P
+  // would necessarily be one of the effective-context records, and we
+  // address that above.  The 'friend' aspect is completely ridiculous
+  // to implement because there are no restrictions at all on P
+  // *unless* the [class.protected] restriction applies.  If it does,
+  // however, we should ignore whether the naming class is a friend,
+  // and instead rely on whether any potential P is a friend.
   if (Access == AS_protected && Target.hasInstanceContext()) {
     const CXXRecordDecl *InstanceContext = Target.resolveInstanceContext(S);
     if (!InstanceContext) return AR_dependent;
-
-    switch (IsDerivedFromInclusive(InstanceContext, NamingClass)) {
-    case AR_accessible: break;
+    switch (GetProtectedFriendKind(S, EC, InstanceContext, NamingClass)) {
+    case AR_accessible: return AR_accessible;
     case AR_inaccessible: return OnFailure;
     case AR_dependent: return AR_dependent;
     }
+    llvm_unreachable("impossible friendship kind");
   }
 
   switch (GetFriendKind(S, EC, NamingClass)) {
@@ -799,6 +930,57 @@ static CXXBasePath *FindBestPath(Sema &S,
   return BestPath;
 }
 
+/// Given that an entity has protected natural access, check whether
+/// access might be denied because of the protected member access
+/// restriction.
+///
+/// \return true if a note was emitted
+static bool TryDiagnoseProtectedAccess(Sema &S, const EffectiveContext &EC,
+                                       AccessTarget &Target) {
+  // Only applies to instance accesses.
+  if (!Target.hasInstanceContext())
+    return false;
+  assert(Target.isMemberAccess());
+  NamedDecl *D = Target.getTargetDecl();
+
+  const CXXRecordDecl *DeclaringClass = Target.getDeclaringClass();
+  DeclaringClass = DeclaringClass->getCanonicalDecl();
+
+  for (EffectiveContext::record_iterator
+         I = EC.Records.begin(), E = EC.Records.end(); I != E; ++I) {
+    const CXXRecordDecl *ECRecord = *I;
+    switch (IsDerivedFromInclusive(ECRecord, DeclaringClass)) {
+    case AR_accessible: break;
+    case AR_inaccessible: continue;
+    case AR_dependent: continue;
+    }
+
+    // The effective context is a subclass of the declaring class.
+    // If that class isn't a superclass of the instance context,
+    // then the [class.protected] restriction applies.
+
+    // To get this exactly right, this might need to be checked more
+    // holistically;  it's not necessarily the case that gaining
+    // access here would grant us access overall.
+
+    const CXXRecordDecl *InstanceContext = Target.resolveInstanceContext(S);
+    assert(InstanceContext && "diagnosing dependent access");
+
+    switch (IsDerivedFromInclusive(InstanceContext, ECRecord)) {
+    case AR_accessible: continue;
+    case AR_dependent: continue;
+    case AR_inaccessible:
+      S.Diag(D->getLocation(), diag::note_access_protected_restricted)
+        << (InstanceContext != Target.getNamingClass()->getCanonicalDecl())
+        << S.Context.getTypeDeclType(InstanceContext)
+        << S.Context.getTypeDeclType(ECRecord);
+      return true;
+    }
+  }
+
+  return false;
+}
+
 /// Diagnose the path which caused the given declaration or base class
 /// to become inaccessible.
 static void DiagnoseAccessPath(Sema &S,
@@ -817,6 +999,10 @@ static void DiagnoseAccessPath(Sema &S,
   if (D && (Access == D->getAccess() || D->getAccess() == AS_private)) {
     switch (HasAccess(S, EC, DeclaringClass, D->getAccess(), Entity)) {
     case AR_inaccessible: {
+      if (Access == AS_protected &&
+          TryDiagnoseProtectedAccess(S, EC, Entity))
+        return;
+
       S.Diag(D->getLocation(), diag::note_access_natural)
         << (unsigned) (Access == AS_protected)
         << /*FIXME: not implicitly*/ 0;
@@ -1033,7 +1219,7 @@ static Sema::AccessResult CheckAccess(Sema &S, SourceLocation Loc,
   // access control.
   if (S.CurContext->isFileContext() && S.ParsingDeclDepth) {
     S.DelayedDiagnostics.push_back(
-        Sema::DelayedDiagnostic::makeAccess(Loc, Entity));
+        DelayedDiagnostic::makeAccess(Loc, Entity));
     return Sema::AR_delayed;
   }
 
@@ -1266,7 +1452,7 @@ Sema::AccessResult Sema::CheckAddressOfMemberAccess(Expr *OvlExpr,
       Found.getAccess() == AS_public)
     return AR_accessible;
 
-  OverloadExpr *Ovl = OverloadExpr::find(OvlExpr).getPointer();
+  OverloadExpr *Ovl = OverloadExpr::find(OvlExpr).Expression;
   CXXRecordDecl *NamingClass = Ovl->getNamingClass();
 
   AccessTarget Entity(Context, AccessTarget::Member, NamingClass, Found,