Merge pull request #4060 from bitjammer/weak-reference-in-mirror-existential

Always, no, never... forget to check your references

Author: bitjammer

include/swift/ABI/MetadataValues.h

@@ -520,6 +520,7 @@ class FieldType {
   // some high bits as well.
   enum : int_type {
     Indirect = 1,
+    Weak = 2,
 
     TypeMask = ((uintptr_t)-1) & ~(alignof(void*) - 1),
   };
@@ -537,10 +538,19 @@ class FieldType {
                      | (indirect ? Indirect : 0));
   }
 
+  constexpr FieldType withWeak(bool weak) const {
+    return FieldType((Data & ~Weak)
+                     | (weak ? Weak : 0));
+  }
+
   bool isIndirect() const {
     return bool(Data & Indirect);
   }
 
+  bool isWeak() const {
+    return bool(Data & Weak);
+  }
+
   const Metadata *getType() const {
     return (const Metadata *)(Data & TypeMask);
   }

include/swift/Runtime/HeapObject.h

@@ -899,8 +899,8 @@ extern "C" void *swift_unknownWeakLoadStrong(WeakReference *ref);
 
 #else
 
-static inline void swift_unknownWeakLoadStrong(WeakReference *ref) {
-  swift_weakLoadStrong(ref);
+static inline void *swift_unknownWeakLoadStrong(WeakReference *ref) {
+  return static_cast<void *>(swift_weakLoadStrong(ref));
 }
 
 #endif /* SWIFT_OBJC_INTEROP */
@@ -918,8 +918,8 @@ extern "C" void *swift_unknownWeakTakeStrong(WeakReference *ref);
 
 #else
 
-static inline void swift_unknownWeakTakeStrong(WeakReference *ref) {
-  swift_weakTakeStrong(ref);
+static inline void *swift_unknownWeakTakeStrong(WeakReference *ref) {
+  return static_cast<void *>(swift_weakTakeStrong(ref));
 }
 
 #endif /* SWIFT_OBJC_INTEROP */

lib/IRGen/GenMeta.cpp

@@ -2414,8 +2414,10 @@ namespace {
                          NominalTypeDecl::StoredPropertyRange storedProperties){
     SmallVector<FieldTypeInfo, 4> types;
     for (VarDecl *prop : storedProperties) {
-      types.push_back(FieldTypeInfo(prop->getType()->getCanonicalType(),
-                                    /*indirect*/ false));
+      auto propertyType = prop->getType()->getCanonicalType();
+      types.push_back(FieldTypeInfo(propertyType,
+                                    /*indirect*/ false,
+                                    propertyType->is<WeakStorageType>()));
     }
     return getFieldTypeAccessorFn(IGM, type, types);
   }
@@ -2431,7 +2433,7 @@ namespace {
       auto caseType = elt.decl->getArgumentType()->getCanonicalType();
       bool isIndirect = elt.decl->isIndirect()
         || elt.decl->getParentEnum()->isIndirect();
-      types.push_back(FieldTypeInfo(caseType, isIndirect));
+      types.push_back(FieldTypeInfo(caseType, isIndirect, /*weak*/ false));
     }
     return getFieldTypeAccessorFn(IGM, type, types);
   }
@@ -2784,9 +2786,13 @@ irgen::emitFieldTypeAccessor(IRGenModule &IGM,
 
     auto metadata = IGF.emitTypeMetadataRef(fieldTy);
 
+    auto fieldTypeInfo = fieldTypes[i];
+
     // Mix in flag bits.
-    if (fieldTypes[i].isIndirect()) {
-      auto flags = FieldType().withIndirect(true);
+    if (fieldTypeInfo.hasFlags()) {
+      auto flags = FieldType()
+        .withIndirect(fieldTypeInfo.isIndirect())
+        .withWeak(fieldTypeInfo.isWeak());
       auto metadataBits = IGF.Builder.CreatePtrToInt(metadata, IGF.IGM.SizeTy);
       metadataBits = IGF.Builder.CreateOr(metadataBits,
                    llvm::ConstantInt::get(IGF.IGM.SizeTy, flags.getIntValue()));

lib/IRGen/IRGenModule.h

@@ -133,25 +133,30 @@ class IRGenModule;
 
 /// A type descriptor for a field type accessor.
 class FieldTypeInfo {
-  llvm::PointerIntPair<CanType, 1, unsigned> Info;
+  llvm::PointerIntPair<CanType, 2, unsigned> Info;
   /// Bits in the "int" part of the Info pair.
   enum : unsigned {
     /// Flag indicates that the case is indirectly stored in a box.
     Indirect = 1,
+    /// Indicates a weak optional reference
+    Weak = 2,
   };
 
-  static unsigned getFlags(bool indirect) {
-    return (indirect ? Indirect : 0);
+  static unsigned getFlags(bool indirect, bool weak) {
+    return (indirect ? Indirect : 0)
+         | (weak ? Weak : 0);
     //   | (blah ? Blah : 0) ...
   }
 
 public:
-  FieldTypeInfo(CanType type, bool indirect)
-    : Info(type, getFlags(indirect))
+  FieldTypeInfo(CanType type, bool indirect, bool weak)
+    : Info(type, getFlags(indirect, weak))
   {}
 
   CanType getType() const { return Info.getPointer(); }
   bool isIndirect() const { return Info.getInt() & Indirect; }
+  bool isWeak() const { return Info.getInt() & Weak; }
+  bool hasFlags() const { return Info.getInt() != 0; }
 };
 
 /// The principal singleton which manages all of IR generation.

stdlib/public/runtime/Reflection.mm

@@ -398,6 +398,40 @@ void swift_TupleMirror_subscript(String *outString,
   return fieldName;
 }
 
+
+static bool loadSpecialReferenceStorage(HeapObject *owner,
+                                        OpaqueValue *fieldData,
+                                        const FieldType fieldType,
+                                        Mirror *outMirror) {
+  // TODO: Switch over the other kinds of reference storage here:
+  // - unowned(safe)
+  // - unowned(unsafe)
+  // - class-bound existentials
+  // - Optional existential types
+  // This will require a change in the two low flag bits in the field type
+  // returned from the field type accessor generated by IRGen.
+
+  // isWeak() implies a reference type via Sema.
+  if (!fieldType.isWeak())
+    return false;
+
+  auto weakField = reinterpret_cast<WeakReference *>(fieldData);
+  auto strongValue = swift_unknownWeakLoadStrong(weakField);
+  fieldData = reinterpret_cast<OpaqueValue *>(&strongValue);
+
+  // This MagicMirror constructor creates a box to hold the loaded refernce
+  // value, which becomes the new owner for the value.
+  new (outMirror) MagicMirror(fieldData, fieldType.getType(), /*take*/ true);
+
+  // However, swift_StructMirror_subscript and swift_ClassMirror_subscript
+  // requires that the owner be consumed. Since we have the new heap box as the
+  // owner now, we need to release the old owner to maintain the contract.
+  if (owner->metadata->isAnyClass())
+    swift_unknownRelease(owner);
+
+  return true;
+}
+
 // -- Struct destructuring.
 
 SWIFT_CC(swift) SWIFT_RUNTIME_STDLIB_INTERFACE
@@ -416,7 +450,7 @@ void swift_StructMirror_subscript(String *outString,
                                   Mirror *outMirror,
                                   intptr_t i,
                                   HeapObject *owner,
-                                  const OpaqueValue *value,
+                                  OpaqueValue *value,
                                   const Metadata *type) {
   auto Struct = static_cast<const StructMetadata *>(type);
 
@@ -427,13 +461,17 @@ void swift_StructMirror_subscript(String *outString,
   auto fieldType = Struct->getFieldTypes()[i];
   auto fieldOffset = Struct->getFieldOffsets()[i];
 
-  auto bytes = reinterpret_cast<const char*>(value);
-  auto fieldData = reinterpret_cast<const OpaqueValue *>(bytes + fieldOffset);
+  auto bytes = reinterpret_cast<char*>(value);
+  auto fieldData = reinterpret_cast<OpaqueValue *>(bytes + fieldOffset);
 
   new (outString) String(getFieldName(Struct->Description->Struct.FieldNames, i));
 
   // 'owner' is consumed by this call.
   assert(!fieldType.isIndirect() && "indirect struct fields not implemented");
+
+  if (loadSpecialReferenceStorage(owner, fieldData, fieldType, outMirror))
+    return;
+
   new (outMirror) Mirror(reflect(owner, fieldData, fieldType.getType()));
 }
 
@@ -614,7 +652,7 @@ void swift_ClassMirror_subscript(String *outString,
                                  Mirror *outMirror,
                                  intptr_t i,
                                  HeapObject *owner,
-                                 const OpaqueValue *value,
+                                 OpaqueValue *value,
                                  const Metadata *type) {
   auto Clas = static_cast<const ClassMetadata*>(type);
 
@@ -655,10 +693,15 @@ void swift_ClassMirror_subscript(String *outString,
 #endif
   }
 
-  auto bytes = *reinterpret_cast<const char * const*>(value);
-  auto fieldData = reinterpret_cast<const OpaqueValue *>(bytes + fieldOffset);
-  
-  new (outString) String(getFieldName(Clas->getDescription()->Class.FieldNames, i));
+  auto bytes = *reinterpret_cast<char * const *>(value);
+  auto fieldData = reinterpret_cast<OpaqueValue *>(bytes + fieldOffset);
+
+  new (outString) String(getFieldName(Clas->getDescription()->Class.FieldNames,
+                                      i));
+
+ if (loadSpecialReferenceStorage(owner, fieldData, fieldType, outMirror))
+   return;
+
   // 'owner' is consumed by this call.
   new (outMirror) Mirror(reflect(owner, fieldData, fieldType.getType()));
 }