[5.7][Sema] Propagate nonisolated attribute from wrapped property to the synthesized projectedValue property

[theblixguy]

Description: We were not propagating the nonisolated attribute from the wrapped property to the synthesised projectedValue variable, thus triggering a main actor-isolated property '$<name>' can not be referenced from a non-isolated context error in some contexts when accessing the projectedValue.

Risk: Low, just adding a missing attribute to a synthesised variable
Review by: @xedin
Testing: PR Testing
Original PR: #60421

[theblixguy]

@swift-ci please test

[hborla]

Hmmm, I don't think it's correct that you can mark a wrapped property as nonisolated, because the accessors use the isolated backing storage variable, and it would not be safe to mark the backing storage variable as nonisolated. This seems to expose a bug in actor isolation checking, because it allows you to use actor-isolated state in a non-isolated context.

[theblixguy]

Hmmm, I don't think it's correct that you can mark a wrapped property as nonisolated, because the accessors use the isolated backing storage variable, and it would not be safe to mark the backing storage variable as nonisolated. This seems to expose a bug in actor isolation checking, because it allows you to use actor-isolated state in a non-isolated context.

Ah okay, so we should disallow marking wrapped properties as nonisolated? We also allow marking the wrappedValue and projectedValue as nonisolated in the property wrapper (which also removes this error). Is that correct?

[xedin]

I thought that the reason why <name> is allowed is because backing property is transitively marked as nonisolated which seemed reasonable to me conceptually.

[hborla]

I thought that the reason why is allowed is because backing property is transitively marked as nonisolated which seemed reasonable to me conceptually.

The backing property cannot be marked as nonisolated because it's a stored var. This means you'd be able to mutate actor state from outside the actor, which is fundamentally unsafe.

[xedin]

Ah I see, I didn't realize that nonisolated is only allowed on computed properties.

[hborla]

Ah okay, so we should disallow marking wrapped properties as nonisolated?

Yes, I think so!

We also allow marking the wrappedValue and projectedValue as nonisolated in the property wrapper (which also removes this error). Is that correct?

This may be correct because actor isolation checking should ensure that the (hand written) accessors for var wrappedValue and var projectedValue only access other nonisolated state.

There's probably some design discussion to be had on the forums about this topic. For example, we could allow nonisolated wrapped properties if we make the backing storage a let constant, which seems reasonable to me, although we may want a way to explicitly say the backing storage is a let constant.

In any case, I think we shouldn't take this change for 5.7 and we should discuss this on the forums!

[xedin]

@theblixguy please revert it from main as well.

[theblixguy]

Thank you for the explanation! It makes sense to me. I will create a forums post to discuss this further.

For now, I will revert the PR on main (and close this one) and will open a new PR to disallow nonisolated on wrapped properties.