Merge pull request #75560 from jckarter/disable-consuming-self-in-deinit

Disallow consuming `self` in a noncopyable `deinit` again.

include/swift/AST/DiagnosticsSIL.def

@@ -870,6 +870,8 @@ ERROR(sil_movechecking_discard_missing_consume_self, none,
       "must consume 'self' before exiting method that discards self", ())
 ERROR(sil_movechecking_reinit_after_discard, none,
       "cannot reinitialize 'self' after 'discard self'", ())
+ERROR(sil_movechecking_consume_during_deinit, none,
+      "'self' cannot be consumed during 'deinit'", ())
 
 NOTE(sil_movechecking_discard_self_here, none,
      "discarded self here", ())

include/swift/Basic/Features.def

@@ -247,6 +247,7 @@ EXPERIMENTAL_FEATURE(OldOwnershipOperatorSpellings, true)
 EXPERIMENTAL_FEATURE(MoveOnlyEnumDeinits, true)
 EXPERIMENTAL_FEATURE(MoveOnlyTuples, true)
 EXPERIMENTAL_FEATURE(MoveOnlyPartialReinitialization, true)
+EXPERIMENTAL_FEATURE(ConsumeSelfInDeinit, true)
 
 EXPERIMENTAL_FEATURE(OneWayClosureParameters, false)
 EXPERIMENTAL_FEATURE(LayoutPrespecialization, true)

lib/AST/FeatureSet.cpp

@@ -189,6 +189,7 @@ UNINTERESTING_FEATURE(DynamicActorIsolation)
 UNINTERESTING_FEATURE(NonfrozenEnumExhaustivity)
 UNINTERESTING_FEATURE(ClosureIsolation)
 UNINTERESTING_FEATURE(Extern)
