[Property Wrappers] Check for ignored assign_by_wrapper loads in

LifetimeChecker::shouldEmitError in order to avoid incorrectly
populating EmittedErrorLocs when no error is emitted.

This fixes a few corner cases where DI would error out or crash
while assigning to a wrapped property with a nonmutating setter.

Author: hborla

lib/SILGen/SILGenLValue.cpp

@@ -1494,21 +1494,19 @@ namespace {
 
           if (setterConv.getSILArgumentConvention(argIdx).isInoutConvention()) {
             capturedBase = base.getValue();
+          } else if (base.getType().isAddress() &&
+                     base.getType().getObjectType() ==
+                     setterConv.getSILArgumentType(argIdx,
+                                                   SGF.getTypeExpansionContext())) {
+            // 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.
+            capturedBase = SGF.B.createTrivialLoadOr(
+                loc, base.getValue(), LoadOwnershipQualifier::Copy);
           } else {
             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,
-                                                SGF.getTypeExpansionContext())) {
-            capturedBase = SGF.B.createTrivialLoadOr(
-                loc, capturedBase, LoadOwnershipQualifier::Take);
-          }
-
           capturedArgs.push_back(capturedBase);
         }
 

lib/SILOptimizer/Mandatory/DefiniteInitialization.cpp

@@ -625,6 +625,22 @@ bool LifetimeChecker::shouldEmitError(const SILInstruction *Inst) {
       }))
     return false;
 
+  // Ignore loads used only by an assign_by_wrapper setter. This
+  // is safe to ignore because assign_by_wrapper will only be
+  // re-written to use the setter if the value is fully initialized.
+  if (auto *load = dyn_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 false;
+        }
+      }
+    }
+  }
+
   EmittedErrorLocs.push_back(InstLoc);
   return true;
 }
@@ -1841,20 +1857,6 @@ 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;

test/SILOptimizer/di_property_wrappers.swift

@@ -537,30 +537,66 @@ func testSR_12341() {
 
 @propertyWrapper
 struct NonMutatingSetterWrapper<Value> {
-    var value: Value
-    init(wrappedValue: Value) {
-        value = wrappedValue
-    }
-    var wrappedValue: Value {
-        get { value }
-        nonmutating set {
-            print("  .. nonmutatingSet \(newValue)")
-        }
+  var value: Value
+  init(wrappedValue: Value) {
+    print("  .. init \(wrappedValue)")
+    value = wrappedValue
+  }
+  var wrappedValue: Value {
+    get { value }
+    nonmutating set {
+      print("  .. nonmutatingSet \(newValue)")
     }
+  }
 }
 
 struct NonMutatingWrapperTestStruct {
-    @NonMutatingSetterWrapper var SomeProp: Int
-    init(val: Int) {
-        SomeProp = val
-    }
+  @NonMutatingSetterWrapper var SomeProp: Int
+  init(val: Int) {
+    SomeProp = val
+    SomeProp = val + 1
+  }
 }
 
 func testNonMutatingSetterStruct() {
   // CHECK: ## NonMutatingSetterWrapper
   print("\n## NonMutatingSetterWrapper")
   let A = NonMutatingWrapperTestStruct(val: 11)
-  // CHECK-NEXT:  .. nonmutatingSet 11
+  // CHECK-NEXT:  .. init 11
+  // CHECK-NEXT:  .. nonmutatingSet 12
+}
+
+@propertyWrapper
+final class ClassWrapper<T> {
+  private var _wrappedValue: T
+  var wrappedValue: T {
+    get { _wrappedValue }
+    set {
+      print(" .. set \(newValue)")
+      _wrappedValue = newValue
+    }
+  }
+
+  init(wrappedValue: T) {
+    print(" .. init \(wrappedValue)")
+    self._wrappedValue = wrappedValue
+  }
+}
+
+struct StructWithClassWrapper<T> {
+   @ClassWrapper private var _storage: T?
+
+   init(value: T?) {
+      _storage = value
+   }
+}
+
+func testStructWithClassWrapper() {
+  // CHECK: ## StructWithClassWrapper
+  print("\n## StructWithClassWrapper")
+  let _ = StructWithClassWrapper<Int>(value: 10)
+  // CHECK-NEXT:  .. init nil
+  // CHECK-NEXT:  .. set Optional(10)
 }
 
 testIntStruct()
@@ -574,3 +610,4 @@ testComposed()
 testWrapperInitWithDefaultArg()
 testSR_12341()
 testNonMutatingSetterStruct()
+testStructWithClassWrapper()

test/SILOptimizer/di_property_wrappers_errors.swift

@@ -23,22 +23,22 @@ struct IntStructWithClassWrapper {
   @ClassWrapper var wrapped: Int
 
   init() {
-    wrapped = 42 // expected-error{{variable 'self.wrapped' used before being initialized}}
+    wrapped = 42
   }
 
   init(conditional b: Bool) {
      if b {
        self._wrapped = ClassWrapper(wrappedValue: 32)
      } else {
-       wrapped = 42 // expected-error{{variable 'self.wrapped' used before being initialized}}
+       wrapped = 42
      }
   }
 
   init(dynamic b: Bool) {
     if b {
-      wrapped = 42 // expected-error{{variable 'self.wrapped' used before being initialized}}
+      wrapped = 42
     }
-    wrapped = 27 // expected-error{{variable 'self.wrapped' used before being initialized}}
+    wrapped = 27
   }
 }