|
@@ -165,6 +165,10 @@ struct AccessTarget : public AccessedEntity {
|
|
|
initialize();
|
|
|
}
|
|
|
|
|
|
+ bool isInstanceMember() const {
|
|
|
+ return (isMemberAccess() && getTargetDecl()->isCXXInstanceMember());
|
|
|
+ }
|
|
|
+
|
|
|
bool hasInstanceContext() const {
|
|
|
return HasInstanceContext;
|
|
|
}
|
|
@@ -671,18 +675,25 @@ struct ProtectedFriendContext {
|
|
|
}
|
|
|
|
|
|
/// Search for a class P that EC is a friend of, under the constraint
|
|
|
-/// InstanceContext <= P <= NamingClass
|
|
|
+/// InstanceContext <= P
|
|
|
+/// if InstanceContext exists, or else
|
|
|
+/// NamingClass <= P
|
|
|
/// and with the additional restriction that a protected member of
|
|
|
-/// NamingClass would have some natural access in P.
|
|
|
+/// NamingClass would have some natural access in P, which implicitly
|
|
|
+/// imposes the constraint that P <= NamingClass.
|
|
|
///
|
|
|
-/// 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
|
|
|
+/// This isn't quite the condition laid out in the standard.
|
|
|
+/// Instead of saying that a notional protected member of NamingClass
|
|
|
+/// would have to have some natural access in P, it says the actual
|
|
|
+/// target has to have some natural access in P, which opens up the
|
|
|
+/// possibility that the target (which is not necessarily a member
|
|
|
+/// of NamingClass) might be more accessible along some path not
|
|
|
+/// passing through it. That's really a bad idea, though, because it
|
|
|
/// 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
|
|
|
+/// - Most importantly, it breaks encapsulation because you can
|
|
|
+/// access a forbidden base class's members by directly subclassing
|
|
|
+/// it elsewhere.
|
|
|
+/// - It also 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,
|
|
@@ -692,9 +703,15 @@ struct ProtectedFriendContext {
|
|
|
static AccessResult GetProtectedFriendKind(Sema &S, const EffectiveContext &EC,
|
|
|
const CXXRecordDecl *InstanceContext,
|
|
|
const CXXRecordDecl *NamingClass) {
|
|
|
- assert(InstanceContext->getCanonicalDecl() == InstanceContext);
|
|
|
+ assert(InstanceContext == 0 ||
|
|
|
+ InstanceContext->getCanonicalDecl() == InstanceContext);
|
|
|
assert(NamingClass->getCanonicalDecl() == NamingClass);
|
|
|
|
|
|
+ // If we don't have an instance context, our constraints give us
|
|
|
+ // that NamingClass <= P <= NamingClass, i.e. P == NamingClass.
|
|
|
+ // This is just the usual friendship check.
|
|
|
+ if (!InstanceContext) return GetFriendKind(S, EC, NamingClass);
|
|
|
+
|
|
|
ProtectedFriendContext PRC(S, EC, InstanceContext, NamingClass);
|
|
|
if (PRC.findFriendship(InstanceContext)) return AR_accessible;
|
|
|
if (PRC.EverDependent) return AR_dependent;
|
|
@@ -737,15 +754,6 @@ static AccessResult HasAccess(Sema &S,
|
|
|
case AR_dependent: OnFailure = AR_dependent; continue;
|
|
|
}
|
|
|
|
|
|
- if (!Target.hasInstanceContext())
|
|
|
- return AR_accessible;
|
|
|
-
|
|
|
- const CXXRecordDecl *InstanceContext = Target.resolveInstanceContext(S);
|
|
|
- if (!InstanceContext) {
|
|
|
- OnFailure = AR_dependent;
|
|
|
- continue;
|
|
|
- }
|
|
|
-
|
|
|
// C++ [class.protected]p1:
|
|
|
// An additional access check beyond those described earlier in
|
|
|
// [class.access] is applied when a non-static data member or
|
|
@@ -758,8 +766,49 @@ static AccessResult HasAccess(Sema &S,
|
|
|
// expression. In this case, the class of the object expression
|
|
|
// shall be C or a class derived from C.
|
|
|
//
|
|
|
- // We interpret this as a restriction on [M3]. Most of the
|
|
|
- // conditions are encoded by not having any instance context.
|
|
|
+ // We interpret this as a restriction on [M3].
|
|
|
+
|
|
|
+ // In this part of the code, 'C' is just our context class ECRecord.
|
|
|
+
|
|
|
+ // These rules are different if we don't have an instance context.
|
|
|
+ if (!Target.hasInstanceContext()) {
|
|
|
+ // If it's not an instance member, these restrictions don't apply.
|
|
|
+ if (!Target.isInstanceMember()) return AR_accessible;
|
|
|
+
|
|
|
+ // If it's an instance member, use the pointer-to-member rule
|
|
|
+ // that the naming class has to be derived from the effective
|
|
|
+ // context.
|
|
|
+
|
|
|
+ // Despite the standard's confident wording, there is a case
|
|
|
+ // where you can have an instance member that's neither in a
|
|
|
+ // pointer-to-member expression nor in a member access: when
|
|
|
+ // it names a field in an unevaluated context that can't be an
|
|
|
+ // implicit member. Pending clarification, we just apply the
|
|
|
+ // same naming-class restriction here.
|
|
|
+ // FIXME: we're probably not correctly adding the
|
|
|
+ // protected-member restriction when we retroactively convert
|
|
|
+ // an expression to being evaluated.
|
|
|
+
|
|
|
+ // We know that ECRecord derives from NamingClass. The
|
|
|
+ // restriction says to check whether NamingClass derives from
|
|
|
+ // ECRecord, but that's not really necessary: two distinct
|
|
|
+ // classes can't be recursively derived from each other. So
|
|
|
+ // along this path, we just need to check whether the classes
|
|
|
+ // are equal.
|
|
|
+ if (NamingClass == ECRecord) return AR_accessible;
|
|
|
+
|
|
|
+ // Otherwise, this context class tells us nothing; on to the next.
|
|
|
+ continue;
|
|
|
+ }
|
|
|
+
|
|
|
+ assert(Target.isInstanceMember());
|
|
|
+
|
|
|
+ const CXXRecordDecl *InstanceContext = Target.resolveInstanceContext(S);
|
|
|
+ if (!InstanceContext) {
|
|
|
+ OnFailure = AR_dependent;
|
|
|
+ continue;
|
|
|
+ }
|
|
|
+
|
|
|
switch (IsDerivedFromInclusive(InstanceContext, ECRecord)) {
|
|
|
case AR_accessible: return AR_accessible;
|
|
|
case AR_inaccessible: continue;
|
|
@@ -778,9 +827,14 @@ static AccessResult HasAccess(Sema &S,
|
|
|
// *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;
|
|
|
+ if (Access == AS_protected && Target.isInstanceMember()) {
|
|
|
+ // Compute the instance context if possible.
|
|
|
+ const CXXRecordDecl *InstanceContext = 0;
|
|
|
+ if (Target.hasInstanceContext()) {
|
|
|
+ InstanceContext = Target.resolveInstanceContext(S);
|
|
|
+ if (!InstanceContext) return AR_dependent;
|
|
|
+ }
|
|
|
+
|
|
|
switch (GetProtectedFriendKind(S, EC, InstanceContext, NamingClass)) {
|
|
|
case AR_accessible: return AR_accessible;
|
|
|
case AR_inaccessible: return OnFailure;
|
|
@@ -950,31 +1004,46 @@ static CXXBasePath *FindBestPath(Sema &S,
|
|
|
static bool TryDiagnoseProtectedAccess(Sema &S, const EffectiveContext &EC,
|
|
|
AccessTarget &Target) {
|
|
|
// Only applies to instance accesses.
|
|
|
- if (!Target.hasInstanceContext())
|
|
|
+ if (!Target.isInstanceMember())
|
|
|
return false;
|
|
|
+
|
|
|
assert(Target.isMemberAccess());
|
|
|
- NamedDecl *D = Target.getTargetDecl();
|
|
|
|
|
|
- const CXXRecordDecl *DeclaringClass = Target.getDeclaringClass();
|
|
|
- DeclaringClass = DeclaringClass->getCanonicalDecl();
|
|
|
+ const CXXRecordDecl *NamingClass = Target.getNamingClass();
|
|
|
+ NamingClass = NamingClass->getCanonicalDecl();
|
|
|
|
|
|
for (EffectiveContext::record_iterator
|
|
|
I = EC.Records.begin(), E = EC.Records.end(); I != E; ++I) {
|
|
|
const CXXRecordDecl *ECRecord = *I;
|
|
|
- switch (IsDerivedFromInclusive(ECRecord, DeclaringClass)) {
|
|
|
+ switch (IsDerivedFromInclusive(ECRecord, NamingClass)) {
|
|
|
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.
|
|
|
+ // Check whether the [class.protected] restriction is limiting
|
|
|
+ // access.
|
|
|
|
|
|
// 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.
|
|
|
|
|
|
+ NamedDecl *D = Target.getTargetDecl();
|
|
|
+
|
|
|
+ // If we don't have an instance context, [class.protected] says the
|
|
|
+ // naming class has to equal the context class.
|
|
|
+ if (!Target.hasInstanceContext()) {
|
|
|
+ // If it does, the restriction doesn't apply.
|
|
|
+ if (NamingClass == ECRecord) continue;
|
|
|
+
|
|
|
+ // TODO: it would be great to have a fixit here, since this is
|
|
|
+ // such an obvious error.
|
|
|
+ S.Diag(D->getLocation(), diag::note_access_protected_restricted_noobject)
|
|
|
+ << S.Context.getTypeDeclType(ECRecord);
|
|
|
+ return true;
|
|
|
+ }
|
|
|
+
|
|
|
const CXXRecordDecl *InstanceContext = Target.resolveInstanceContext(S);
|
|
|
assert(InstanceContext && "diagnosing dependent access");
|
|
|
|
|
@@ -982,12 +1051,25 @@ static bool TryDiagnoseProtectedAccess(Sema &S, const EffectiveContext &EC,
|
|
|
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);
|
|
|
+ break;
|
|
|
+ }
|
|
|
+
|
|
|
+ // Okay, the restriction seems to be what's limiting us.
|
|
|
+
|
|
|
+ // Use a special diagnostic for constructors and destructors.
|
|
|
+ if (isa<CXXConstructorDecl>(D) || isa<CXXDestructorDecl>(D) ||
|
|
|
+ (isa<FunctionTemplateDecl>(D) &&
|
|
|
+ isa<CXXConstructorDecl>(
|
|
|
+ cast<FunctionTemplateDecl>(D)->getTemplatedDecl()))) {
|
|
|
+ S.Diag(D->getLocation(), diag::note_access_protected_restricted_ctordtor)
|
|
|
+ << isa<CXXDestructorDecl>(D);
|
|
|
return true;
|
|
|
}
|
|
|
+
|
|
|
+ // Otherwise, use the generic diagnostic.
|
|
|
+ S.Diag(D->getLocation(), diag::note_access_protected_restricted_object)
|
|
|
+ << S.Context.getTypeDeclType(ECRecord);
|
|
|
+ return true;
|
|
|
}
|
|
|
|
|
|
return false;
|
|
@@ -1427,7 +1509,8 @@ Sema::AccessResult Sema::CheckUnresolvedMemberAccess(UnresolvedMemberExpr *E,
|
|
|
|
|
|
Sema::AccessResult Sema::CheckDestructorAccess(SourceLocation Loc,
|
|
|
CXXDestructorDecl *Dtor,
|
|
|
- const PartialDiagnostic &PDiag) {
|
|
|
+ const PartialDiagnostic &PDiag,
|
|
|
+ QualType ObjectTy) {
|
|
|
if (!getLangOpts().AccessControl)
|
|
|
return AR_accessible;
|
|
|
|
|
@@ -1437,9 +1520,11 @@ Sema::AccessResult Sema::CheckDestructorAccess(SourceLocation Loc,
|
|
|
return AR_accessible;
|
|
|
|
|
|
CXXRecordDecl *NamingClass = Dtor->getParent();
|
|
|
+ if (ObjectTy.isNull()) ObjectTy = Context.getTypeDeclType(NamingClass);
|
|
|
+
|
|
|
AccessTarget Entity(Context, AccessTarget::Member, NamingClass,
|
|
|
DeclAccessPair::make(Dtor, Access),
|
|
|
- QualType());
|
|
|
+ ObjectTy);
|
|
|
Entity.setDiag(PDiag); // TODO: avoid copy
|
|
|
|
|
|
return CheckAccess(*this, Loc, Entity);
|
|
@@ -1451,14 +1536,9 @@ Sema::AccessResult Sema::CheckConstructorAccess(SourceLocation UseLoc,
|
|
|
const InitializedEntity &Entity,
|
|
|
AccessSpecifier Access,
|
|
|
bool IsCopyBindingRefToTemp) {
|
|
|
- if (!getLangOpts().AccessControl ||
|
|
|
- Access == AS_public)
|
|
|
+ if (!getLangOpts().AccessControl || Access == AS_public)
|
|
|
return AR_accessible;
|
|
|
|
|
|
- CXXRecordDecl *NamingClass = Constructor->getParent();
|
|
|
- AccessTarget AccessEntity(Context, AccessTarget::Member, NamingClass,
|
|
|
- DeclAccessPair::make(Constructor, Access),
|
|
|
- QualType());
|
|
|
PartialDiagnostic PD(PDiag());
|
|
|
switch (Entity.getKind()) {
|
|
|
default:
|
|
@@ -1490,26 +1570,38 @@ Sema::AccessResult Sema::CheckConstructorAccess(SourceLocation UseLoc,
|
|
|
|
|
|
}
|
|
|
|
|
|
- return CheckConstructorAccess(UseLoc, Constructor, Access, PD);
|
|
|
+ return CheckConstructorAccess(UseLoc, Constructor, Entity, Access, PD);
|
|
|
}
|
|
|
|
|
|
/// Checks access to a constructor.
|
|
|
Sema::AccessResult Sema::CheckConstructorAccess(SourceLocation UseLoc,
|
|
|
CXXConstructorDecl *Constructor,
|
|
|
+ const InitializedEntity &Entity,
|
|
|
AccessSpecifier Access,
|
|
|
- PartialDiagnostic PD) {
|
|
|
+ const PartialDiagnostic &PD) {
|
|
|
if (!getLangOpts().AccessControl ||
|
|
|
Access == AS_public)
|
|
|
return AR_accessible;
|
|
|
|
|
|
CXXRecordDecl *NamingClass = Constructor->getParent();
|
|
|
+
|
|
|
+ // Initializing a base sub-object is an instance method call on an
|
|
|
+ // object of the derived class. Otherwise, we have an instance method
|
|
|
+ // call on an object of the constructed type.
|
|
|
+ CXXRecordDecl *ObjectClass;
|
|
|
+ if (Entity.getKind() == InitializedEntity::EK_Base) {
|
|
|
+ ObjectClass = cast<CXXConstructorDecl>(CurContext)->getParent();
|
|
|
+ } else {
|
|
|
+ ObjectClass = NamingClass;
|
|
|
+ }
|
|
|
+
|
|
|
AccessTarget AccessEntity(Context, AccessTarget::Member, NamingClass,
|
|
|
DeclAccessPair::make(Constructor, Access),
|
|
|
- QualType());
|
|
|
+ Context.getTypeDeclType(ObjectClass));
|
|
|
AccessEntity.setDiag(PD);
|
|
|
|
|
|
return CheckAccess(*this, UseLoc, AccessEntity);
|
|
|
-}
|
|
|
+}
|
|
|
|
|
|
/// Checks direct (i.e. non-inherited) access to an arbitrary class
|
|
|
/// member.
|
|
@@ -1583,7 +1675,7 @@ Sema::AccessResult Sema::CheckAddressOfMemberAccess(Expr *OvlExpr,
|
|
|
CXXRecordDecl *NamingClass = Ovl->getNamingClass();
|
|
|
|
|
|
AccessTarget Entity(Context, AccessTarget::Member, NamingClass, Found,
|
|
|
- Context.getTypeDeclType(NamingClass));
|
|
|
+ /*no instance context*/ QualType());
|
|
|
Entity.setDiag(diag::err_access)
|
|
|
<< Ovl->getSourceRange();
|
|
|
|