Skip to content

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

Conversation

tshortli
Copy link
Contributor

@tshortli tshortli commented Feb 2, 2023

The getInnermostDeclWithAvailability() utility was not looking through extensions to the nominal type declaration when searching for enclosing availability.

Resolves rdar://104931478

…ions.

The `getInnermostDeclWithAvailability()` utility was not looking through extensions to the nominal type declaration when searching for enclosing availability.

Resolves rdar://104931478
@tshortli
Copy link
Contributor Author

tshortli commented Feb 2, 2023

@swift-ci please test

@tshortli tshortli requested review from xymus and DougGregor February 2, 2023 01:09
@tshortli
Copy link
Contributor Author

tshortli commented Feb 2, 2023

@swift-ci please test macOS

@tshortli tshortli merged commit 882e105 into swiftlang:main Feb 2, 2023
@tshortli tshortli deleted the suppress-over-availability-diags-for-synthesized-code branch February 2, 2023 19:18
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
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant