Sema: Enable exportability checks in non-library-evolution mode as warnings by default #86335
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
…ault Always enable checking for references to `@_implementationOnly` imports even without library-evolution enabled but downgrade errors to warnings by default. Clients should enable them as errors by adopting `CheckImplementationOnly`.
…rors Recent commits changed the behavior downgrading enum cases with errors to private in `AccessLevelRequest`. It checks `hasInterfaceType` likely to avoid circular references. This is very prone to side effects. It is dependent on the timing of the check but the result was cached, making this behavior unreliable. Let's remove the downgrade to private behavior.
Invite users of `@_implementationsOnly` imports from modules without library-evolution to adopt `CheckImplementationOnly`. Use the existing warning on the import statement. Adopting that feature reports unsafe references to implementation-only imported types as errors and silences the warning on the import.
xymus
requested review from
hamishknight,
hborla,
rintaro,
slavapestov,
tshortli and
xedin
as code owners
Contributor
Author
|
@swift-ci Please smoke test |
Contributor
Author
|
@swift-ci Please smoke test |
xymus
force-pushed
the
exportability-nle-warn
branch
from
f4682d3 to
97e06cb
Compare
Contributor
Author
|
@swift-ci Please smoke test |
1 similar comment
Contributor
Author
|
@swift-ci Please smoke test |
xymus
force-pushed
the
exportability-nle-warn
branch
from
97e06cb to
810611d
Compare
Contributor
Author
|
@swift-ci Please smoke test |
1 similar comment
Contributor
Author
|
@swift-ci Please smoke test |
xymus
force-pushed
the
exportability-nle-warn
branch
from
211c38b to
b56c143
Compare
Contributor
Author
|
@swift-ci Please smoke test |
Introduce `limitBehaviorIfMorePermissive` as an alternative to `limitBehavior` that merges the new limit with the previous one. This allows for composing different conditions that would downgrade a diagnostics in succession.
Centralize the logic downgrading diagnostics to warning for newly introduced exportability checks along with the one silencing the errors. Update client sites to the new API and to uniformly apply the downgrade to warning behavior. Diagnostics about typealiases and conformances are now properly downgraded to warnings if not opt-in.
xymus
force-pushed
the
exportability-nle-warn
branch
from
b56c143 to
b0d6c7f
Compare
Contributor
Author
|
@swift-ci Please smoke test |
This one is specific. Allow extensions to define public members to package types implicitly exporting their memory layout when imported non-publicly. Enabling exportability checks for non-library-evolution by default as a warning triggered marking such types as implicitly exported to clients. This in turn enabled erroring on the extensions in clients when imported non-publicly. This error would be source breaking and shouldn't be required in such a context. We may even consider lifting this restriction entierly, even for public types imported non-public.
Extending type-checking to non-public decls triggered new errors when `-target-min-inlining-version min` is enabled. Ensure these checks are only applied to explicitly exported decls. Affected test: test/Concurrency/deinit_isolation_availability.swift
xymus
force-pushed
the
exportability-nle-warn
branch
from
b0d6c7f to
9d4ed92
Compare
Contributor
Author
|
@swift-ci Please smoke test |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
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.
Always check references to
@_implementationOnlyimports even without library-evolution enabled but downgrade errors to warnings by default. Clients should enable them as errors by adopting-enable-experimental-feature CheckImplementationOnly.Use the existing warning on the
@_implementationsOnlyimports from modules without library-evolution to invite existing users to adoptCheckImplementationOnly. Adopting that feature will also silence the warning on the import.rdar://167805228
Note, I looked into using
-WerroroverCheckImplementationOnlybut this use case doesn't apply very well since this is an existing diagnostic targeting many different exportability source to reference combinations that's already an error by default. If we really want to we could do so by accessing the-Werrorflags directly, with the implied added complexity.