Skip to content

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

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 2 commits into from
Sep 17, 2019
Merged

Conversation

theblixguy
Copy link
Collaborator

@theblixguy theblixguy commented Sep 16, 2019

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).

Resolves SR-11478.

…ble, use the original property's static spelling
Copy link
Contributor

@jrose-apple jrose-apple left a comment

Choose a reason for hiding this comment

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

Nice that this is so small!

@theblixguy
Copy link
Collaborator Author

@swift-ci please smoke test

@theblixguy theblixguy merged commit 0bd7621 into swiftlang:master Sep 17, 2019
@theblixguy theblixguy deleted the fix/SR-11478 branch September 17, 2019 09:10
@DougGregor
Copy link
Member

This is great! Can you create a pull request against swift-5.1-branch?

@theblixguy theblixguy restored the fix/SR-11478 branch September 17, 2019 16:35
@theblixguy theblixguy deleted the fix/SR-11478 branch September 17, 2019 16:44
@theblixguy
Copy link
Collaborator Author

@DougGregor Sure, here you go: #27212

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