NCGenerics: limit field metadata heuristic

We have had a heuristic that lets you reflect fields of types that
are conditionally copyable, despite the reflection infrastructure
always copying the fields. This is really to allow types like
Optional in the stdlib continue to be reflected, despite having
been upgraded to support noncopyable types, because there are many
uses of that type with a Copyable payload.

As a consequence, we were also allowing reflection of user-defined
conditionally copyable types, when that's unsafe for no real benefit.
So, this patch limits the damage 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
+}