Skip to content

[5.1] [PropertyWrapper] Fix a bug with static property wrapper being rejected in a class #27212

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
merged 3 commits into from
Sep 19, 2019
Merged

Conversation

theblixguy
Copy link
Collaborator

@theblixguy theblixguy commented Sep 17, 2019

Cherry-picking #27198. The first commit is new because we don't have TypeCheckStorage.cpp in the 5.1 branch. The second commit is from the original PR.

Explanation: We were rejecting the following valid code:

@propertyWrapper
struct Foo<Value> {
  var wrappedValue: Value
}

class Bar {
  // error: class stored properties not supported in classes; did you mean 'static'?
  @Foo static var bool: Bool = true
}

The problem here was that we were using the backing variable's static spelling, which is not present (because we haven't created its pattern binding at the time we access the spelling), so the check was falling down to calling getCorrectStaticSpellingForDecl() which returned class as the spelling since the context is a class.

We should be using the original variable's static spelling instead (we already use its static-ness to decide whether the backing variable should be static or not, might as well use its static spelling too).

Scope: Property wrapper usage inside a class

Issue: SR-11478

Testing: Swift CI

Risk: Low. This allows code that was previously rejected (incorrectly)

Reviewed by: @jrose-apple

@theblixguy
Copy link
Collaborator Author

@swift-ci please test

@DougGregor
Copy link
Member

@swift-ci please nominate

@theblixguy
Copy link
Collaborator Author

theblixguy commented Sep 19, 2019

Uh oh, a merge conflict sneaked in! Rerunning CI...

@swift-ci please test

@swift-ci

This comment has been minimized.

@swift-ci

This comment has been minimized.

@DougGregor
Copy link
Member

Drat, the conflict is probably from the merge of your other 5.1 PR. Thanks for fixing the conflict!

@DougGregor DougGregor merged commit db50701 into swiftlang:swift-5.1-branch Sep 19, 2019
@theblixguy theblixguy deleted the fix/SR-11478_5.1 branch September 19, 2019 23:58
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