AST/Sema: Continue adopting AvailabilityConstraint #79260
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
tshortli
requested review from
hborla,
slavapestov,
AnthonyLatsis and
xedin
as code owners
tshortli
force-pushed
the
more-availability-constraint-adoption
branch
from
0841b68 to
1db5896
Compare
tshortli
commented
Feb 10, 2025
| var prop2: Int32 { | ||
| didSet { | ||
| _ = prop2 // expected-error {{'prop2' is unavailable}} | ||
| _ = prop2 |
There was a problem hiding this comment.
@beccadax I wanted to give you a heads up about the changes I'm making to this test since you just added it. The behavior that the test was reproducing before was incorrect, since the context of the methods in the @objc @implementation extension are themselves as unavailable as the properties and therefore the diagnostic should be supressed. After the refactoring in this PR to adopt some of my new shared infrastructure, the suppression of the diagnostics is working more consistently.
I do think we'll want to start diagnosing when an @objc @implementation extension is less available than the interface it's implementing. As it stands, this allows a back door into calling arbitrary unavailable code which ought to be unreachable.
tshortli
force-pushed
the
more-availability-constraint-adoption
branch
2 times, most recently
from
c62bfdd to
c1e6063
Compare
tshortli
requested review from
ahoppen,
bnbarham,
CodaFi,
DougGregor,
hamishknight and
rintaro
as code owners
tshortli
force-pushed
the
more-availability-constraint-adoption
branch
6 times, most recently
from
427434d to
2400e9c
Compare
|
swiftlang/swift-package-manager#8289 @swift-ci please smoke test |
tshortli
force-pushed
the
more-availability-constraint-adoption
branch
from
2400e9c to
b9b9b19
Compare
|
@swift-ci please smoke test |
|
swiftlang/swift-package-manager#8291 @swift-ci please smoke test |
tshortli
force-pushed
the
more-availability-constraint-adoption
branch
2 times, most recently
from
2a4a96e to
0382dea
Compare
|
@swift-ci please smoke test |
tshortli
force-pushed
the
more-availability-constraint-adoption
branch
from
0382dea to
54cb2a2
Compare
|
@swift-ci please test |
|
@swift-ci please test source compatibility debug |
|
@swift-ci please build toolchain macOS |
Switch to calling `swift::getAvailabilityConstraintsForDecl()` to get the unsatisfied availability constraints that should be diagnosed. This was intended to be NFC, but it turns out it fixed a bug in the recently introduced objc_implementation_direct_to_storage.swift test. In the test, the stored properties are as unavailable as the context that is accessing them so the accesses should not be diagnosed. However, this test demonstrates a bigger issue with `@objc @implementation`, which is that it allows the implementations of Obj-C interfaces to be less available than the interface, which effectively provides an availability checking loophole that can be used to invoke unavailable code.
When building up AvailabilityContexts, we assume that all of the enclosing decls have already been accounted for in the AvailabilityContext that we are constraining. Therefore, it doesn't make sense to merge availability constraints from the enclosing extension of the target decl.
Replaces `getUnsatisfiedAvailabilityConstraint()`. NFC.
Query for availability constraints instead of calling getVersionAvailability().
tshortli
force-pushed
the
more-availability-constraint-adoption
branch
from
54cb2a2 to
e804c93
Compare
|
@swift-ci please test |
tshortli
added a commit
to tshortli/swift
that referenced
this pull request
Apr 10, 2025
…raints. Potential unavailability of a declaration is always diagnosed in a context that does not have a sufficient platform introduction constraint, even if that context is also unavailable on that platform. This behavior is overly strict, since the potential unavailability will never matter, but it's a longstanding quirk of availability checking. As a result, some source code has been written to work around this quirk by marking declarations as simultaneously unavailable and introduced for a given platform: ``` @available(macOS, unavailable, introduced: 15) func unavailableAndIntroducedInMacOS15() { // ... allowed to call functions introduced in macOS 15. } ``` When availability checking was refactored to be based on a constraint engine in swiftlang#79260, the compiler started effectively treating `@available(macOS, unavailable, introduced: 15)` as just `@available(macOS, unavailable)` because the introduction constraint was treated as lower priority and therefore superseded by the unavailability constraint. This caused a regression for the code that was written to work around the availability checker's strictness. The fix is to allow the constraint engine to track multiple kinds of constraints per-domain which in turn ensures that the platform introduction of an availability context can be further refined even if that context is also unavailable. A better fix would be to stop diagnosing potential unavailability entirely in unavailable contexts but that change is out of scope for the 6.2 release. Resolves rdar://147945883.
tshortli
added a commit
to tshortli/swift
that referenced
this pull request
Apr 10, 2025
…raints. Potential unavailability of a declaration is always diagnosed in a context that does not have a sufficient platform introduction constraint, even if that context is also unavailable on that platform. This behavior is overly strict, since the potential unavailability will never matter, but it's a longstanding quirk of availability checking. As a result, some source code has been written to work around this quirk by marking declarations as simultaneously unavailable and introduced for a given platform: ``` @available(macOS, unavailable, introduced: 15) func unavailableAndIntroducedInMacOS15() { // ... allowed to call functions introduced in macOS 15. } ``` When availability checking was refactored to be based on a constraint engine in swiftlang#79260, the compiler started effectively treating `@available(macOS, unavailable, introduced: 15)` as just `@available(macOS, unavailable)` because the introduction constraint was treated as lower priority and therefore superseded by the unavailability constraint. This caused a regression for the code that was written to work around the availability checker's strictness. The fix is to allow the constraint engine to track multiple kinds of constraints per-domain which in turn ensures that the platform introduction of an availability context can be further refined even if that context is also unavailable. A better fix would be to stop diagnosing potential unavailability entirely in unavailable contexts but that change is out of scope for the 6.2 release. Resolves rdar://147945883.
tshortli
added a commit
to tshortli/swift
that referenced
this pull request
Apr 10, 2025
…raints Potential unavailability of a declaration is always diagnosed in a context that does not have a sufficient platform introduction constraint, even if that context is also unavailable on that platform. This behavior is overly strict, since the potential unavailability will never matter, but it's a longstanding quirk of availability checking. As a result, some source code has been written to work around this quirk by marking declarations as simultaneously unavailable and introduced for a given platform: ``` @available(macOS, unavailable, introduced: 15) func unavailableAndIntroducedInMacOS15() { // ... allowed to call functions introduced in macOS 15. } ``` When availability checking was refactored to be based on a constraint engine in swiftlang#79260, the compiler started effectively treating `@available(macOS, unavailable, introduced: 15)` as just `@available(macOS, unavailable)` because the introduction constraint was treated as lower priority and therefore superseded by the unavailability constraint. This caused a regression for the code that was written to work around the availability checker's strictness. We could try to match the behavior from previous releases, but it's actually tricky to match the behavior well enough in the new availability checking architecture to fully fix source compatibility. Consequently, it seems like the best fix is actually to address this long standing issue and stop diagnosing potential unavailability in unavailable contexts. The main risk of this approach is source compatibility for regions of unavailable code. It's theoretically possible that restricting available declarations by introduction version in unavailable contexts is important to prevent ambiguities during overload resolution in some codebases. If we find that is a problem that is too prevalent, we may have to take a different approach. Resolves rdar://147945883.
tshortli
added a commit
to tshortli/swift
that referenced
this pull request
Apr 10, 2025
…ble contexts. Potential unavailability of a declaration has always been diagnosed in contexts that do not have a sufficient platform introduction constraint, even when those contexts are also unavailable on the target platform. This behavior is overly strict, since the potential unavailability will never matter, but it's a longstanding quirk of availability checking. As a result, some source code has been written to work around this quirk by marking declarations as simultaneously unavailable and introduced for a given platform: ``` @available(macOS, unavailable, introduced: 15) func unavailableAndIntroducedInMacOS15() { // ... allowed to call functions introduced in macOS 15. } ``` When availability checking was refactored to be based on a constraint engine in swiftlang#79260, the compiler started effectively treating `@available(macOS, unavailable, introduced: 15)` as just `@available(macOS, unavailable)` because the introduction constraint was treated as lower priority and therefore superseded by the unavailability constraint. This caused a regression for the code that was written to work around the availability checker's strictness. We could try to match the behavior from previous releases, but it's actually tricky to match the behavior well enough in the new availability checking architecture to fully fix source compatibility. Consequently, it seems like the best fix is actually to address this long standing issue and stop diagnosing potential unavailability in unavailable contexts. The main risk of this approach is source compatibility for regions of unavailable code. It's theoretically possible that restricting available declarations by introduction version in unavailable contexts is important to prevent ambiguities during overload resolution in some codebases. If we find that is a problem that is too prevalent, we may have to take a different approach. Resolves rdar://147945883.
tshortli
added a commit
to tshortli/swift
that referenced
this pull request
Apr 10, 2025
…ble contexts. Potential unavailability of a declaration has always been diagnosed in contexts that do not have a sufficient platform introduction constraint, even when those contexts are also unavailable on the target platform. This behavior is overly strict, since the potential unavailability will never matter, but it's a longstanding quirk of availability checking. As a result, some source code has been written to work around this quirk by marking declarations as simultaneously unavailable and introduced for a given platform: ``` @available(macOS, unavailable, introduced: 15) func unavailableAndIntroducedInMacOS15() { // ... allowed to call functions introduced in macOS 15. } ``` When availability checking was refactored to be based on a constraint engine in swiftlang#79260, the compiler started effectively treating `@available(macOS, unavailable, introduced: 15)` as just `@available(macOS, unavailable)` because the introduction constraint was treated as lower priority and therefore superseded by the unavailability constraint. This caused a regression for the code that was written to work around the availability checker's strictness. We could try to match the behavior from previous releases, but it's actually tricky to match the behavior well enough in the new availability checking architecture to fully fix source compatibility. Consequently, it seems like the best fix is actually to address this long standing issue and stop diagnosing potential unavailability in unavailable contexts. The main risk of this approach is source compatibility for regions of unavailable code. It's theoretically possible that restricting available declarations by introduction version in unavailable contexts is important to prevent ambiguities during overload resolution in some codebases. If we find that is a problem that is too prevalent, we may have to take a different approach. Resolves rdar://147945883.
tshortli
added a commit
to tshortli/swift
that referenced
this pull request
Apr 11, 2025
…ble contexts. Potential unavailability of a declaration has always been diagnosed in contexts that do not have a sufficient platform introduction constraint, even when those contexts are also unavailable on the target platform. This behavior is overly strict, since the potential unavailability will never matter, but it's a longstanding quirk of availability checking. As a result, some source code has been written to work around this quirk by marking declarations as simultaneously unavailable and introduced for a given platform: ``` @available(macOS, unavailable, introduced: 15) func unavailableAndIntroducedInMacOS15() { // ... allowed to call functions introduced in macOS 15. } ``` When availability checking was refactored to be based on a constraint engine in swiftlang#79260, the compiler started effectively treating `@available(macOS, unavailable, introduced: 15)` as just `@available(macOS, unavailable)` because the introduction constraint was treated as lower priority and therefore superseded by the unavailability constraint. This caused a regression for the code that was written to work around the availability checker's strictness. We could try to match the behavior from previous releases, but it's actually tricky to match the behavior well enough in the new availability checking architecture to fully fix source compatibility. Consequently, it seems like the best fix is actually to address this long standing issue and stop diagnosing potential unavailability in unavailable contexts. The main risk of this approach is source compatibility for regions of unavailable code. It's theoretically possible that restricting available declarations by introduction version in unavailable contexts is important to prevent ambiguities during overload resolution in some codebases. If we find that is a problem that is too prevalent, we may have to take a different approach. Resolves rdar://147945883.
tshortli
added a commit
to tshortli/swift
that referenced
this pull request
Apr 11, 2025
…ble contexts. Potential unavailability of a declaration has always been diagnosed in contexts that do not have a sufficient platform introduction constraint, even when those contexts are also unavailable on the target platform. This behavior is overly strict, since the potential unavailability will never matter, but it's a longstanding quirk of availability checking. As a result, some source code has been written to work around this quirk by marking declarations as simultaneously unavailable and introduced for a given platform: ``` @available(macOS, unavailable, introduced: 15) func unavailableAndIntroducedInMacOS15() { // ... allowed to call functions introduced in macOS 15. } ``` When availability checking was refactored to be based on a constraint engine in swiftlang#79260, the compiler started effectively treating `@available(macOS, unavailable, introduced: 15)` as just `@available(macOS, unavailable)` because the introduction constraint was treated as lower priority and therefore superseded by the unavailability constraint. This caused a regression for the code that was written to work around the availability checker's strictness. We could try to match the behavior from previous releases, but it's actually tricky to match the behavior well enough in the new availability checking architecture to fully fix source compatibility. Consequently, it seems like the best fix is actually to address this long standing issue and stop diagnosing potential unavailability in unavailable contexts. The main risk of this approach is source compatibility for regions of unavailable code. It's theoretically possible that restricting available declarations by introduction version in unavailable contexts is important to prevent ambiguities during overload resolution in some codebases. If we find that is a problem that is too prevalent, we may have to take a different approach. Resolves rdar://147945883.
tshortli
added a commit
to tshortli/swift
that referenced
this pull request
Apr 11, 2025
…ble contexts. Potential unavailability of a declaration has always been diagnosed in contexts that do not have a sufficient platform introduction constraint, even when those contexts are also unavailable on the target platform. This behavior is overly strict, since the potential unavailability will never matter, but it's a longstanding quirk of availability checking. As a result, some source code has been written to work around this quirk by marking declarations as simultaneously unavailable and introduced for a given platform: ``` @available(macOS, unavailable, introduced: 15) func unavailableAndIntroducedInMacOS15() { // ... allowed to call functions introduced in macOS 15. } ``` When availability checking was refactored to be based on a constraint engine in swiftlang#79260, the compiler started effectively treating `@available(macOS, unavailable, introduced: 15)` as just `@available(macOS, unavailable)` because the introduction constraint was treated as lower priority and therefore superseded by the unavailability constraint. This caused a regression for the code that was written to work around the availability checker's strictness. We could try to match the behavior from previous releases, but it's actually tricky to match the behavior well enough in the new availability checking architecture to fully fix source compatibility. Consequently, it seems like the best fix is actually to address this long standing issue and stop diagnosing potential unavailability in unavailable contexts. The main risk of this approach is source compatibility for regions of unavailable code. It's theoretically possible that restricting available declarations by introduction version in unavailable contexts is important to prevent ambiguities during overload resolution in some codebases. If we find that is a problem that is too prevalent, we may have to take a different approach. Resolves rdar://147945883.
tshortli
added a commit
to tshortli/swift
that referenced
this pull request
Apr 11, 2025
…ble contexts. Potential unavailability of a declaration has always been diagnosed in contexts that do not have a sufficient platform introduction constraint, even when those contexts are also unavailable on the target platform. This behavior is overly strict, since the potential unavailability will never matter, but it's a longstanding quirk of availability checking. As a result, some source code has been written to work around this quirk by marking declarations as simultaneously unavailable and introduced for a given platform: ``` @available(macOS, unavailable, introduced: 15) func unavailableAndIntroducedInMacOS15() { // ... allowed to call functions introduced in macOS 15. } ``` When availability checking was refactored to be based on a constraint engine in swiftlang#79260, the compiler started effectively treating `@available(macOS, unavailable, introduced: 15)` as just `@available(macOS, unavailable)` because the introduction constraint was treated as lower priority and therefore superseded by the unavailability constraint. This caused a regression for the code that was written to work around the availability checker's strictness. We could try to match the behavior from previous releases, but it's actually tricky to match the behavior well enough in the new availability checking architecture to fully fix source compatibility. Consequently, it seems like the best fix is actually to address this long standing issue and stop diagnosing potential unavailability in unavailable contexts. The main risk of this approach is source compatibility for regions of unavailable code. It's theoretically possible that restricting available declarations by introduction version in unavailable contexts is important to prevent ambiguities during overload resolution in some codebases. If we find that is a problem that is too prevalent, we may have to take a different approach. Resolves rdar://147945883.
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.
Builds on top of #79249 by reimplementing various availability diagnostics and queries on top of
swift::getAvailabilityConstraintsForDecl().