Skip to content

[Sema] Diagnose access to the underlying storage of a lazy variable #33144

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 1 commit into from
Jul 28, 2020
Merged

[Sema] Diagnose access to the underlying storage of a lazy variable #33144

merged 1 commit into from
Jul 28, 2020

Conversation

theblixguy
Copy link
Collaborator

@theblixguy theblixguy commented Jul 27, 2020

Since #21996, the name of the underlying storage variable of a lazy var has been changed to $__lazy_storage_$_{property_name}. You couldn't actually write an identifier in (Swift) source beginning with an $ before (except anonymous closure params like $0 and etc), but since the introduction of property wrappers, you can do that and this has inadvertently allowed access to the lazy var storage and made it possible to "reset" it.

So, detect use of $__lazy_storage_$_{property_name} identifier in (Swift) source and emit a tailored error diagnostic.

@theblixguy theblixguy requested a review from rintaro July 27, 2020 22:46
@harlanhaskins
Copy link
Contributor

Good catch, thank you!

@rintaro
Copy link
Member

rintaro commented Jul 28, 2020

Not 100% sure we should diagnose this in Lexer/Parser. What if a user actually declares __lazy_storage_$_foo?

@propertyWrapper
struct Wrapper {
  var wrappedValue: Int { 1 }
  var projectedValue: Int { 1 }
}

struct MyStruct {
  @Wrapper var __lazy_storage_$_foo
  func test() {
    _ = $__lazy_storage_$_foo  // Should be OK.
  }
}

I know It's fairly unrealistic, but still.

BTW, I noticed this crashes the compiler:

@propertyWrapper
struct Wrapper {
  var wrappedValue: Int { 1 }
  var projectedValue: Int { 1 }
}

struct MyStruct {
  @Wrapper var __lazy_storage_$_foo
  lazy var foo
}

@theblixguy
Copy link
Collaborator Author

theblixguy commented Jul 28, 2020

Hmm, that's interesting. I think one solution might be to do this in Sema instead. We have isLazyStorageProperty() so we could use this to disallow access to the variable. I think we can do this in MiscDiagnostics.cpp.

BTW, I noticed this crashes the compiler:

It doesn't crash for me on master (I am on Swift version 5.3-dev (LLVM f8bd914aadc2e7b, Swift 69f543e908a2434)):

/Users/spare/Desktop/test.swift:9:3: error: lazy properties must have an initializer
  lazy var foo
  ^
/Users/spare/Desktop/test.swift:9:12: error: type annotation missing in pattern
  lazy var foo
           ^

but I'll update again and re-check.

@rintaro
Copy link
Member

rintaro commented Jul 28, 2020

Ah, sorry. The crasher was missing the initializer.

@propertyWrapper
struct Wrapper {
  var wrappedValue: Int { 1 }
  var projectedValue: Int { 1 }
}

struct MyStruct {
  @Wrapper var __lazy_storage_$_foo
  lazy var foo = 1
}

@theblixguy
Copy link
Collaborator Author

Oh yeah, that reproduces for me! I previously some work in this area (#31915) so I'll take a look at it soon.

@theblixguy
Copy link
Collaborator Author

I have re-implemented this in Sema (in MiscDiagnostics). Does this approach seem okay?

@theblixguy theblixguy changed the title [Parser] Diagnose access to the underlying storage of a lazy variable [Sema] Diagnose access to the underlying storage of a lazy variable Jul 28, 2020
@theblixguy
Copy link
Collaborator Author

@rintaro I have fixed the crash you reported in #33152 :)

@theblixguy
Copy link
Collaborator Author

Thanks!

@swift-ci please smoke test

@theblixguy
Copy link
Collaborator Author

@swift-ci please smoke test

@theblixguy theblixguy merged commit d3b5996 into swiftlang:master Jul 28, 2020
@theblixguy theblixguy deleted the fix/lazy-var-storage-access branch July 28, 2020 18:12
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.

4 participants