Check protocol witnesses using access scopes. #4176
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -68,7 +68,7 @@ namespace { | |
| unsigned &bestIdx, | ||
| bool &doNotDiagnoseMatches); | ||
|
|
||
| bool checkWitnessAccessibility(const DeclContext *&requiredAccessScope, | ||
| ValueDecl *requirement, | ||
| ValueDecl *witness, | ||
| bool *isSetter); | ||
|
|
@@ -77,7 +77,7 @@ namespace { | |
| ValueDecl *witness, | ||
| AvailabilityContext *requirementInfo); | ||
|
|
||
| RequirementCheck checkWitness(const DeclContext *requiredAccessScope, | ||
| ValueDecl *requirement, | ||
| RequirementMatch match); | ||
| }; | ||
|
|
@@ -392,24 +392,24 @@ namespace { | |
| struct RequirementCheck { | ||
| CheckKind Kind; | ||
|
|
||
| /// The required access scope, if the check failed due to the | ||
| /// witness being less accessible than the requirement. | ||
| 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), RequiredAccessScope(nullptr), | ||
| RequiredAvailability(AvailabilityContext::alwaysAvailable()) { } | ||
|
|
||
| RequirementCheck(CheckKind kind, const DeclContext *requiredAccessScope) | ||
| : Kind(kind), RequiredAccessScope(requiredAccessScope), | ||
| RequiredAvailability(AvailabilityContext::alwaysAvailable()) { } | ||
|
|
||
| RequirementCheck(CheckKind kind, AvailabilityContext requiredAvailability) | ||
| : Kind(kind), RequiredAccessScope(nullptr), | ||
| RequiredAvailability(requiredAvailability) { } | ||
| }; | ||
| } | ||
|
|
@@ -1214,48 +1214,35 @@ bool WitnessChecker::findBestWitness(ValueDecl *requirement, | |
| } | ||
|
|
||
| bool WitnessChecker:: | ||
| checkWitnessAccessibility(const DeclContext *&requiredAccessScope, | ||
| ValueDecl *requirement, | ||
| ValueDecl *witness, | ||
| bool *isSetter) { | ||
| *isSetter = false; | ||
|
|
||
| const DeclContext *protoAccessScope = Proto->getFormalAccessScope(DC); | ||
|
|
||
| // FIXME: This is the same operation as TypeCheckDecl.cpp's | ||
|
There was a problem hiding this comment. Why not make it a method on Accessibility or DeclContext then? There was a problem hiding this comment. Accessibility is an enum, and DeclContext shouldn't be cluttered with this. The real answer is an AccessScope type, which I filed a starter bug for. |
||
| // TypeAccessScopeChecker::intersectAccess. | ||
| if (!requiredAccessScope) { | ||
| requiredAccessScope = protoAccessScope; | ||
| } else if (protoAccessScope) { | ||
| if (protoAccessScope->isChildContextOf(requiredAccessScope)) { | ||
| requiredAccessScope = protoAccessScope; | ||
| } else { | ||
| assert(requiredAccessScope == protoAccessScope || | ||
| requiredAccessScope->isChildContextOf(protoAccessScope)); | ||
| } | ||
| } | ||
|
|
||
| if (!witness->isAccessibleFrom(requiredAccessScope)) | ||
| return true; | ||
|
|
||
| if (requirement->isSettable(DC)) { | ||
| *isSetter = true; | ||
|
|
||
| auto ASD = cast<AbstractStorageDecl>(witness); | ||
| if (!ASD->isSetterAccessibleFrom(requiredAccessScope)) | ||
| return true; | ||
| } | ||
|
|
||
|
|
@@ -1272,19 +1259,19 @@ checkWitnessAvailability(ValueDecl *requirement, | |
| } | ||
|
|
||
| RequirementCheck WitnessChecker:: | ||
| checkWitness(const DeclContext *requiredAccessScope, | ||
| ValueDecl *requirement, | ||
| RequirementMatch match) { | ||
| if (!match.OptionalAdjustments.empty()) | ||
| return CheckKind::OptionalityConflict; | ||
|
|
||
| bool isSetter = false; | ||
| if (checkWitnessAccessibility(requiredAccessScope, requirement, | ||
| match.Witness, &isSetter)) { | ||
| CheckKind kind = (isSetter | ||
| ? CheckKind::AccessibilityOfSetter | ||
| : CheckKind::Accessibility); | ||
| return RequirementCheck(kind, requiredAccessScope); | ||
| } | ||
|
|
||
| auto requiredAvailability = AvailabilityContext::alwaysAvailable(); | ||
|
|
@@ -1910,17 +1897,23 @@ void ConformanceChecker::recordTypeWitness(AssociatedTypeDecl *assocType, | |
|
|
||
| if (typeDecl) { | ||
| // Check access. | ||
| const DeclContext *requiredAccessScope = | ||
| Adoptee->getAnyNominal()->getFormalAccessScope(DC); | ||
| bool isSetter = false; | ||
| if (checkWitnessAccessibility(requiredAccessScope, assocType, typeDecl, | ||
| &isSetter)) { | ||
| assert(!isSetter); | ||
|
|
||
| // Avoid relying on the lifetime of 'this'. | ||
| const DeclContext *DC = this->DC; | ||
| diagnoseOrDefer(assocType, false, | ||
| [DC, typeDecl, requiredAccessScope, assocType]( | ||
| TypeChecker &tc, NormalProtocolConformance *conformance) { | ||
| Accessibility requiredAccess = | ||
| accessibilityFromScopeForDiagnostics(requiredAccessScope); | ||
| auto proto = conformance->getProtocol(); | ||
| bool protoForcesAccess = | ||
| (requiredAccessScope == proto->getFormalAccessScope(DC)); | ||
| auto diagKind = protoForcesAccess | ||
| ? diag::type_witness_not_accessible_proto | ||
| : diag::type_witness_not_accessible_type; | ||
|
|
@@ -2010,6 +2003,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())) { | ||
|
|
@@ -2158,21 +2153,27 @@ ConformanceChecker::resolveWitnessViaLookup(ValueDecl *requirement) { | |
| }); | ||
| } | ||
|
|
||
| const DeclContext *nominalAccessScope = | ||
| Adoptee->getAnyNominal()->getFormalAccessScope(DC); | ||
| auto check = checkWitness(nominalAccessScope, requirement, best); | ||
|
|
||
| switch (check.Kind) { | ||
| case CheckKind::Success: | ||
| break; | ||
|
|
||
| case CheckKind::Accessibility: | ||
| case CheckKind::AccessibilityOfSetter: { | ||
| // Avoid relying on the lifetime of 'this'. | ||
| const DeclContext *DC = this->DC; | ||
| diagnoseOrDefer(requirement, false, | ||
| [DC, witness, check, requirement]( | ||
| TypeChecker &tc, NormalProtocolConformance *conformance) { | ||
| Accessibility requiredAccess = | ||
| accessibilityFromScopeForDiagnostics(check.RequiredAccessScope); | ||
|
|
||
| auto proto = conformance->getProtocol(); | ||
| bool protoForcesAccess = | ||
| (check.RequiredAccessScope == proto->getFormalAccessScope(DC)); | ||
| auto diagKind = protoForcesAccess | ||
| ? diag::witness_not_accessible_proto | ||
| : diag::witness_not_accessible_type; | ||
|
|
@@ -2182,9 +2183,9 @@ ConformanceChecker::resolveWitnessViaLookup(ValueDecl *requirement) { | |
| getRequirementKind(requirement), | ||
| witness->getFullName(), | ||
| isSetter, | ||
| requiredAccess, | ||
| proto->getName()); | ||
| fixItAccessibility(diag, witness, requiredAccess, isSetter); | ||
| }); | ||
| break; | ||
| } | ||
|
|
@@ -4966,7 +4967,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(/*access: public*/nullptr, requirement, best); | ||
| if (check.Kind != CheckKind::Success) | ||
| return ResolveWitnessResult::ExplicitFailed; | ||
|
|
||
|
|
||
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Do we still need this staging flag?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Probably not, but I'm going to stay consistent with it until we take it out completely.