NCGenerics: limit field metadata heuristic

We have had a heuristic that lets you reflect fields of types, where
the field's type is conditionally Copyable, despite the reflection
infrastructure always copying a field. That heuristic was added to allow
ubiquitous types like Optional in the stdlib to continue to be
reflected, even if the Optional at runtime really isn't Copyable.

As a consequence, we were also allowing reflection of fields containing
user-defined conditionally copyable types, when that's unsafe for no
real benefit, yielding runtime crashes!

I think in that case it's better to fall-back on the non-crashy case of
reflection seeing it as the EmptyTupleType, which isn't inhabited, so it
won't try to copy the field and instead basically skip-over it until a
future runtime supports the reflection safely.

So, this patch limits the dangerous reflection to only stdlib-defined
types, until Mirror and friends are updated.

Author: kavon

lib/IRGen/GenReflection.cpp

@@ -437,21 +437,31 @@ getTypeRefImpl(IRGenModule &IGM,
 
     bool isAlwaysNoncopyable = false;
     if (contextualTy->isNoncopyable()) {
+      isAlwaysNoncopyable = true;
+
       // If the contextual type has any archetypes in it, it's plausible that
-      // we could end up with a copyable type in some instances. Look for those.
+      // we could end up with a copyable type in some instances. Look for those
+      // so we can permit unsafe reflection of the field, by assuming it could
+      // be Copyable.
       if (contextualTy->hasArchetype()) {
         // If this is a nominal type, check whether it can ever be copyable.
         if (auto nominal = contextualTy->getAnyNominal()) {
-          if (!nominal->canBeCopyable())
-            isAlwaysNoncopyable = true;
+          // If it's a nominal that can ever be Copyable _and_ it's defined in
+          // the stdlib, assume that we could end up with a Copyable type.
+          if (nominal->canBeCopyable()
+              && nominal->getModuleContext()->isStdlibModule())
+            isAlwaysNoncopyable = false;
         } else {
-          // Assume that we could end up with a copyable type somehow.
+          // Assume that we could end up with a Copyable type somehow.
+          // This allows you to reflect a 'T: ~Copyable' stored in a type.
+          isAlwaysNoncopyable = false;
         }
-      } else {
-        isAlwaysNoncopyable = true;
       }
     }
 
+    // The getTypeRefByFunction strategy will emit a forward-compatible runtime
+    // check to see if the runtime can safely reflect such fields. Otherwise,
+    // the field will be artificially hidden to reflectors.
     if (isAlwaysNoncopyable) {
       IGM.IRGen.noteUseOfTypeMetadata(type);
       return getTypeRefByFunction(IGM, sig, type);

test/IRGen/noncopyable_field_descriptors.swift

@@ -41,9 +41,9 @@ public struct NonCopyable: ~Copyable { }
 // 'MF' constant as a separator that precedes each field descriptor.
 
 // NEW: @"$s4test8CC_TestsCMF" =
-// NEW-SAME: @"symbolic _____yxG 4test21ConditionallyCopyableOAARi_zrlE"
+// NEW-SAME: @"get_type_metadata Ri_zr0_l4test21ConditionallyCopyableOyxG.3"
 // NEW-SAME: @"symbolic _____yq_G 4test21ConditionallyCopyableOAARi_zrlE"
-// NEW-SAME: @"get_type_metadata Ri_zr0_l4test21ConditionallyCopyableOyAA03NonC0VG.3"
+// NEW-SAME: @"get_type_metadata Ri_zr0_l4test21ConditionallyCopyableOyAA03NonC0VG.4"
 // NEW-SAME: @"symbolic _____ySSG 4test21ConditionallyCopyableOAARi_zrlE"
 
 // OLD: @"$s4test8CC_TestsCMF" =
@@ -64,10 +64,10 @@ public class CC_Tests<NCG: ~Copyable, T> {
 /// fields until a future runtime says they're safe to reflect.
 
 // NEW: @"$s4test8NC_TestsCMF" =
-// NEW-SAME: @"get_type_metadata Ri_zr0_l4test13NeverCopyableOyxG.4"
-// NEW-SAME: @"get_type_metadata Ri_zr0_l4test13NeverCopyableOyq_G.5"
-// NEW-SAME: @"get_type_metadata Ri_zr0_l4test13NeverCopyableOyAA03NonC0VG.6"
-// NEW-SAME: @"get_type_metadata Ri_zr0_l4test13NeverCopyableOySSG.7"
+// NEW-SAME: @"get_type_metadata Ri_zr0_l4test13NeverCopyableOyxG.5"
+// NEW-SAME: @"get_type_metadata Ri_zr0_l4test13NeverCopyableOyq_G.6"
+// NEW-SAME: @"get_type_metadata Ri_zr0_l4test13NeverCopyableOyAA03NonC0VG.7"
+// NEW-SAME: @"get_type_metadata Ri_zr0_l4test13NeverCopyableOySSG.8"
 
 // OLD: @"$s4test8NC_TestsCMF" =
 // OLD-SAME: @"get_type_metadata Ri_zr0_l4test13NeverCopyableOyxG.7"
@@ -85,7 +85,7 @@ public class NC_Tests<NCG: ~Copyable, T> {
 // NEW: @"$s4test17StdlibTypes_TestsCMF" =
 // NEW-SAME: @"symbolic xSg"
 // NEW-SAME: @"symbolic q_Sg"
-// NEW-SAME: @"get_type_metadata Ri_zr0_l4test11NonCopyableVSg.8"
+// NEW-SAME: @"get_type_metadata Ri_zr0_l4test11NonCopyableVSg.9"
 // NEW-SAME: @"symbolic SSSg"
 // NEW-SAME: @"symbolic SPyxG"
 // NEW-SAME: @"symbolic SPyq_G"
@@ -112,3 +112,29 @@ public class StdlibTypes_Tests<NCG: ~Copyable, T> {
   var upNC: UnsafePointer<NonCopyable> = .init(bitPattern: 64)!
   var upC: UnsafePointer<String> = .init(bitPattern: 128)!
 }
+
+
+// NEW: @"$s4test19PlainlyStored_TestsCMF" =
+// NEW-SAME: @"symbolic x"
+// NEW-SAME: @"symbolic q_"
+// NEW-SAME: @"get_type_metadata Ri_zr0_l4test11NonCopyableV.10"
+// NEW-SAME: @"symbolic SS"
+
+// OLD: @"$s4test19PlainlyStored_TestsCMF" =
+// OLD-SAME: @"symbolic x"
+// OLD-SAME: @"symbolic q_"
+// OLD-SAME: @"get_type_metadata Ri_zr0_l4test11NonCopyableV.12"
+// OLD-SAME: @"symbolic SS"
+public class PlainlyStored_Tests<NCG: ~Copyable, T> {
+  var ncg: NCG
+  var t: T
+  var concreteNC: NonCopyable
+  var str: String
+
+  public init(_ ncg: consuming NCG, _ t: T) {
+    self.ncg = ncg
+    self.t = t
+    self.concreteNC = NonCopyable()
+    self.str = ""
+  }
+}

test/Interpreter/moveonly_reflection.swift

@@ -0,0 +1,48 @@
+// RUN: %empty-directory(%t)
+// RUN: %target-build-swift -D CRASH %s -o %t/crash
+// RUN: %target-build-swift %s -o %t/exe
+
+// RUN: %target-run %t/exe | %FileCheck %s
+// RUN: %target-run not --crash %t/crash
+
+// REQUIRES: executable_test
+
+enum Maybe<Wrapped: ~Copyable>: ~Copyable {
+  case some(Wrapped)
+  case none
+}
+extension Maybe: Copyable where Wrapped: Copyable {}
+
+struct NC: ~Copyable {
+  let data: Int
+}
+
+class Protected<T: ~Copyable> {
+  var field: Maybe<T>
+  init(_ t: consuming T) {
+    self.field = .some(t)
+  }
+}
+
+class Dangerous<T: ~Copyable> {
+  var field: Optional<T>
+  init(_ t: consuming T) {
+    self.field = .some(t)
+  }
+}
+
+func printFields<T: Copyable>(_ val: T) {
+  let mirror = Mirror.init(reflecting: val)
+  mirror.children.forEach { print($0.label ?? "", $0.value) }
+}
+
+defer { test() }
+func test() {
+#if CRASH
+  printFields(Dangerous(NC(data: 22)))
+#else
+  printFields(Protected("oreo"))        // CHECK: field ()
+  printFields(Protected(NC(data: 11)))  // CHECK: field ()
+  printFields(Dangerous("spots"))       // CHECK: field Optional("spots")
+#endif
+}