Merge pull request swiftlang#40789 from gottesmm/pr-c08df4932418337dc441a39a68c787e659dcb6ec

[move-function] Make sure we error if code reinitializes a var's fields separately instead of the var all at once.

Author: gottesmm

lib/SILOptimizer/Mandatory/MoveKillsCopyableAddressesChecker.cpp

@@ -917,12 +917,26 @@ bool GatherClosureUseVisitor::visitUse(Operand *op, AccessUseType useTy) {
     return true;
 
   if (memInstMustInitialize(op)) {
+    if (stripAccessMarkers(op->get()) != useState.address) {
+      LLVM_DEBUG(llvm::dbgs()
+                 << "!!! Error! Found init use not on base address: "
+                 << *op->getUser());
+      return false;
+    }
+
     LLVM_DEBUG(llvm::dbgs() << "ClosureUse: Found init: " << *op->getUser());
     useState.inits.insert(op->getUser());
     return true;
   }
 
   if (memInstMustReinitialize(op)) {
+    if (stripAccessMarkers(op->get()) != useState.address) {
+      LLVM_DEBUG(llvm::dbgs()
+                 << "!!! Error! Found reinit use not on base address: "
+                 << *op->getUser());
+      return false;
+    }
+
     LLVM_DEBUG(llvm::dbgs() << "ClosureUse: Found reinit: " << *op->getUser());
     useState.insertReinit(op->getUser());
     return true;
@@ -1279,12 +1293,26 @@ bool GatherLexicalLifetimeUseVisitor::visitUse(Operand *op,
   }
 
   if (memInstMustInitialize(op)) {
+    if (stripAccessMarkers(op->get()) != useState.address) {
+      LLVM_DEBUG(llvm::dbgs()
+                 << "!!! Error! Found init use not on base address: "
+                 << *op->getUser());
+      return false;
+    }
+
     LLVM_DEBUG(llvm::dbgs() << "Found init: " << *op->getUser());
     useState.inits.insert(op->getUser());
     return true;
   }
 
   if (memInstMustReinitialize(op)) {
+    if (stripAccessMarkers(op->get()) != useState.address) {
+      LLVM_DEBUG(llvm::dbgs()
+                 << "!!! Error! Found reinit use not on base address: "
+                 << *op->getUser());
+      return false;
+    }
+
     LLVM_DEBUG(llvm::dbgs() << "Found reinit: " << *op->getUser());
     useState.insertReinit(op->getUser());
     return true;
@@ -1308,6 +1336,14 @@ bool GatherLexicalLifetimeUseVisitor::visitUse(Operand *op,
   // are going to try and extend move checking into the partial apply using
   // cloning to eliminate destroys or reinits.
   if (auto fas = FullApplySite::isa(op->getUser())) {
+    if (stripAccessMarkers(op->get()) != useState.address) {
+      LLVM_DEBUG(
+          llvm::dbgs()
+          << "!!! Error! Found consuming closure use not on base address: "
+          << *op->getUser());
+      return false;
+    }
+
     if (fas.getArgumentOperandConvention(*op) ==
         SILArgumentConvention::Indirect_InoutAliasable) {
       // If we don't find the function, we can't handle this, so bail.

test/SILOptimizer/move_function_kills_copyable_addressonly_vars.swift

@@ -631,3 +631,62 @@ extension MiscTests {
     }
 }
 
+//////////////////////////////////
+// Multiple Captures from Defer //
+//////////////////////////////////
+
+func multipleCapture1<T : P>(_ k: T) -> () {
+    let kType = type(of: k)
+    var k2 = k
+    var k3 = k
+    let _ = _move(k2)
+    let _ = _move(k3)
+    var k4 = k
+    k4 = k
+    defer {
+        k2 = kType.getP()
+        print(k4)
+        k3 = kType.getP()
+    }
+    print("foo bar")
+}
+
+func multipleCapture2<T : P>(_ k: T) -> () {
+    let kType = type(of: k)
+    var k2 = k // expected-error {{'k2' used after being moved}}
+    k2 = k
+    var k3 = k
+    let _ = _move(k2) // expected-note {{move here}}
+    let _ = _move(k3)
+    var k4 = k
+    k4 = k
+    defer {
+        print(k2) // expected-note {{use here}}
+        print(k4)
+        k3 = kType.getP()
+    }
+    print("foo bar")
+}
+
+//////////////////////
+// Reinit in pieces //
+//////////////////////
+
+// These tests exercise the diagnostic to see how we error if we re-initialize a
+// var in pieces. Eventually we should teach either this diagnostic pass how to
+// handle this or teach DI how to combine the initializations into one large
+// reinit.
+struct ProtPair<T : P> {
+    var lhs: T
+    var rhs: T
+}
+
+func reinitInPieces1<T : P>(_ k: ProtPair<T>) {
+    let selfType = type(of: k.lhs)
+    var k2 = k
+    k2 = k
+
+    let _ = _move(k2) // expected-error {{_move applied to value that the compiler does not support checking}}
+    k2.lhs = selfType.getP()
+    k2.rhs = selfType.getP()
+}

test/SILOptimizer/move_function_kills_copyable_loadable_vars.swift

@@ -674,3 +674,60 @@ extension KlassWrapper {
         print("foo bar")
     }
 }
+
+//////////////////////////////////
+// Multiple Captures from Defer //
+//////////////////////////////////
+
+func multipleCapture1(_ k: Klass) -> () {
+    var k2 = k
+    var k3 = k
+    let _ = _move(k2)
+    let _ = _move(k3)
+    var k4 = k
+    k4 = k
+    defer {
+        k2 = Klass()
+        print(k4)
+        k3 = Klass()
+    }
+    print("foo bar")
+}
+
+func multipleCapture2(_ k: Klass) -> () {
+    var k2 = k // expected-error {{'k2' used after being moved}}
+    k2 = k
+    var k3 = k
+    let _ = _move(k2) // expected-note {{move here}}
+    let _ = _move(k3)
+    var k4 = k
+    k4 = k
+    defer {
+        print(k2) // expected-note {{use here}}
+        print(k4)
+        k3 = Klass()
+    }
+    print("foo bar")
+}
+
+//////////////////////
+// Reinit in pieces //
+//////////////////////
+
+// These tests exercise the diagnostic to see how we error if we re-initialize a
+// var in pieces. Eventually we should teach either this diagnostic pass how to
+// handle this or teach DI how to combine the initializations into one large
+// reinit.
+struct KlassPair {
+    var lhs: Klass
+    var rhs: Klass
+}
+
+func reinitInPieces1(_ k: KlassPair) {
+    var k2 = k
+    k2 = k
+
+    let _ = _move(k2) // expected-error {{_move applied to value that the compiler does not support checking}}
+    k2.lhs = Klass()
+    k2.rhs = Klass()
+}