disallow "non-trivial" stored properties when using forget

We haven't quite got the semantics we want implemented
for `discard` aka `_forget` statements. Allowing people
to use `_forget` in noncopyable types that have stored
properties that require destruction will not let us
implement it the way we'd like in the future without
source break. But, if the type only has "trivial" stored
properties with a no-op destruction, like `Int`, then we
can still provide utility for users like FileDescriptor
who just want to disable the deinit and have nothing
fancy stored in the type itself.

rdar://108877261

Author: kavon

include/swift/AST/DiagnosticsSIL.def

@@ -735,6 +735,18 @@ ERROR(noimplicitcopy_used_on_generic_or_existential, none,
       "@_noImplicitCopy can not be used on a generic or existential typed "
       "binding or a nominal type containing such typed things", ())
 
+// forget statement
+ERROR(forget_nontrivial_storage,none,
+      "can only 'forget' type %0 if it contains trivially-destroyed "
+      "stored properties at this time",
+      (Type))
+NOTE(forget_nontrivial_storage_note,none,
+      "type %0 cannot be trivially destroyed",
+      (Type))
+NOTE(forget_nontrivial_implicit_storage_note,none,
+     "type %0 implicitly contains %1 which cannot be trivially destroyed",
+     (Type, Type))
+
 // move only checker diagnostics
 ERROR(sil_moveonlychecker_owned_value_consumed_more_than_once, none,
       "'%0' consumed more than once", (StringRef))

lib/SILGen/SILGenStmt.cpp

@@ -765,6 +765,36 @@ void StmtEmitter::visitForgetStmt(ForgetStmt *S) {
   auto *nominal = fn->getDeclContext()->getSelfNominalTypeDecl();
   assert(nominal);
 
+  // Check if the nominal's contents are trivial. This is a temporary
+  // restriction until we get forget implemented the way we want.
+  for (auto *varDecl : nominal->getStoredProperties()) {
+    assert(varDecl->hasStorage());
+    auto varType = varDecl->getType();
+    auto &varTypeLowering = SGF.getTypeLowering(varType);
+    if (!varTypeLowering.isTrivial()) {
+      diagnose(getASTContext(),
+               S->getStartLoc(),
+               diag::forget_nontrivial_storage,
+               nominal->getDeclaredInterfaceType());
+
+      // emit a note pointing out the problematic storage type
+      if (auto varLoc = varDecl->getLoc()) {
+        diagnose(getASTContext(),
+                 varLoc,
+                 diag::forget_nontrivial_storage_note,
+                 varType);
+      } else {
+        diagnose(getASTContext(),
+                 nominal->getLoc(),
+                 diag::forget_nontrivial_implicit_storage_note,
+                 nominal->getDeclaredInterfaceType(),
+                 varType);
+      }
+
+      break; // only one diagnostic is needed per forget
+    }
+  }
+
   SGF.emitMoveOnlyMemberDestruction(selfValue.forward(SGF), nominal, loc,
                                     nullptr);
 }

test/SILGen/forget.swift

