-
Notifications
You must be signed in to change notification settings - Fork 10.5k
AST: Correct inferred availability attributes on synthesized declarations #63361
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Merged
tshortli
merged 1 commit into
swiftlang:main
from
tshortli:suppress-over-availability-diags-for-synthesized-code
Feb 2, 2023
Merged
AST: Correct inferred availability attributes on synthesized declarations #63361
tshortli
merged 1 commit into
swiftlang:main
from
tshortli:suppress-over-availability-diags-for-synthesized-code
Feb 2, 2023
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
…ions. The `getInnermostDeclWithAvailability()` utility was not looking through extensions to the nominal type declaration when searching for enclosing availability. Resolves rdar://104931478
@swift-ci please test |
@swift-ci please test macOS |
tshortli
added a commit
to tshortli/swift
that referenced
this pull request
Mar 2, 2023
…cutor` property. When synthesizing a declaration and inferring its availability, the synthesized attribute should factor in unavailability of the parent declarations. This recently regressed with swiftlang#63361. However, the previous implementation did not produce correct results, either, because the logic for merging availability attributes produced a non-sensical result when both `unavailable` and `introduced:` availability attributes were merged. For example, this was the result for the synthesized `unownedExecutor` property of an actor when the actor was marked unavailable: ``` @available(macOS, unavailable) actor A { // Incorrectly synthesized availability for `unownedExecutor` which results from merging // the unavailability of the parent and the availability of the UnownedSerialExecutor type. @available(macOS, unavailable, introduced: macOS 10.15) @_semantics("defaultActor") nonisolated final var unownedExecutor: UnownedSerialExecutor { get } } ``` This is fixed by omitting all version components from the synthesized attribute when the overall attribute kind is "unavailable". Additionally, I discovered that the `concurrency_availability.swift` test case was no longer testing what it intended to test. The conformances to `Actor` for each `actor` in the test were no longer being synthesized and therefore `unownedExecutor` was not being synthesized. That was fixed by importing the `_Concurrency` module directly, which seems to be necessary because of the `-parse-stdlib` flag in the test. Resolves rdar://106055566
tshortli
added a commit
to tshortli/swift
that referenced
this pull request
Mar 2, 2023
…cutor` property. When synthesizing a declaration and inferring its availability, the synthesized attribute should factor in unavailability of the parent declarations. This recently regressed with swiftlang#63361. However, the previous implementation did not produce correct results, either, because the logic for merging availability attributes produced a non-sensical result when both `unavailable` and `introduced:` availability attributes were merged. For example, this was the result for the synthesized `unownedExecutor` property of an actor when the actor was marked unavailable: ``` @available(macOS, unavailable) actor A { // Incorrectly synthesized availability for `unownedExecutor` which results from merging // the unavailability of the parent and the availability of the UnownedSerialExecutor type. @available(macOS, unavailable, introduced: macOS 10.15) @_semantics("defaultActor") nonisolated final var unownedExecutor: UnownedSerialExecutor { get } } ``` This is fixed by omitting all version components from the synthesized attribute when the overall attribute kind is "unavailable". Additionally, I discovered that the `concurrency_availability.swift` test case was no longer testing what it intended to test. The conformances to `Actor` for each `actor` in the test were no longer being synthesized and therefore `unownedExecutor` was not being synthesized. That was fixed by importing the `_Concurrency` module directly, which seems to be necessary because of the `-parse-stdlib` flag in the test. Resolves rdar://106055566
tshortli
added a commit
to tshortli/swift
that referenced
this pull request
Mar 2, 2023
…cutor` property. When synthesizing a declaration and inferring its availability, the synthesized attribute should factor in unavailability of the parent declarations. This recently regressed with swiftlang#63361. However, the previous implementation did not produce correct results, either, because the logic for merging availability attributes produced a non-sensical result when both `unavailable` and `introduced:` availability attributes were merged. For example, this was the result for the synthesized `unownedExecutor` property of an actor when the actor was marked unavailable: ``` @available(macOS, unavailable) actor A { // Incorrectly synthesized availability for `unownedExecutor` which results from merging // the unavailability of the parent and the availability of the UnownedSerialExecutor type. @available(macOS, unavailable, introduced: macOS 10.15) @_semantics("defaultActor") nonisolated final var unownedExecutor: UnownedSerialExecutor { get } } ``` This is fixed by omitting all version components from the synthesized attribute when the overall attribute kind is "unavailable". Additionally, I discovered that the `concurrency_availability.swift` test case was no longer testing what it intended to test. The conformances to `Actor` for each `actor` in the test were no longer being synthesized and therefore `unownedExecutor` was not being synthesized. That was fixed by importing the `_Concurrency` module directly, which seems to be necessary because of the `-parse-stdlib` flag in the test. Resolves rdar://106055566
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.
The
getInnermostDeclWithAvailability()
utility was not looking through extensions to the nominal type declaration when searching for enclosing availability.Resolves rdar://104931478