[Property Wrappers] Refactor property wrappers so the synthesized backing init is only type checked once #30807
Conversation
innermost call to init(wrappedValue:) if the target is for initialization of a property wrapper's backing storage.
|
@swift-ci please smoke test |
|
@swift-ci please test source compatibility |
| @@ -298,6 +298,7 @@ namespace { | |||
| ConstraintSystem &cs; | |||
| DeclContext *dc; | |||
| Solution &solution; | |||
| Optional<SolutionApplicationTarget> target; | |||
There was a problem hiding this comment.
This shouldn't be Optional, but there's a Solution::coerceToType that creates an ExprRewriter just to call ExprRewriter:: coerceToType ...
There was a problem hiding this comment.
We could probably refactor away Solution::coerceToType later. It looks unnecessary.
hborla
changed the title
hborla
force-pushed
the
property-wrapper-refactoring
branch
from
ec54de5 to
37e3950
Compare
|
Looks like MovieSwiftUI exposed some bugs. I'll investigate! |
wrapper original wrapped value expression inside of CSApply. This prevents type checking the synthesized backing storage initializer twice - once with the original expression and again with the placeholder.
…s that involve inout closure parameters and were previously type checked twice.
initializer for property wrappers that are not memberwise initialized but have an explicit original wrapped value.
hborla
force-pushed
the
property-wrapper-refactoring
branch
from
37e3950 to
5f00a1b
Compare
|
@swift-ci please smoke test |
|
@swift-ci please test source compatibility |
| @@ -298,6 +298,7 @@ namespace { | |||
| ConstraintSystem &cs; | |||
| DeclContext *dc; | |||
| Solution &solution; | |||
| Optional<SolutionApplicationTarget> target; | |||
There was a problem hiding this comment.
We could probably refactor away Solution::coerceToType later. It looks unnecessary.
include/swift/AST/Expr.h
Outdated
| @@ -4021,11 +4021,14 @@ class DynamicTypeExpr : public Expr { | |||
| /// subexpressions of that AST node. | |||
| class OpaqueValueExpr : public Expr { | |||
| SourceRange Range; | |||
| Expr *UnderlyingValue; | |||
There was a problem hiding this comment.
OpaqueValueExpr is a general-purpose construct where the "underlying value" comes from something elsewhere in the AST, above the OpaqueValueExpr. Is there some other place we can put the original initializer value that doesn't require adding this field to OpaqueValueExpr? One option: create a new Expr node ImplicitlyWrappedPropertyValueExpr that stores the OpaqueValueExpr and its underlying value. This will make findWrappedValuePlaceholder even simpler and avoid altering the design of OpaqueValueExpr.
There was a problem hiding this comment.
Yes, I think having a dedicated Expr node for the wrapped value is the right thing to do. I was also worried about something else starting to use the underlyingValue field that I added here and breaking my assumptions in this PR that it's only used for property wrappers. Thank you!
wrapped value placeholder in an init(wrappedValue:) call that was previously injected as an OpaqueValueExpr. This commit also restores the old design of OpaqueValueExpr.
|
@swift-ci please smoke test |
|
@swift-ci please test source compatibility |
…ng a property wrapper backing init if the property type doesn't match the wrapped value type.
|
@swift-ci please smoke test |
|
@swift-ci please test source compatibility |
|
@swift-ci please smoke test Linux platform |
1 similar comment
|
@swift-ci please smoke test Linux platform |
|
@swift-ci please smoke test |
Previously, if a property wrapper had an initial wrapped value expression:
Then
PropertyWrapperBackingPropertyInfoRequestwould synthesize and type-check the backing property initializer twice - once with the wrapped value expression, and again with an opaque value placeholder.Instead, we can have CSApply inject the opaque value placeholder around the
wrappedValueargument incoerceCallArguments.Resolves: rdar://problem/59685601
Resolves: rdar://problem/56163425
Resolves: SR-11594