SE-0025: Allow public members inside internal types. #3404
Merged
Conversation
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
|
@swift-ci Please test |
|
@CodaFi, @slavapestov, mind reviewing? |
| } else if (auto enclosingExt = dyn_cast<ExtensionDecl>(getDeclContext())) { | ||
| // Just check the base type. If it's a constrained extension, Sema should | ||
| // have already enforced access more strictly. | ||
| if (auto nominal = enclosingExt->getExtendedType()->getAnyNominal()) { |
There was a problem hiding this comment.
enclosingExt->getExtendedType() should be conditionally accessed as well.
There was a problem hiding this comment.
Yyyep, apparently so. Fixed.
jrose-apple
force-pushed
the
allow-broad-access
branch
from
96cdef2 to
59af066
Compare
(and any other member with higher access control than its enclosing type)
There's no effect, but it is now considered legal and the compiler will
no longer warn about it. This allows an API author to prototype their
API with proper access levels and still limit the top-level type.
If the new getEffectiveAccess computation turns out to be expensive, we
can cache the result.
Note that the compiler will still warn when putting a public member
inside an extension explicitly marked internal, because the extended
type could be public and then including a public member would be valid.
It is also still an error to put a public member inside a constrained
extension of an internal type, though I think this one is safe to
relax later.
Progress on SE-0025 ('private' and 'fileprivate')
|
@swift-ci Please test |
|
Looks reasonable. I recall seeing some radars where someone reported seeing a diagnostic about how something (an extension?) could not be public, and then after removing the public keyword, another diagnostic complained it cannot be private. I don't remember the details though, but maybe if you know, it could be worth cleaning that up as well. |
|
@swift-ci Please smoke test |
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.
(and any other member with higher access control than its enclosing type)
There's no effect, but it is now considered legal and the compiler will no longer warn about it. This allows an API author to prototype their API with proper access levels and still limit the top-level type.
If the new
getEffectiveAccesscomputation turns out to be expensive, we can cache the result.Note that the compiler will still warn when putting a public member inside an extension explicitly marked
internal, because the extended type could be public and then including a public member would be valid. It is also still an error to put a public member inside a constrained extension of an internal type, though I think this one is safe to relax later.Progress on SE-0025 (
privateandfileprivate)Before merging this pull request to apple/swift repository:
Triggering Swift CI
The swift-ci is triggered by writing a comment on this PR addressed to the GitHub user @swift-ci. Different tests will run depending on the specific comment that you use. The currently available comments are:
Smoke Testing
Validation Testing
Lint Testing
Note: Only members of the Apple organization can trigger swift-ci.