Merge pull request swiftlang#31161 from artemcm/NonmutatingPWSetterInit

Allow initializing a wrapped property with a nonmutating setter

Author: artemcm

lib/SILGen/SILGenLValue.cpp

@@ -1302,18 +1302,28 @@ namespace {
     {
     }
 
-    bool canRewriteSetAsPropertyWrapperInit() const {
+    bool canRewriteSetAsPropertyWrapperInit(SILGenFunction &SGF) const {
       if (auto *VD = dyn_cast<VarDecl>(Storage)) {
         // If this is not a wrapper property that can be initialized from
         // a value of the wrapped type, we can't perform the initialization.
         auto wrapperInfo = VD->getPropertyWrapperBackingPropertyInfo();
         if (!wrapperInfo.initializeFromOriginal)
           return false;
 
+        bool isAssignmentToSelfParamInInit =
+            IsOnSelfParameter &&
+            isa<ConstructorDecl>(SGF.FunctionDC->getAsDecl());
+
         // If we have a nonmutating setter on a value type, the call
         // captures all of 'self' and we cannot rewrite an assignment
         // into an initialization.
-        if (!VD->isSetterMutating() &&
+
+        // Unless this is an assignment to a self parameter inside a
+        // constructor, in which case we would like to still emit a
+        // assign_by_wrapper because the setter will be deleted by lowering
+        // anyway.
+        if (!isAssignmentToSelfParamInInit &&
+            !VD->isSetterMutating() &&
             VD->getDeclContext()->getSelfNominalTypeDecl() &&
             VD->isInstanceMember() &&
             !VD->getDeclContext()->getDeclaredInterfaceType()
@@ -1396,7 +1406,7 @@ namespace {
       assert(getAccessorDecl()->isSetter());
       SILDeclRef setter = Accessor;
 
-      if (IsOnSelfParameter && canRewriteSetAsPropertyWrapperInit() &&
+      if (IsOnSelfParameter && canRewriteSetAsPropertyWrapperInit(SGF) &&
           !Storage->isStatic() &&
           isBackingVarVisible(cast<VarDecl>(Storage),
                               SGF.FunctionDC)) {
@@ -1477,6 +1487,16 @@ namespace {
           capturedBase = base.copy(SGF, loc).forward(SGF);
         }
 
+        // If the base is a reference and the setter expects a value, emit a
+        // load. This pattern is emitted for property wrappers with a
+        // nonmutating setter, for example.
+        if (base.getType().isAddress() &&
+            base.getType().getObjectType() ==
+                setterConv.getSILArgumentType(argIdx)) {
+          capturedBase = SGF.B.createTrivialLoadOr(
+              loc, capturedBase, LoadOwnershipQualifier::Take);
+        }
+
         PartialApplyInst *setterPAI =
           SGF.B.createPartialApply(loc, setterFRef,
                                    Substitutions, { capturedBase },
@@ -1515,6 +1535,7 @@ namespace {
         SGF.B.createAssignByWrapper(loc, Mval.forward(SGF), proj.forward(SGF),
                                      initFn.getValue(), setterFn.getValue(),
                                      AssignOwnershipQualifier::Unknown);
+
         return;
       }
 
@@ -4300,7 +4321,7 @@ static bool trySetterPeephole(SILGenFunction &SGF, SILLocation loc,
   }
 
   auto &setterComponent = static_cast<GetterSetterComponent&>(component);
-  if (setterComponent.canRewriteSetAsPropertyWrapperInit())
+  if (setterComponent.canRewriteSetAsPropertyWrapperInit(SGF))
     return false;
   setterComponent.emitAssignWithSetter(SGF, loc, std::move(dest),
                                        std::move(src));

lib/SILOptimizer/Mandatory/DefiniteInitialization.cpp

@@ -17,6 +17,7 @@
 #include "swift/AST/Expr.h"
 #include "swift/AST/Stmt.h"
 #include "swift/ClangImporter/ClangModule.h"
+#include "swift/SIL/SILValue.h"
 #include "swift/SIL/InstructionUtils.h"
 #include "swift/SIL/SILArgument.h"
 #include "swift/SIL/SILBuilder.h"
@@ -1802,6 +1803,20 @@ void LifetimeChecker::handleLoadUseFailure(const DIMemoryUse &Use,
       TheMemory.isAnyInitSelf() && !TheMemory.isClassInitSelf()) {
     if (!shouldEmitError(Inst)) return;
 
+    // Ignore loads used only for a set-by-value (nonmutating) setter
+    // since it will be deleted by lowering anyway.
+    auto load = cast<SingleValueInstruction>(Inst);
+    if (auto Op = load->getSingleUse()) {
+      if (auto PAI = dyn_cast<PartialApplyInst>(Op->getUser())) {
+        if (std::find_if(PAI->use_begin(), PAI->use_end(),
+                         [](auto PAIUse) {
+                           return isa<AssignByWrapperInst>(PAIUse->getUser());
+                         }) != PAI->use_end()) {
+          return;
+        }
+      }
+    }
+
     diagnose(Module, Inst->getLoc(), diag::use_of_self_before_fully_init);
     noteUninitializedMembers(Use);
     return;

lib/Sema/CSApply.cpp

@@ -6859,9 +6859,37 @@ Expr *ExprRewriter::coerceToType(Expr *expr, Type toType,
   llvm_unreachable("Unhandled coercion");
 }
 
+/// Detect if the expression is an assignment to a `self` wrapped property that
+/// has a nonmutating setter, inside a constructor.
+///
+/// We use this to decide when to produce an inout_expr instead of a load_expr
+/// for the sake of emitting a reference required by the assign_by_wrapper
+/// instruction.
+static bool isNonMutatingSetterPWAssignInsideInit(Expr *baseExpr,
+                                                  ValueDecl *member,
+                                                  DeclContext *UseDC) {
+  // Setter is mutating
+  if (cast<AbstractStorageDecl>(member)->isSetterMutating())
+    return false;
+  // Member is not a wrapped property
+  auto *VD = dyn_cast<VarDecl>(member);
+  if (!(VD && VD->hasAttachedPropertyWrapper()))
+    return false;
+  // This is not an expression inside a constructor
+  auto *CD = dyn_cast<ConstructorDecl>(UseDC);
+  if (!CD)
+    return false;
+  // This is not an assignment on self
+  if (!baseExpr->isSelfExprOf(CD))
+    return false;
+
+  return true;
+}
+
 /// Adjust the given type to become the self type when referring to
 /// the given member.
-static Type adjustSelfTypeForMember(Type baseTy, ValueDecl *member,
+static Type adjustSelfTypeForMember(Expr *baseExpr,
+                                    Type baseTy, ValueDecl *member,
                                     AccessSemantics semantics,
                                     DeclContext *UseDC) {
   auto baseObjectTy = baseTy->getWithoutSpecifierType();
@@ -6885,10 +6913,15 @@ static Type adjustSelfTypeForMember(Type baseTy, ValueDecl *member,
   bool isSettableFromHere =
       SD->isSettable(UseDC) && SD->isSetterAccessibleFrom(UseDC);
 
-  // If neither the property's getter nor its setter are mutating, the base
-  // can be an rvalue.
-  if (!SD->isGetterMutating()
-      && (!isSettableFromHere || !SD->isSetterMutating()))
+  // If neither the property's getter nor its setter are mutating, and
+  // this is not a nonmutating property wrapper setter,
+  // the base can be an rvalue.
+  // With the exception of assignments to a wrapped property inside a
+  // constructor, where we need to produce a reference to be used on
+  // the assign_by_wrapper instruction. 
+  if (!SD->isGetterMutating() && 
+      (!isSettableFromHere || !SD->isSetterMutating()) &&
+      !isNonMutatingSetterPWAssignInsideInit(baseExpr, member, UseDC))
     return baseObjectTy;
 
   // If we're calling an accessor, keep the base as an inout type, because the
@@ -6918,7 +6951,7 @@ ExprRewriter::coerceObjectArgumentToType(Expr *expr,
                                          Type baseTy, ValueDecl *member,
                                          AccessSemantics semantics,
                                          ConstraintLocatorBuilder locator) {
-  Type toType = adjustSelfTypeForMember(baseTy, member, semantics, dc);
+  Type toType = adjustSelfTypeForMember(expr, baseTy, member, semantics, dc);
 
   // If our expression already has the right type, we're done.
   Type fromType = cs.getType(expr);

test/SILGen/property_wrappers.swift

@@ -845,6 +845,38 @@ struct ObservedObject<ObjectType : AnyObject > {
   }
 }
 
+
+
+// rdar://problem/60600911
+// Ensure assign_by_wrapper is emitted for initialization
+// of a property wrapper with a nonmutating set. Even though such setters
+// take `self` by-value.
+@propertyWrapper
+struct NonMutatingSetterWrapper<Value> {
+    var value: Value
+    init(wrappedValue: Value) {
+        value = wrappedValue
+    }
+    var wrappedValue: Value {
+        get { value }
+        nonmutating set {
+            print("  .. nonmutatingSet \(newValue)")
+        }
+    }
+}
+
+struct NonMutatingWrapperTestStruct {
+    // CHECK-LABEL: sil hidden [ossa] @$s17property_wrappers28NonMutatingWrapperTestStructV3valACSi_tcfC : $@convention(method) (Int, @thin NonMutatingWrapperTestStruct.Type) -> NonMutatingWrapperTestStruct {
+    // CHECK: %[[LOAD:[0-9]+]] = load [trivial] %[[SRC:[0-9]+]] : $*NonMutatingWrapperTestStruct
+    // CHECK-NEXT: %[[SET_PA:[0-9]+]] = partial_apply [callee_guaranteed] %[[PW_SETTER:[0-9]+]](%[[LOAD]]) : $@convention(method) (Int, NonMutatingWrapperTestStruct) -> ()
+    // CHECK-NEXT: assign_by_wrapper %[[SETVAL:[0-9]+]] : $Int to %[[ADDR:[0-9]+]] : $*NonMutatingSetterWrapper<Int>, init %[[INIT_PA:[0-9]+]] : $@callee_guaranteed (Int) -> NonMutatingSetterWrapper<Int>, set %[[SET_PA]] : $@callee_guaranteed (Int) -> ()
+    @NonMutatingSetterWrapper var SomeProp: Int
+    init(val: Int) {
+        SomeProp = val
+    }
+}
+
+
 // SR-12443: Crash on property with wrapper override that adds observer.
 @propertyWrapper
 struct BasicIntWrapper {

test/SILOptimizer/di_property_wrappers.swift

@@ -501,6 +501,7 @@ public final class Synchronized<Value> {
   }
 }
 
+
 struct SR_12341 {
   @Wrapper var wrapped: Int = 10
   var str: String
@@ -533,6 +534,34 @@ func testSR_12341() {
   _ = SR_12341(condition: true)
 }
 
+@propertyWrapper
+struct NonMutatingSetterWrapper<Value> {
+    var value: Value
+    init(wrappedValue: Value) {
+        value = wrappedValue
+    }
+    var wrappedValue: Value {
+        get { value }
+        nonmutating set {
+            print("  .. nonmutatingSet \(newValue)")
+        }
+    }
+}
+
+struct NonMutatingWrapperTestStruct {
+    @NonMutatingSetterWrapper var SomeProp: Int
+    init(val: Int) {
+        SomeProp = val
+    }
+}
+
+func testNonMutatingSetterStruct() {
+  // CHECK: ## NonMutatingSetterWrapper
+  print("\n## NonMutatingSetterWrapper")
+  let A = NonMutatingWrapperTestStruct(val: 11)
+  // CHECK-NEXT:  .. nonmutatingSet 11
+}
+
 testIntStruct()
 testIntClass()
 testRefStruct()
@@ -543,3 +572,4 @@ testDefaultNilOptIntStruct()
 testComposed()
 testWrapperInitWithDefaultArg()
 testSR_12341()
+testNonMutatingSetterStruct()

test/SILOptimizer/di_property_wrappers_errors.swift

@@ -23,30 +23,23 @@ struct IntStructWithClassWrapper {
   @ClassWrapper var wrapped: Int
 
   init() {
-    wrapped = 42 // expected-error{{'self' used before all stored properties are initialized}}
-    // expected-note@-1{{'self.wrapped' not initialized}}
-  } // expected-error{{return from initializer without initializing all stored properties}}
-  // expected-note@-1{{'self.wrapped' not initialized}}
+    wrapped = 42 // expected-error{{variable 'self.wrapped' used before being initialized}}
+  }
 
   init(conditional b: Bool) {
      if b {
        self._wrapped = ClassWrapper(wrappedValue: 32)
      } else {
-       wrapped = 42 // expected-error{{'self' used before all stored properties are initialized}}
-      // expected-note@-1{{'self.wrapped' not initialized}}
+       wrapped = 42 // expected-error{{variable 'self.wrapped' used before being initialized}}
      }
-  } // expected-error{{return from initializer without initializing all stored properties}}
-  // expected-note@-1{{'self.wrapped' not initialized}}
+  }
 
   init(dynamic b: Bool) {
     if b {
-      wrapped = 42 // expected-error{{'self' used before all stored properties are initialized}}
-      // expected-note@-1{{'self.wrapped' not initialized}}
+      wrapped = 42 // expected-error{{variable 'self.wrapped' used before being initialized}}
     }
-    wrapped = 27 // expected-error{{'self' used before all stored properties are initialized}}
-    // expected-note@-1{{'self.wrapped' not initialized}}
-  } // expected-error{{return from initializer without initializing all stored properties}}
-  // expected-note@-1{{'self.wrapped' not initialized}}  
+    wrapped = 27 // expected-error{{variable 'self.wrapped' used before being initialized}}
+  }
 }
 
 // SR_11477