Disallow consuming self in a noncopyable deinit again.

The changes to allow for partial consumption unintentionally also allowed for `self` to be consumed as a whole during `deinit`, which we don't yet want to allow because it could lead to accidental "resurrection" and/or accidental infinite recursion if the consuming method lets `deinit` be implicitly run again. This makes it an error again. The experimental feature `ConsumeSelfInDeinit` will allow it for test coverage or experimentation purposes. rdar://132761460

Author: jckarter

include/swift/AST/DiagnosticsSIL.def

@@ -876,6 +876,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

@@ -246,6 +246,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

@@ -695,6 +695,7 @@ UNINTERESTING_FEATURE(BorrowingSwitch)
 
 UNINTERESTING_FEATURE(ClosureIsolation)
 UNINTERESTING_FEATURE(Extern)
+UNINTERESTING_FEATURE(ConsumeSelfInDeinit)
 
 static bool usesFeatureConformanceSuppression(Decl *decl) {
  auto *nominal = dyn_cast<NominalTypeDecl>(decl);

lib/SILOptimizer/Mandatory/MoveOnlyAddressCheckerUtils.cpp

@@ -1816,7 +1816,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

@@ -889,5 +889,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 @@ class PartialMutationError {
  /// 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 @@ class PartialMutationError {
  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 @@ class PartialMutationError {
  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) {}