diff options
Diffstat (limited to 'lib')
| -rw-r--r-- | lib/Sema/SemaAccess.cpp | 188 | ||||
| -rw-r--r-- | lib/Sema/SemaDeclCXX.cpp | 10 | ||||
| -rw-r--r-- | lib/Sema/SemaExprCXX.cpp | 5 | ||||
| -rw-r--r-- | lib/Sema/SemaInit.cpp | 2 |
4 files changed, 153 insertions, 52 deletions
diff --git a/lib/Sema/SemaAccess.cpp b/lib/Sema/SemaAccess.cpp index ea74346c16..74c4f34d75 100644 --- a/lib/Sema/SemaAccess.cpp +++ b/lib/Sema/SemaAccess.cpp @@ -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(); diff --git a/lib/Sema/SemaDeclCXX.cpp b/lib/Sema/SemaDeclCXX.cpp index fb34126943..975ea5b7a7 100644 --- a/lib/Sema/SemaDeclCXX.cpp +++ b/lib/Sema/SemaDeclCXX.cpp @@ -3344,7 +3344,8 @@ Sema::MarkBaseAndMemberDestructorsReferenced(SourceLocation Location, CheckDestructorAccess(Base->getLocStart(), Dtor, PDiag(diag::err_access_dtor_base) << Base->getType() - << Base->getSourceRange()); + << Base->getSourceRange(), + Context.getTypeDeclType(ClassDecl)); MarkFunctionReferenced(Location, const_cast<CXXDestructorDecl*>(Dtor)); DiagnoseUseOfDecl(Dtor, Location); @@ -6277,6 +6278,13 @@ NamedDecl *Sema::BuildUsingDeclaration(Scope *S, AccessSpecifier AS, if (!IsInstantiation) R.setHideTags(false); + // For the purposes of this lookup, we have a base object type + // equal to that of the current context. + if (CurContext->isRecord()) { + R.setBaseObjectType( + Context.getTypeDeclType(cast<CXXRecordDecl>(CurContext))); + } + LookupQualifiedName(R, LookupContext); if (R.empty()) { diff --git a/lib/Sema/SemaExprCXX.cpp b/lib/Sema/SemaExprCXX.cpp index 4692bf8b4b..41f2116600 100644 --- a/lib/Sema/SemaExprCXX.cpp +++ b/lib/Sema/SemaExprCXX.cpp @@ -2276,8 +2276,9 @@ static ExprResult BuildCXXCastArgument(Sema &S, CastLoc, ConstructorArgs)) return ExprError(); - S.CheckConstructorAccess(CastLoc, Constructor, Constructor->getAccess(), - S.PDiag(diag::err_access_ctor)); + S.CheckConstructorAccess(CastLoc, Constructor, + InitializedEntity::InitializeTemporary(Ty), + Constructor->getAccess()); ExprResult Result = S.BuildCXXConstructExpr(CastLoc, Ty, cast<CXXConstructorDecl>(Method), diff --git a/lib/Sema/SemaInit.cpp b/lib/Sema/SemaInit.cpp index be48c7ea18..ee051428da 100644 --- a/lib/Sema/SemaInit.cpp +++ b/lib/Sema/SemaInit.cpp @@ -4584,7 +4584,7 @@ static void CheckCXX98CompatAccessibleCopy(Sema &S, switch (OR) { case OR_Success: S.CheckConstructorAccess(Loc, cast<CXXConstructorDecl>(Best->Function), - Best->FoundDecl.getAccess(), Diag); + Entity, Best->FoundDecl.getAccess(), Diag); // FIXME: Check default arguments as far as that's possible. break; |