+UNINTERESTING_FEATURE(ConsumeSelfInDeinit)
 
 static bool usesFeatureBitwiseCopyable2(Decl *decl) {
   if (!decl->getModuleContext()->isStdlibModule()) {

lib/SILOptimizer/Mandatory/MoveOnlyAddressCheckerUtils.cpp

@@ -1820,7 +1820,21 @@ shouldEmitPartialMutationError(UseState &useState, PartialMutation::Kind kind,
   LLVM_DEBUG(llvm::dbgs() << "    Iter Type: " << iterType << '\n'
                           << "    Target Type: " << targetType << '\n');
 
+  // Allowing full object consumption in a deinit is still not allowed.
   if (iterType == targetType && !isa<DropDeinitInst>(user)) {
+    // Don't allow whole-value consumption of `self` from a `deinit`.
+    if (!fn->getModule().getASTContext().LangOpts
+            .hasFeature(Feature::ConsumeSelfInDeinit)
+        && kind == PartialMutation::Kind::Consume
+        && useState.sawDropDeinit
+        // TODO: Revisit this when we introduce deinits on enums.
+        && !targetType.getEnumOrBoundGenericEnum()) {
+      LLVM_DEBUG(llvm::dbgs() << "    IterType is TargetType in deinit! "
+                                 "Not allowed yet");
+      
+      return {PartialMutationError::consumeDuringDeinit(iterType)};
+    }
+
     LLVM_DEBUG(llvm::dbgs() << "    IterType is TargetType! Exiting early "
                                "without emitting error!\n");
     return {};

lib/SILOptimizer/Mandatory/MoveOnlyDiagnostics.cpp

@@ -890,5 +890,11 @@ void DiagnosticEmitter::emitCannotPartiallyMutateError(
     registerDiagnosticEmitted(address);
     return;
   }
+  case PartialMutationError::Kind::ConsumeDuringDeinit: {
+    astContext.Diags.diagnose(user->getLoc().getSourceLoc(),
+                              diag::sil_movechecking_consume_during_deinit);
+    return;
   }
+  }
+  llvm_unreachable("unhandled case");
 }

lib/SILOptimizer/Mandatory/MoveOnlyTypeUtils.h

@@ -164,8 +164,10 @@ class PartialMutationError {
   struct NonfrozenUsableFromInlineType {
     NominalTypeDecl &nominal;
   };
+  struct ConsumeDuringDeinit {
+  };
   using Payload = TaggedUnion<FeatureDisabled, HasDeinit, NonfrozenImportedType,
-                              NonfrozenUsableFromInlineType>;
+                              NonfrozenUsableFromInlineType, ConsumeDuringDeinit>;
   Payload payload;
 
   PartialMutationError(SILType type, Payload payload)
@@ -212,6 +214,8 @@ public:
     /// with library evolution, prevent the aggregate from being partially
     /// mutated.
     NonfrozenUsableFromInlineType,
+    /// The value was fully consumed in a `deinit`.
+    ConsumeDuringDeinit,
   };
 
   operator Kind() {
@@ -221,8 +225,12 @@ public:
       return Kind::HasDeinit;
     else if (payload.isa<NonfrozenImportedType>())
       return Kind::NonfrozenImportedType;
-    assert(payload.isa<NonfrozenUsableFromInlineType>());
-    return Kind::NonfrozenUsableFromInlineType;
+    else if (payload.isa<NonfrozenUsableFromInlineType>())
+      return Kind::NonfrozenUsableFromInlineType;
+    else if (payload.isa<ConsumeDuringDeinit>())
+      return Kind::ConsumeDuringDeinit;
+    
+    llvm_unreachable("unhandled tag");
   }
 
   static PartialMutationError featureDisabled(SILType type,
@@ -245,6 +253,10 @@ public:
     return PartialMutationError(type, NonfrozenUsableFromInlineType{nominal});
   }
 
+  static PartialMutationError consumeDuringDeinit(SILType type) {
+    return PartialMutationError(type, ConsumeDuringDeinit{});
+  }
+
   PartialMutation::Kind getKind() {
     return payload.get<FeatureDisabled>().kind;
   }

test/SILOptimizer/deinit_total_consumption.swift

@@ -0,0 +1,104 @@
+// RUN: %target-swift-frontend -DEMPTY -emit-sil -verify %s
+// RUN: %target-swift-frontend -DTRIVIAL -emit-sil -verify %s
+// RUN: %target-swift-frontend -DLOADABLE -emit-sil -verify %s
+// RUN: %target-swift-frontend -DADDRESS_ONLY -emit-sil -verify %s
+
+// Don't allow whole-value consumption or mutation of a value in its own
+// `deinit`, but allow consumption of its individual fields.
+
+struct T1: ~Copyable {
+#if EMPTY
+#elseif TRIVIAL
+    var _x: Int
+#elseif LOADABLE
+    var _x: AnyObject
+#elseif ADDRESS_ONLY
+    var _x: Any
+#else
+    #error("pick one")
+#endif
+    deinit {
+        self.foo() // expected-error{{'self' cannot be consumed during 'deinit'}}
+    }
+
+    consuming func foo() {}
+}
+
+struct T2: ~Copyable {
+#if EMPTY
+#elseif TRIVIAL
+    var _x: Int
+#elseif LOADABLE
+    var _x: AnyObject
+#elseif ADDRESS_ONLY
+    var _x: Any
+#else
+    #error("pick one")
+#endif
+
+    deinit {
+        let myself = self // expected-error{{'self' cannot be consumed during 'deinit'}}
+        _ = myself
+    }
+}
+
+func consume<T: ~Copyable>(_: consuming T) {}
+
+struct NC<T: ~Copyable>: ~Copyable {
+    var x: T
+}
+
+struct T3: ~Copyable {
+#if EMPTY
+    var _x: NC<()>
+#elseif TRIVIAL
+    var _x: NC<Int>
+#elseif LOADABLE
+    var _x: NC<AnyObject>
+#elseif ADDRESS_ONLY
+    var _x: NC<Any>
+#else
+    #error("pick one")
+#endif
+
+    // consuming the single field of a one-field type is OK
+    deinit {
+        consume(_x)
+    }
+}
+
+struct T4: ~Copyable {
+#if EMPTY
+#elseif TRIVIAL
+    var _x: Int
+#elseif LOADABLE
+    var _x: AnyObject
+#elseif ADDRESS_ONLY
+    var _x: Any
+#else
+    #error("pick one")
+#endif
+
+    // the implicit cleanup of the value is OK
+    deinit {
+    }
+}
+
+struct T5: ~Copyable {
+#if EMPTY
+    var _x: NC<()>
+#elseif TRIVIAL
+    var _x: NC<Int>
+#elseif LOADABLE
+    var _x: NC<AnyObject>
+#elseif ADDRESS_ONLY
+    var _x: NC<Any>
+#else
+    #error("pick one")
+#endif
+
+    // the implicit cleanup of the value is OK
+    deinit {
+    }
+}
+

test/SILOptimizer/moveonly_deinits.swift

@@ -1,4 +1,4 @@
-// RUN: %target-swift-frontend -sil-verify-all -verify -emit-sil -enable-experimental-feature MoveOnlyEnumDeinits %s
+// RUN: %target-swift-frontend -sil-verify-all -verify -emit-sil -enable-experimental-feature MoveOnlyEnumDeinits -enable-experimental-feature ConsumeSelfInDeinit %s
 
 class Klass {}
 

test/SILOptimizer/moveonly_discard.swift

@@ -1,4 +1,4 @@
-// RUN: %target-swift-frontend -sil-verify-all -verify -emit-sil -enable-experimental-feature MoveOnlyEnumDeinits %s
+// RUN: %target-swift-frontend -sil-verify-all -verify -emit-sil -enable-experimental-feature MoveOnlyEnumDeinits -enable-experimental-feature ConsumeSelfInDeinit %s
 
 
 func posix_close(_ t: Int) {}