Skip to content

Sema: Allow explicitly available overrides to be as available as their context#59857

Merged
tshortli merged 1 commit intoswiftlang:mainfrom
tshortli:incorrect-override-cannot-be-less-available
Jul 11, 2022
Merged

Sema: Allow explicitly available overrides to be as available as their context#59857
tshortli merged 1 commit intoswiftlang:mainfrom
tshortli:incorrect-override-cannot-be-less-available

Conversation

@tshortli
Copy link
Copy Markdown
Contributor

@tshortli tshortli commented Jul 2, 2022

Previously, the following test case produced an erroneous diagnostic:

class A {
  init() {}
}

@available(macOS 12, *)
class B: A {
  @available(macOS 12, *)
  override init() { // error: overriding 'init' must be as available as declaration it overrides
    super.init()
  }
}

The overridden init() constructor is as available as it can possibly be. Removing the explicit @available annotation suppressed the diagnostic.

To fix this, we check to see if the override is as available as its self type and accept it if it is.

You may be wondering how this works when the @available annotation is removed from override init() in the example. It turns out that AvailabilityInference::availableRange() returns a result that is based only on the explicit availability of the decl in question without taking the availability of the context into account (except when the context is an extension). So with the explicit annotation gone, both the base init() and the override are both considered to be "always" available. This is pretty unintuitive and arguably wrong. However, it seems like a lot of existing code depends on this behavior so I've left it for now.

Resolves rdar://96253347

@tshortli tshortli force-pushed the incorrect-override-cannot-be-less-available branch 2 times, most recently from 61609ab to e8bbe23 Compare July 2, 2022 00:44
…r context.

Previously, the following test case produced an erroneous diagnostic:

```
class A {
  init() {}
}

@available(macOS 12, *)
class B: A {
  @available(macOS 12, *)
  override init() { // error: overriding 'init' must be as available as declaration it overrides
    super.init()
  }
}
```

The overridden `init()` constructor is as available as it can possibly be. Removing the explicit `@available` annotation suppressed the diagnostic.

To fix this, we check to see if the override is as available as its self type and accept it if it is.

You may be wondering how this works when the `@available` annotation is removed from `override init()` in the example. It turns out that `AvailabilityInference::availableRange()` returns a result that is based only on the explicit availability of the decl in question without taking the availability of the context into account (except when the context is an extension). So with the explicit annotation gone, both the base `init()` and the override are both considered to be "always" available. This is pretty unintuitive and arguably wrong. However, it seems like a lot of existing code depends on this behavior so I've left it for now.

Resolves rdar://96253347
@tshortli tshortli force-pushed the incorrect-override-cannot-be-less-available branch from e8bbe23 to bd7a394 Compare July 2, 2022 00:53
@tshortli
Copy link
Copy Markdown
Contributor Author

tshortli commented Jul 2, 2022

@swift-ci please smoke test

Copy link
Copy Markdown
Contributor

@xymus xymus left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That's a nice change, I'm surprised we did without it for so long.

@tshortli tshortli merged commit 99ee194 into swiftlang:main Jul 11, 2022
@gsecula
Copy link
Copy Markdown
Contributor

gsecula commented Jul 21, 2022

Can we get a matching PR for release/5.7 as well?

@tshortli
Copy link
Copy Markdown
Contributor Author

Can we get a matching PR for release/5.7 as well?

Done: #60185

@tshortli tshortli deleted the incorrect-override-cannot-be-less-available branch July 22, 2022 00:06
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.

3 participants