Retain public accessibility for witnesses of external protocol requirements#1139
Open
danwood wants to merge 1 commit into
Open
Retain public accessibility for witnesses of external protocol requirements#1139danwood wants to merge 1 commit into
danwood wants to merge 1 commit into
Conversation
…ements A public member declared in a protocol extension can be the witness for a requirement of an external public protocol such as the standard library's Identifiable. Removing the public modifier from such a witness breaks compilation, because the witness must be at least as accessible as the requirement it satisfies. The redundant public accessibility analysis previously marked the enclosing extension as redundantly public when the extended type was itself redundant, without considering that one of the extension's members could be an external protocol witness. Detect this case by inspecting the member's related references: a related reference to a protocol member whose USR does not resolve to a declaration in the source graph indicates the requirement belongs to an external protocol, which is necessarily public. When such a witness is present, the extension's public accessibility is retained.
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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
Symptom
A
publicmember declared in a protocol extension is reported as redundantly public — even when it is the witness for a requirement of an external public protocol such as the standard library'sIdentifiable. Removing thepublicmodifier, as the report suggests, breaks compilation:Downgrading the extension from
publicyields: "property 'id' must be declared public because it matches a requirement in public protocol 'Identifiable'".Root cause
RedundantExplicitPublicAccessibilityMarker.validateExtension(_:)marks apublic extensionas redundantly public whenever the extended type is itself flagged redundant. It does not consider that a member of the extension may be the witness for a requirement of an external public protocol, which obliges the witness to remain at least as accessible as the requirement.The witness relationship survives in the graph as a related reference on the member whose USR does not resolve to any local declaration (because the protocol — e.g.
Identifiable— is external). The existing cross-module exemption logic only covers witnesses of local protocols, so external-protocol witnesses fall through.Fix
Before marking an extension redundantly public, check whether any of its members is the witness for an external protocol requirement: a related reference of a protocol-member-conforming kind, with a matching name, whose USR resolves to no local declaration. If so, the extension is exempt. This is general — it covers any witness for any external public protocol requirement supplied in a protocol-extension default implementation, with no special-casing of
Identifiableorid.Adds an accessibility test covering a public protocol refining
Identifiablewhoseidwitness is supplied in a default-implementation extension.