Skip to content

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