Diagnose Escaping self In Initializer through Property Wrapper

[briantkelley]

If a property wrapper is implemented using the static subscript to gain access to the enclosing self, an object must be fully initialized before assigning to the property wrapper.

Add an attribute, [enclosingSelfAccess], to the assign_by_wrapper instruction so use analysis can determine if self is escaping.

Resolves SR-12430.

[CodaFi]

This needs more SILGen tests. In particular, I'd like to see a test for valid property wrapper code that emits this instruction with the attribute attached.

[briantkelley]

@CodaFi Thank you for the feedback! I've added a SILGen test that exercises the new attribute and expanded the test case for valid and invalid uses of assigning through a property wrapper in a class initializer.

[briantkelley]

Hi @theblixguy and @DougGregor, I'm just checking in to see if you had feedback on this PR or if there's anything else I can do to help move it along. Thanks in advance! I appreciate your time!

[CodaFi]

This also needs a SIL parsing test.

[briantkelley]

@CodaFi thanks for pointing me to that test type. I added a test to verify the parsing of that instruction. I tried to minimize the supporting code as much as possible but if you have pointers on how to further simplify the test I'd be happy to update it. Thanks again!

[briantkelley]

Hey @CodaFi, I was just checking in to see if you had any additional feedback for this PR. Thank you!

[briantkelley]

Hello @CodaFi, @DougGregor, and @theblixguy! I know everyone is busy, especially with the Swift 5.3 release underway. But if you had a moment to review the change and provide any further feedback, I'd definitely appreciate it! I'm looking forward to getting this fix merged! Thank you!

[CodaFi]

This should be using the same shouldEmitError(Inst) precondition as the rest of the diagnostics. Otherwise dead usages of self will show up.

[briantkelley]

Thank you for the feedback @CodaFi! Please accept my apologies for the delay in responding to it. PR #31248 changed the semantics of property wrapper initialization and I'm not sure about the following corner case:

class SubclassWithEnclosingSelfInInitializer: Superclass {

  init(test: String) {
    // Verify the init operand is accepted.
    self.y = test

    // Verify the init operand is not accepted.
    self.z = test
  }

  @Observable
  var y: String

  @Observable
  var z: String = ""
}

With the change from the PR, the compiler now treats self.z = test1 as an initialization rather than an assign. I need to do some more investigation to see if it's possible for self to escape with this new behavior or whether the PR fully mitigates this problem.

cc: @hborla

[hborla]

Hi @briantkelley ! Sorry it took me a few days to get to this. The change in #31248 ensures that an assign_by_wrapper instruction is never re-written as assignment before all of self is initialized, including all inherited properties in superclasses. This is because re-assignment of a property wrapper can invoke property observers, which could access uninitialized memory if self is not fully initialized. So, I think self can never escape before all stored properties are initialized and super.init is called because that will always be treated as another property wrapper initialization rather than re-assignment.

[briantkelley]

So sorry this fell off my radar. Thanks for the explanation @hborla ! After working through a bunch of test cases I agree the ability to access uninitialized memory is fixed so I'm going to close this PR. However, I did identify an issue where it's possible to leak resources when assigning through the property wrapper in the initializer, which I suspect is due to not re-writing assign_by_wrapper to assignment, so I opened SR-13495/FB8621431 to track that. Thanks everyone for your input on this PR!