@@ -62,10 +62,10 @@ func invokedDeinit() {}
 }
 
 @_moveOnly struct PointerTree {
-  let left: Ptr = Ptr()
-  let file: File = File()
-  let popularity: Int = 0
-  var right: Ptr = Ptr()
+  let left: UnsafePointer<UInt>?
+  let file: Int = 0
+  lazy var popularity: Int = 0
+  var right: Float = 0.0
 
   consuming func tryDestroy(doForget: Bool) throws {
     if doForget {
@@ -77,50 +77,40 @@ func invokedDeinit() {}
 // CHECK-LABEL: sil hidden [ossa] @$s4test11PointerTreeV10tryDestroy8doForgetySb_tKF : $@convention(method) (Bool, @owned PointerTree) -> @error any Error {
 // CHECK:   bb0{{.*}}:
 // CHECK:     [[SELF_BOX:%.*]] = alloc_box ${ var PointerTree }, var, name "self"
-// CHECK:     [[SELF:%.*]] = begin_borrow [lexical] [[SELF_BOX]] : ${ var PointerTree }
-// CHECK:     [[SELF_PTR:%.*]] = project_box [[SELF]] : ${ var PointerTree }, 0
+// CHECK:     [[SELF_PTR:%.*]] = project_box [[SELF_BOX]] : ${ var PointerTree }, 0
 //            .. skip to the conditional test ..
-// CHECK:     [[SHOULD_THROW:%.*]] = struct_extract {{.*}} : $Bool, #Bool._value
-// CHECK:     cond_br [[SHOULD_THROW]], bb1, bb2
+// CHECK:     [[SHOULD_FORGET:%.*]] = struct_extract {{.*}} : $Bool, #Bool._value
+// CHECK:     cond_br [[SHOULD_FORGET]], bb1, bb2
 //
 // CHECK:   bb1:
 // CHECK:     [[ACCESS:%.*]] = begin_access [read] [unknown] [[SELF_PTR]] : $*PointerTree
 // CHECK:     [[MMC:%.*]] = mark_must_check [no_consume_or_assign] [[ACCESS]] : $*PointerTree
 // CHECK:     [[COPIED_SELF:%.*]] = load [copy] [[MMC]] : $*PointerTree
 // CHECK:     end_access [[ACCESS]] : $*PointerTree
 // CHECK:     [[DD:%.*]] = drop_deinit [[COPIED_SELF]]
-// CHECK:     ([[LEFT:%.*]], [[FILE:%.*]], {{%.*}}, [[RIGHT:%.*]]) = destructure_struct [[DD]] : $PointerTree
-// CHECK:     destroy_value [[LEFT]] : $Ptr
-// CHECK:     destroy_value [[FILE]] : $File
-// CHECK:     destroy_value [[RIGHT]] : $Ptr
+// CHECK:     end_lifetime [[DD]]
 // CHECK:     br bb3
 //
 // CHECK:   bb2:
 // CHECK:     br bb3
 //
 // CHECK:   bb3:
-// CHECK:     end_borrow [[SELF]] : ${ var PointerTree }
 // CHECK:     destroy_value [[SELF_BOX]] : ${ var PointerTree }
 // CHECK:     throw
 // CHECK: } // end sil function
 
 // After the mandatory passes have run, check for correct deinitializations within the init.
 
 // CHECK-SIL-LABEL: sil hidden @$s4test11PointerTreeV10tryDestroy8doForgetySb_tKF
-// CHECK-SIL:     [[SHOULD_THROW:%.*]] = struct_extract {{.*}} : $Bool, #Bool._value
-// CHECK-SIL:     cond_br [[SHOULD_THROW]], bb1, bb2
+// CHECK-SIL:     [[SHOULD_FORGET:%.*]] = struct_extract {{.*}} : $Bool, #Bool._value
+// CHECK-SIL:     cond_br [[SHOULD_FORGET]], bb1, bb2
 //
 // CHECK-SIL:  bb1:
 // CHECK-SIL:     [[ACCESS:%.*]] = begin_access [modify] [static] {{.*}} : $*PointerTree
 // CHECK-SIL:     [[SELF_VAL:%.*]] = load [[ACCESS]] : $*PointerTree
 // CHECK-SIL:     end_access [[ACCESS]] : $*PointerTree
-// CHECK-SIL:     [[LEFT:%.*]] = struct_extract [[SELF_VAL]] : $PointerTree, #PointerTree.left
-// CHECK-SIL:     [[FILE:%.*]] = struct_extract [[SELF_VAL]] : $PointerTree, #PointerTree.file
-// CHECK-SIL:     [[RIGHT:%.*]] = struct_extract [[SELF_VAL]] : $PointerTree, #PointerTree.right
-// CHECK-SIL:     strong_release [[LEFT]] : $Ptr
-// CHECK-SIL:     [[FILE_DEINIT:%.*]] = function_ref @$s4test4FileVfD : $@convention(method) (@owned File) -> ()
-// CHECK-SIL:     apply [[FILE_DEINIT]]([[FILE]])
-// CHECK-SIL:     strong_release [[RIGHT]] : $Ptr
+// CHECK-SIL-NOT: struct_extract
+                  // no accesses to the fields are expected because the fields are no-op destroyed.
 // CHECK-SIL:     br bb3
 //
 // CHECK-SIL:  bb2:

test/Sema/forget_trivially_destroyed.swift

@@ -0,0 +1,100 @@
+// RUN: %target-swift-emit-silgen -verify %s
+
+// Yes, you read that right. This is being checked in SILGen! So we have to
+// separate these tests from ones emitting errors during Sema.
+
+class ClassyGal {}
+
+@propertyWrapper
+struct Appending<Val> {
+  let terminator: Val
+  var store: [Val] = []
+
+  var wrappedValue: [Val] {
+    get {
+      return store + [terminator]
+    }
+    set {
+      fatalError("mutation is the root of all bugs")
+    }
+  }
+}
+
+struct StructuredGuy: ~Copyable {
+  let x = ClassyGal()
+  // expected-note@-1 {{type 'ClassyGal' cannot be trivially destroyed}}
+
+  @Appending(terminator: ClassyGal())
+  var bestC: [ClassyGal]
+
+  consuming func doForget() { _forget self }
+  // expected-error@-1 {{can only 'forget' type 'StructuredGuy' if it contains trivially-destroyed stored properties at this time}}
+  deinit {}
+}
+
+struct AppendyEnby: ~Copyable {
+  @Appending(terminator: 0)
+  var string: [Int]
+  // expected-note@-1 {{type 'Appending<Int>' cannot be trivially destroyed}}
+
+
+  consuming func doForget() { _forget self }
+  // expected-error@-1 {{can only 'forget' type 'AppendyEnby' if it contains trivially-destroyed stored properties at this time}}
+  deinit {}
+}
+
+struct HasGeneric: ~Copyable {
+  var thing: Any // expected-note {{type 'Any' cannot be trivially destroyed}}
+
+  consuming func forget() { _forget self }
+  // expected-error@-1 {{can only 'forget' type 'HasGeneric' if it contains trivially-destroyed stored properties at this time}}
+
+  deinit{}
+}
+
+struct WrappingNoncopyable: ~Copyable {
+  var computed: String { "mine" }
+
+  var x: AppendyEnby // expected-note{{type 'AppendyEnby' cannot be trivially destroyed}}
+  consuming func doForget() { _forget self }
+  // expected-error@-1 {{can only 'forget' type 'WrappingNoncopyable' if it contains trivially-destroyed stored properties at this time}}
+  deinit {}
+}
+
+struct LazyGuy: ~Copyable {
+  lazy var thing: String = "asdf"
+  // expected-note@-1 {{type 'String?' cannot be trivially destroyed}}
+
+  consuming func doForget() { _forget self }
+  // expected-error@-1 {{can only 'forget' type 'LazyGuy' if it contains trivially-destroyed stored properties at this time}}
+  deinit {}
+}
+
+struct BoringNoncopyable: ~Copyable {
+  let x = 0
+  let y = 0
+}
+
+struct Boring {
+  var x = 0
+  var y = 1.9
+}
+
+// FIXME: Despite not having a deinit, the noncopyable struct isn't considered trivial?
+struct ContainsBoring: ~Copyable {
+  let z: BoringNoncopyable // expected-note {{type 'BoringNoncopyable' cannot be trivially destroyed}}
+  consuming func forget() { _forget self }
+  // expected-error@-1 {{can only 'forget' type 'ContainsBoring' if it contains trivially-destroyed stored properties at this time}}
+  deinit {}
+}
+
+struct AllOK: ~Copyable {
+  var maybeDigits: Int?
+  var maybeFloat: Float?
+  let dbl: Double
+  var location: Boring = Boring()
+  var unsafePtr: UnsafePointer<Int>
+
+  consuming func doForget() { _forget self }
+  deinit {}
+}