averello/swift-mirror
1
0
Fork 0
mirror of https://github.com/apple/swift.git synced 2025-12-14 20:36:38 +01:00
Code Issues Packages Projects Releases Wiki Activity

Check protocol witnesses using access scopes. (#4176)

Browse Source 
...rather than relying on the access-as-spelled, which may be greater
than the effective access due to parent scopes.

(Some of this will get cleaned up with SR-2209.)

rdar://problem/27663492
This commit is contained in:
Jordan Rose
2016-08-11 14:30:09 -07:00
committed by GitHub
parent 84d4685ad4
commit f65ad810df
6 changed files with 155 additions and 78 deletions
Download Patch File Download Diff File Expand all files Collapse all files

32
lib/Sema/MiscDiagnostics.cpp
View File

@@ -3606,6 +3606,20 @@ void swift::performStmtDiagnostics(TypeChecker &TC, const Stmt *S) {
// Utility functions
//===----------------------------------------------------------------------===//
Accessibility
swift::accessibilityFromScopeForDiagnostics(const DeclContext *accessScope) {
if (!accessScope)
return Accessibility::Public;
if (isa<ModuleDecl>(accessScope))
return Accessibility::Internal;
if (accessScope->isModuleScopeContext() &&
accessScope->getASTContext().LangOpts.EnableSwift3Private) {
return Accessibility::FilePrivate;
}
return Accessibility::Private;
}
void swift::fixItAccessibility(InFlightDiagnostic &diag, ValueDecl *VD,
Accessibility desiredAccess, bool isForSetter) {
StringRef fixItString;
@@ -3618,7 +3632,7 @@ void swift::fixItAccessibility(InFlightDiagnostic &diag, ValueDecl *VD,
}
DeclAttributes &attrs = VD->getAttrs();
DeclAttribute *attr;
AbstractAccessibilityAttr *attr;
if (isForSetter) {
attr = attrs.getAttribute<SetterAccessibilityAttr>();
cast<AbstractStorageDecl>(VD)->overwriteSetterAccessibility(desiredAccess);
@@ -3646,10 +3660,18 @@ void swift::fixItAccessibility(InFlightDiagnostic &diag, ValueDecl *VD,
diag.fixItRemove(attr->Range);
} else if (attr) {
// This uses getLocation() instead of getRange() because we don't want to
// replace the "(set)" part of a setter attribute.
diag.fixItReplace(attr->getLocation(), fixItString.drop_back());
attr->setInvalid();
// If the formal access already matches the desired access, the problem
// must be in a parent scope. Don't emit a fix-it.
// FIXME: It's also possible for access to already be /broader/ than what's
// desired, in which case the problem is also in a parent scope. However,
// this function is sometimes called to make access narrower, so assuming
// that a broader scope is acceptable breaks some diagnostics.
if (attr->getAccess() != desiredAccess) {
// This uses getLocation() instead of getRange() because we don't want to
// replace the "(set)" part of a setter attribute.
diag.fixItReplace(attr->getLocation(), fixItString.drop_back());
attr->setInvalid();
}
} else if (auto var = dyn_cast<VarDecl>(VD)) {
if (auto PBD = var->getParentPatternBinding())

7
lib/Sema/MiscDiagnostics.h
View File

@@ -31,6 +31,13 @@ namespace swift {
class TypeChecker;
class ValueDecl;
/// Returns the access level associated with \p accessScope, for diagnostic
/// purposes.
///
/// \sa ValueDecl::getFormalAccessScope
Accessibility
accessibilityFromScopeForDiagnostics(const DeclContext *accessScope);
/// \brief Emit diagnostics for syntactic restrictions on a given expression.
void performSyntacticExprDiagnostics(TypeChecker &TC, const Expr *E,
const DeclContext *DC,

17
lib/Sema/TypeCheckDecl.cpp
View File

@@ -1538,23 +1538,6 @@ static void highlightOffendingType(TypeChecker &TC, InFlightDiagnostic &diag,
}
}
/// Returns the access level associated with \p accessScope, for diagnostic
/// purposes.
///
/// \sa ValueDecl::getFormalAccessScope
static Accessibility
accessibilityFromScopeForDiagnostics(const DeclContext *accessScope) {
if (!accessScope)
return Accessibility::Public;
if (isa<ModuleDecl>(accessScope))
return Accessibility::Internal;
if (accessScope->isModuleScopeContext() &&
accessScope->getASTContext().LangOpts.EnableSwift3Private) {
return Accessibility::FilePrivate;
}
return Accessibility::Private;
}
static void checkGenericParamAccessibility(TypeChecker &TC,
const GenericParamList *params,
const Decl *owner,

101
lib/Sema/TypeCheckProtocol.cpp
View File

@@ -68,7 +68,7 @@ namespace {
unsigned &bestIdx,
bool &doNotDiagnoseMatches);
bool checkWitnessAccessibility(Accessibility *requiredAccess,
bool checkWitnessAccessibility(const DeclContext *&requiredAccessScope,
ValueDecl *requirement,
ValueDecl *witness,
bool *isSetter);
@@ -77,7 +77,7 @@ namespace {
ValueDecl *witness,
AvailabilityContext *requirementInfo);
RequirementCheck checkWitness(Accessibility requiredAccess,
RequirementCheck checkWitness(const DeclContext *requiredAccessScope,
ValueDecl *requirement,
RequirementMatch match);
};
@@ -392,24 +392,24 @@ namespace {
struct RequirementCheck {
CheckKind Kind;
/// The required accessibility, if the check failed due to the
/// The required access scope, if the check failed due to the
/// witness being less accessible than the requirement.
Accessibility RequiredAccess;
const DeclContext *RequiredAccessScope;
/// The required availability, if the check failed due to the
/// witness being less available than the requirement.
AvailabilityContext RequiredAvailability;
RequirementCheck(CheckKind kind)
: Kind(kind), RequiredAccess(Accessibility::Public),
: Kind(kind), RequiredAccessScope(nullptr),
RequiredAvailability(AvailabilityContext::alwaysAvailable()) { }
RequirementCheck(CheckKind kind, Accessibility requiredAccess)
: Kind(kind), RequiredAccess(requiredAccess),
RequirementCheck(CheckKind kind, const DeclContext *requiredAccessScope)
: Kind(kind), RequiredAccessScope(requiredAccessScope),
RequiredAvailability(AvailabilityContext::alwaysAvailable()) { }
RequirementCheck(CheckKind kind, AvailabilityContext requiredAvailability)
: Kind(kind), RequiredAccess(Accessibility::Public),
: Kind(kind), RequiredAccessScope(nullptr),
RequiredAvailability(requiredAvailability) { }
};
}
@@ -1218,48 +1218,35 @@ bool WitnessChecker::findBestWitness(ValueDecl *requirement,
}
bool WitnessChecker::
checkWitnessAccessibility(Accessibility *requiredAccess,
checkWitnessAccessibility(const DeclContext *&requiredAccessScope,
ValueDecl *requirement,
ValueDecl *witness,
bool *isSetter) {
*isSetter = false;
*requiredAccess = std::min(Proto->getFormalAccess(), *requiredAccess);
if (TC.getLangOpts().EnableSwift3Private)
*requiredAccess = std::max(*requiredAccess, Accessibility::FilePrivate);
const DeclContext *protoAccessScope = Proto->getFormalAccessScope(DC);
Accessibility witnessAccess = witness->getFormalAccess(DC);
// Leave a hole for old-style top-level operators to be declared 'private' for
// a fileprivate conformance.
if (witnessAccess == Accessibility::Private &&
witness->getDeclContext()->isModuleScopeContext()) {
witnessAccess = Accessibility::FilePrivate;
// FIXME: This is the same operation as TypeCheckDecl.cpp's
// TypeAccessScopeChecker::intersectAccess.
if (!requiredAccessScope) {
requiredAccessScope = protoAccessScope;
} else if (protoAccessScope) {
if (protoAccessScope->isChildContextOf(requiredAccessScope)) {
requiredAccessScope = protoAccessScope;
} else {
assert(requiredAccessScope == protoAccessScope ||
requiredAccessScope->isChildContextOf(protoAccessScope));
}
}
if (witnessAccess < *requiredAccess)
if (!witness->isAccessibleFrom(requiredAccessScope))
return true;
if (requirement->isSettable(DC)) {
*isSetter = true;
auto ASD = cast<AbstractStorageDecl>(witness);
const DeclContext *accessDC;
switch (*requiredAccess) {
case Accessibility::Open:
case Accessibility::Public:
accessDC = nullptr;
break;
case Accessibility::Internal:
accessDC = DC->getParentModule();
break;
case Accessibility::FilePrivate:
case Accessibility::Private:
accessDC = DC->getModuleScopeContext();
break;
}
if (!ASD->isSetterAccessibleFrom(accessDC))
if (!ASD->isSetterAccessibleFrom(requiredAccessScope))
return true;
}
@@ -1276,19 +1263,19 @@ checkWitnessAvailability(ValueDecl *requirement,
}
RequirementCheck WitnessChecker::
checkWitness(Accessibility requiredAccess,
checkWitness(const DeclContext *requiredAccessScope,
ValueDecl *requirement,
RequirementMatch match) {
if (!match.OptionalAdjustments.empty())
return CheckKind::OptionalityConflict;
bool isSetter = false;
if (checkWitnessAccessibility(&requiredAccess, requirement,
if (checkWitnessAccessibility(requiredAccessScope, requirement,
match.Witness, &isSetter)) {
CheckKind kind = (isSetter
? CheckKind::AccessibilityOfSetter
: CheckKind::Accessibility);
return RequirementCheck(kind, requiredAccess);
return RequirementCheck(kind, requiredAccessScope);
}
auto requiredAvailability = AvailabilityContext::alwaysAvailable();
@@ -1914,17 +1901,23 @@ void ConformanceChecker::recordTypeWitness(AssociatedTypeDecl *assocType,
if (typeDecl) {
// Check access.
Accessibility requiredAccess = Adoptee->getAnyNominal()->getFormalAccess();
const DeclContext *requiredAccessScope =
Adoptee->getAnyNominal()->getFormalAccessScope(DC);
bool isSetter = false;
if (checkWitnessAccessibility(&requiredAccess, assocType, typeDecl,
if (checkWitnessAccessibility(requiredAccessScope, assocType, typeDecl,
&isSetter)) {
assert(!isSetter);
// Avoid relying on the lifetime of 'this'.
const DeclContext *DC = this->DC;
diagnoseOrDefer(assocType, false,
[typeDecl, requiredAccess, assocType](
[DC, typeDecl, requiredAccessScope, assocType](
TypeChecker &tc, NormalProtocolConformance *conformance) {
Accessibility requiredAccess =
accessibilityFromScopeForDiagnostics(requiredAccessScope);
auto proto = conformance->getProtocol();
bool protoForcesAccess = (requiredAccess == proto->getFormalAccess());
bool protoForcesAccess =
(requiredAccessScope == proto->getFormalAccessScope(DC));
auto diagKind = protoForcesAccess
? diag::type_witness_not_accessible_proto
: diag::type_witness_not_accessible_type;
@@ -2014,6 +2007,8 @@ void ConformanceChecker::recordTypeWitness(AssociatedTypeDecl *assocType,
static void diagnoseNoWitness(ValueDecl *Requirement, Type RequirementType,
NormalProtocolConformance *Conformance,
TypeChecker &TC) {
// FIXME: Try an ignore-access lookup?
SourceLoc FixitLocation;
SourceLoc TypeLoc;
if (auto Extension = dyn_cast<ExtensionDecl>(Conformance->getDeclContext())) {
@@ -2162,8 +2157,9 @@ ConformanceChecker::resolveWitnessViaLookup(ValueDecl *requirement) {
});
}
Accessibility requiredAccess = Adoptee->getAnyNominal()->getFormalAccess();
auto check = checkWitness(requiredAccess, requirement, best);
const DeclContext *nominalAccessScope =
Adoptee->getAnyNominal()->getFormalAccessScope(DC);
auto check = checkWitness(nominalAccessScope, requirement, best);
switch (check.Kind) {
case CheckKind::Success:
@@ -2171,12 +2167,17 @@ ConformanceChecker::resolveWitnessViaLookup(ValueDecl *requirement) {
case CheckKind::Accessibility:
case CheckKind::AccessibilityOfSetter: {
// Avoid relying on the lifetime of 'this'.
const DeclContext *DC = this->DC;
diagnoseOrDefer(requirement, false,
[witness, check, requirement](
[DC, witness, check, requirement](
TypeChecker &tc, NormalProtocolConformance *conformance) {
Accessibility requiredAccess =
accessibilityFromScopeForDiagnostics(check.RequiredAccessScope);
auto proto = conformance->getProtocol();
bool protoForcesAccess =
(check.RequiredAccess == proto->getFormalAccess());
(check.RequiredAccessScope == proto->getFormalAccessScope(DC));
auto diagKind = protoForcesAccess
? diag::witness_not_accessible_proto
: diag::witness_not_accessible_type;
@@ -2186,9 +2187,9 @@ ConformanceChecker::resolveWitnessViaLookup(ValueDecl *requirement) {
getRequirementKind(requirement),
witness->getFullName(),
isSetter,
check.RequiredAccess,
requiredAccess,
proto->getName());
fixItAccessibility(diag, witness, check.RequiredAccess, isSetter);
fixItAccessibility(diag, witness, requiredAccess, isSetter);
});
break;
}
@@ -4968,7 +4969,7 @@ DefaultWitnessChecker::resolveWitnessViaLookup(ValueDecl *requirement) {
// Perform the same checks as conformance witness matching, but silently
// ignore the candidate instead of diagnosing anything.
auto check = checkWitness(Accessibility::Public, requirement, best);
auto check = checkWitness(/*access: public*/nullptr, requirement, best);
if (check.Kind != CheckKind::Success)
return ResolveWitnessResult::ExplicitFailed;

6
test/NameBinding/accessibility.swift
View File

@@ -96,9 +96,10 @@ protocol MethodProto {
}
extension OriginallyEmpty : MethodProto {}
extension HiddenMethod : MethodProto {} // expected-error {{type 'HiddenMethod' does not conform to protocol 'MethodProto'}}
// TESTABLE-NOT: :[[@LINE-1]]:{{[^:]+}}:
#if !ACCESS_DISABLED
extension HiddenMethod : MethodProto {} // expected-error {{type 'HiddenMethod' does not conform to protocol 'MethodProto'}}
extension Foo : MethodProto {} // expected-error {{type 'Foo' does not conform to protocol 'MethodProto'}}
#endif
@@ -108,9 +109,10 @@ protocol TypeProto {
}
extension OriginallyEmpty {}
#if !ACCESS_DISABLED
extension HiddenType : TypeProto {} // expected-error {{type 'HiddenType' does not conform to protocol 'TypeProto'}}
// TESTABLE-NOT: :[[@LINE-1]]:{{[^:]+}}:
#if !ACCESS_DISABLED
extension Foo : TypeProto {} // expected-error {{type 'Foo' does not conform to protocol 'TypeProto'}}
#endif

70
test/Sema/accessibility.swift
View File

@@ -22,7 +22,7 @@ public struct PublicStruct: PublicProto, InternalProto, FilePrivateProto, Privat
private func publicReq() {} // expected-error {{method 'publicReq()' must be declared public because it matches a requirement in public protocol 'PublicProto'}} {{3-10=public}}
private func internalReq() {} // expected-error {{method 'internalReq()' must be declared internal because it matches a requirement in internal protocol 'InternalProto'}} {{3-10=internal}}
private func filePrivateReq() {} // expected-error {{method 'filePrivateReq()' must be declared fileprivate because it matches a requirement in fileprivate protocol 'FilePrivateProto'}} {{3-10=fileprivate}}
private func privateReq() {} // expected-error {{method 'privateReq()' must be as accessible as its enclosing type because it matches a requirement in protocol 'PrivateProto'}} {{3-10=fileprivate}}
private func privateReq() {} // expected-error {{method 'privateReq()' must be declared fileprivate because it matches a requirement in fileprivate protocol 'PrivateProto'}} {{3-10=fileprivate}}
public var publicVar = 0
}
@@ -32,7 +32,7 @@ internal struct InternalStruct: PublicProto, InternalProto, FilePrivateProto, Pr
private func publicReq() {} // expected-error {{method 'publicReq()' must be as accessible as its enclosing type because it matches a requirement in protocol 'PublicProto'}} {{3-10=internal}}
private func internalReq() {} // expected-error {{method 'internalReq()' must be declared internal because it matches a requirement in internal protocol 'InternalProto'}} {{3-10=internal}}
private func filePrivateReq() {} // expected-error {{method 'filePrivateReq()' must be declared fileprivate because it matches a requirement in fileprivate protocol 'FilePrivateProto'}} {{3-10=fileprivate}}
private func privateReq() {} // expected-error {{method 'privateReq()' must be as accessible as its enclosing type because it matches a requirement in protocol 'PrivateProto'}} {{3-10=fileprivate}}
private func privateReq() {} // expected-error {{method 'privateReq()' must be declared fileprivate because it matches a requirement in fileprivate protocol 'PrivateProto'}} {{3-10=fileprivate}}
public var publicVar = 0
}
@@ -42,7 +42,7 @@ fileprivate struct FilePrivateStruct: PublicProto, InternalProto, FilePrivatePro
private func publicReq() {} // expected-error {{method 'publicReq()' must be as accessible as its enclosing type because it matches a requirement in protocol 'PublicProto'}} {{3-10=fileprivate}}
private func internalReq() {} // expected-error {{method 'internalReq()' must be as accessible as its enclosing type because it matches a requirement in protocol 'InternalProto'}} {{3-10=fileprivate}}
private func filePrivateReq() {} // expected-error {{method 'filePrivateReq()' must be declared fileprivate because it matches a requirement in fileprivate protocol 'FilePrivateProto'}} {{3-10=fileprivate}}
private func privateReq() {} // expected-error {{method 'privateReq()' must be as accessible as its enclosing type because it matches a requirement in protocol 'PrivateProto'}} {{3-10=fileprivate}}
private func privateReq() {} // expected-error {{method 'privateReq()' must be declared fileprivate because it matches a requirement in fileprivate protocol 'PrivateProto'}} {{3-10=fileprivate}}
public var publicVar = 0
}
@@ -52,7 +52,7 @@ private struct PrivateStruct: PublicProto, InternalProto, FilePrivateProto, Priv
private func publicReq() {} // expected-error {{method 'publicReq()' must be as accessible as its enclosing type because it matches a requirement in protocol 'PublicProto'}} {{3-10=fileprivate}}
private func internalReq() {} // expected-error {{method 'internalReq()' must be as accessible as its enclosing type because it matches a requirement in protocol 'InternalProto'}} {{3-10=fileprivate}}
private func filePrivateReq() {} // expected-error {{method 'filePrivateReq()' must be declared fileprivate because it matches a requirement in fileprivate protocol 'FilePrivateProto'}} {{3-10=fileprivate}}
private func privateReq() {} // expected-error {{method 'privateReq()' must be as accessible as its enclosing type because it matches a requirement in protocol 'PrivateProto'}} {{3-10=fileprivate}}
private func privateReq() {} // expected-error {{method 'privateReq()' must be declared fileprivate because it matches a requirement in fileprivate protocol 'PrivateProto'}} {{3-10=fileprivate}}
public var publicVar = 0
}
@@ -389,6 +389,7 @@ enum DefaultEnumPublic {
case A(PublicStruct) // no-warning
}
struct DefaultGeneric<T> {}
// FIXME: These types are actually private; they're just being reported as
// fileprivate because they're top-level.
@@ -536,3 +537,64 @@ private prefix func !(input: PrivateOperatorAdopter) -> PrivateOperatorAdopter {
private prefix func !(input: PrivateOperatorAdopter.Inner) -> PrivateOperatorAdopter.Inner {
return input
}
public protocol Equatablish {
static func ==(lhs: Self, rhs: Self) /* -> bool */ // expected-note {{protocol requires function '=='}}
}
fileprivate struct EquatablishOuter {
internal struct Inner : Equatablish {}
}
private func ==(lhs: EquatablishOuter.Inner, rhs: EquatablishOuter.Inner) {}
// expected-note@-1 {{candidate has non-matching type}}
fileprivate struct EquatablishOuter2 {
internal struct Inner : Equatablish {
fileprivate static func ==(lhs: Inner, rhs: Inner) {}
// expected-note@-1 {{candidate has non-matching type}}
}
}
fileprivate struct EquatablishOuterProblem {
internal struct Inner : Equatablish { // expected-error {{type 'EquatablishOuterProblem.Inner' does not conform to protocol 'Equatablish'}}
private static func ==(lhs: Inner, rhs: Inner) {}
}
}
internal struct EquatablishOuterProblem2 {
public struct Inner : Equatablish {
fileprivate static func ==(lhs: Inner, rhs: Inner) {} // expected-error {{method '==' must be as accessible as its enclosing type because it matches a requirement in protocol 'Equatablish'}} {{5-16=internal}}
// expected-note@-1 {{candidate has non-matching type}}
}
}
internal struct EquatablishOuterProblem3 {
public struct Inner : Equatablish {
}
}
private func ==(lhs: EquatablishOuterProblem3.Inner, rhs: EquatablishOuterProblem3.Inner) {} // expected-error {{method '==' must be as accessible as its enclosing type because it matches a requirement in protocol 'Equatablish'}} {{1-8=internal}}
// expected-note@-1 {{candidate has non-matching type}}
public protocol AssocTypeProto {
associatedtype Assoc
}
fileprivate struct AssocTypeOuter {
internal struct Inner : AssocTypeProto {
fileprivate typealias Assoc = Int
}
}
fileprivate struct AssocTypeOuterProblem {
internal struct Inner : AssocTypeProto {
private typealias Assoc = Int // expected-error {{type alias 'Assoc' must be as accessible as its enclosing type because it matches a requirement in protocol 'AssocTypeProto'}} {{5-12=fileprivate}}
}
}
internal struct AssocTypeOuterProblem2 {
public struct Inner : AssocTypeProto {
fileprivate typealias Assoc = Int // expected-error {{type alias 'Assoc' must be as accessible as its enclosing type because it matches a requirement in protocol 'AssocTypeProto'}} {{5-16=internal}}
}
}