[wip] Remove the Swift bit hack

Turns out that it won’t work for Objective-C subclasses; the superclass realization code path doesn’t try to handle Swift bits or metadata updaters in any way. We’ll need a change to the ObjC runtime to fix that issue, and as long as we’re doing that we might as well support metadata updaters without the Swift bit, so remove the Swift bit hack and add an availability check instead.

This will break the resilient parts of the execution tests, hence the [wip].

Author: beccadax

include/swift/AST/DiagnosticsIRGen.def

@@ -69,5 +69,11 @@ NOTE(layout_strings_blocked,none,
      "Layout string value witnesses have been disabled for module '%0' "
      "through block list entry", (StringRef))
 
+ERROR(attr_objc_implementation_resilient_property_unsupported, none,
+      "'@implementation' on %0 %1 does not support stored properties whose "
+      "size can change due to library evolution; raise the minimum deployment "
+      "target to %0 %2 or store this value in an object or 'any' type",
+      (StringRef, const llvm::VersionTuple, const llvm::VersionTuple))
+
 #define UNDEFINE_DIAGNOSTIC_MACROS
 #include "DefineDiagnosticMacros.h"

include/swift/AST/FeatureAvailability.def

@@ -74,6 +74,7 @@ FEATURE(IsolatedAny,                                    (5, 11))
 FEATURE(TaskExecutor,                                   FUTURE)
 FEATURE(Differentiation,                                FUTURE)
 FEATURE(InitRawStructMetadata,                          FUTURE)
+FEATURE(UpdatePureObjCClassMetadata,                    FUTURE)
 
 #undef FEATURE
 #undef FUTURE

lib/IRGen/GenMeta.cpp

@@ -4309,6 +4309,9 @@ namespace {
     llvm::Constant *getROData() { return emitClassPrivateData(IGM, Target); }
 
     uint64_t getClassDataPointerHasSwiftMetadataBits() {
+      // objcImpl classes should not have the Swift bit set.
+      if (isPureObjC())
+        return 0;
       return IGM.UseDarwinPreStableABIBit ? 1 : 2;
     }
 
@@ -4324,20 +4327,13 @@ namespace {
       // Derive the RO-data.
       llvm::Constant *data = asImpl().getROData();
 
-      // Fixed-layout objcImpl classes should not have the Swift bit set.
-      // Variable-layout objcImpl classes have the Swift bit set *temporarily*
-      // so that the ObjC runtime realizes them as though they are Swift
-      // classes, but the metadata update callback will clear the Swift bit
-      // before the rest of the ObjC runtime starts looking at the class.
-      if (!isPureObjC() || !hasFixedLayout()) {
-        // Set a low bit to indicate this class has Swift metadata.
-        auto bit = llvm::ConstantInt::get(
-            IGM.IntPtrTy, asImpl().getClassDataPointerHasSwiftMetadataBits());
-
-        // Emit data + bit.
-        data = llvm::ConstantExpr::getPtrToInt(data, IGM.IntPtrTy);
-        data = llvm::ConstantExpr::getAdd(data, bit);
-      }
+      // Set a low bit to indicate this class has Swift metadata.
+      auto bit = llvm::ConstantInt::get(
+          IGM.IntPtrTy, asImpl().getClassDataPointerHasSwiftMetadataBits());
+
+      // Emit data + bit.
+      data = llvm::ConstantExpr::getPtrToInt(data, IGM.IntPtrTy);
+      data = llvm::ConstantExpr::getAdd(data, bit);
 
       B.add(data);
     }
@@ -5016,6 +5012,7 @@ namespace {
     }
 
     uint64_t getClassDataPointerHasSwiftMetadataBits() {
+      assert(!isPureObjC());
       return super::getClassDataPointerHasSwiftMetadataBits() | 2;
     }
 
@@ -5060,13 +5057,81 @@ static void emitObjCClassSymbol(IRGenModule &IGM, ClassDecl *classDecl,
       .to(alias, link.isForDefinition());
 }
 
+/// Check whether the metadata update strategy requires runtime support that is
+/// not guaranteed available by the minimum deployment target, and diagnose if
+/// so.
+static void
+diagnoseUnsupportedObjCImplLayout(IRGenModule &IGM, ClassDecl *classDecl,
+                                  const ClassLayout &fragileLayout) {
+  if (!fragileLayout.hasObjCImplementation())
+    return;
+
+  auto strategy = IGM.getClassMetadataStrategy(classDecl);
+
+  switch (strategy) {
+  case ClassMetadataStrategy::Fixed:
+    // Fixed is just fine; no special support needed.
+    break;
+
+  case ClassMetadataStrategy::FixedOrUpdate:
+  case ClassMetadataStrategy::Update: {
+    auto &ctx = IGM.Context;
+
+    // Update and FixedOrUpdate require support in both the Swift and ObjC
+    // runtimes.
+    auto requiredAvailability=ctx.getUpdatePureObjCClassMetadataAvailability();
+    // FIXME: Take the class's availability into account
+    auto currentAvailability = AvailabilityContext::forDeploymentTarget(ctx);
+    if (currentAvailability.isContainedIn(requiredAvailability))
+      break;
+
+    // We don't have the support we need. Find and diagnose the variable-size
+    // stored properties.
+    auto &diags = ctx.Diags;
+
+    bool diagnosed = false;
+    forEachField(IGM, classDecl, [&](Field field) {
+      auto elemLayout = fragileLayout.getFieldAccessAndElement(field).second;
+      if (field.getKind() != Field::Kind::Var ||
+            elemLayout.getType().isFixedSize())
+        return;
+
+      diags.diagnose(
+          field.getVarDecl(),
+          diag::attr_objc_implementation_resilient_property_unsupported,
+          prettyPlatformString(targetPlatform(ctx.LangOpts)),
+          currentAvailability.getOSVersion().getLowerEndpoint(),
+          requiredAvailability.getOSVersion().getLowerEndpoint());
+      diagnosed = true;
+    });
+
+    // We should have found at least one property to complain about.
+    if (diagnosed)
+      break;
+
+    LLVM_FALLTHROUGH;
+  }
+
+  case ClassMetadataStrategy::Singleton:
+  case ClassMetadataStrategy::Resilient:
+    // This isn't supposed to happen, but just in case, let's give some sort of
+    // vaguely legible output instead of using llvm_unreachable().
+    IGM.error(classDecl->getLoc(),
+              llvm::Twine("class '") + classDecl->getBaseIdentifier().str() +
+              "' needs a metadata strategy not supported by @implementation");
+    break;
+  }
+}
+
 /// Emit the type metadata or metadata template for a class.
 void irgen::emitClassMetadata(IRGenModule &IGM, ClassDecl *classDecl,
                               const ClassLayout &fragileLayout,
                               const ClassLayout &resilientLayout) {
   assert(!classDecl->isForeign());
   PrettyStackTraceDecl stackTraceRAII("emitting metadata for", classDecl);
 
+  diagnoseUnsupportedObjCImplLayout(IGM, classDecl, fragileLayout);
+
   emitFieldOffsetGlobals(IGM, classDecl, fragileLayout, resilientLayout);
 
   // Set up a dummy global to stand in for the metadata object while we produce

stdlib/public/runtime/Metadata.cpp

@@ -3980,16 +3980,6 @@ swift::swift_updatePureObjCClassMetadata(Class cls,
                                          ClassLayoutFlags flags,
                                          size_t numFields,
                                          const TypeLayout * const *fieldTypes) {
-  auto self = (ObjCClass *)cls;
-
-  // Realize the superclass first.
-  (void)swift_getInitializedObjCClass((Class)self->Isa);
-
-  // The Swift bit has been set so that we use Swift class initialization paths,
-  // but from here on in, we want to treat this as a pure ObjC class. Clear it.
-  auto rodata = getROData(self);
-  self->RODataAndFlags = (uintptr_t)rodata;
-
   bool requiresRealizeClassFromSwift =
     SWIFT_RUNTIME_WEAK_CHECK(_objc_realizeClassFromSwift);
   assert(requiresRealizeClassFromSwift);
@@ -4002,7 +3992,10 @@ swift::swift_updatePureObjCClassMetadata(Class cls,
 
   // Update the field offset globals using runtime type information; the layout
   // of resilient types might be different than the statically-emitted layout.
+  ObjCClass *self = (ObjCClass *)cls;
+  ClassROData *rodata = getROData(self);
   ClassIvarList *ivars = rodata->IvarList;
+  
   if (!ivars) {
     assert(numFields == 0);
     return cls;

test/IRGen/objc_implementation.swift

@@ -1,7 +1,7 @@
 // Test doesn't pass on all platforms (rdar://101420862)
 // REQUIRES: OS=macosx
 
-// RUN: %target-swift-frontend(mock-sdk: %clang-importer-sdk) -I %S/Inputs/abi -F %clang-importer-sdk-path/frameworks %s -import-objc-header %S/Inputs/objc_implementation.h -emit-ir > %t.ir
+// RUN: %target-swift-frontend(mock-sdk: %clang-importer-sdk) -I %S/Inputs/abi -F %clang-importer-sdk-path/frameworks %s -import-objc-header %S/Inputs/objc_implementation.h -emit-ir -target %target-future-triple > %t.ir
 // RUN: %FileCheck --input-file %t.ir %s
 // RUN: %FileCheck --input-file %t.ir --check-prefix NEGATIVE %s
 // REQUIRES: objc_interop
@@ -17,7 +17,7 @@
 
 // Metaclass
 // CHECK: @"OBJC_METACLASS_$_ImplClass" = global %objc_class { ptr @"OBJC_METACLASS_$_NSObject", ptr @"OBJC_METACLASS_$_NSObject", ptr @_objc_empty_cache, ptr null, {{.*}}ptr [[_METACLASS_DATA_ImplClass:@[^, ]+]]{{.*}}, align 8
-// CHECK: @_PROTOCOLS_ImplClass = internal constant { i64, [2 x ptr] } { i64 2, [2 x ptr] [ptr @_PROTOCOL_NSCopying, ptr @_PROTOCOL_NSMutableCopying] }, section "__DATA, __objc_const", align 8
+// CHECK: @_PROTOCOLS_ImplClass = internal constant { i64, [2 x ptr] } { i64 2, [2 x ptr] [ptr @"_OBJC_PROTOCOL_$_NSCopying", ptr @"_OBJC_PROTOCOL_$_NSMutableCopying"] }, section "__DATA, __objc_const", align 8
 // CHECK: [[_METACLASS_DATA_ImplClass]] = internal constant { i32, i32, i32, i32, ptr, ptr, ptr, ptr, ptr, ptr, ptr } { i32 129, i32 40, i32 40, i32 0, ptr null, ptr @.str.9.ImplClass, ptr null, ptr @_PROTOCOLS_ImplClass, ptr null, ptr null, ptr null }, section "__DATA, __objc_const", align 8
 // TODO: Why the extra i32 field above?
 
@@ -121,7 +121,7 @@
 // CHECK: @"$s19objc_implementationMXM" = linkonce_odr hidden constant <{ i32, i32, i32 }> <{ i32 0, i32 0, i32 trunc (i64 sub (i64 ptrtoint (ptr @.str.19.objc_implementation to i64), i64 ptrtoint (ptr getelementptr inbounds (<{ i32, i32, i32 }>, ptr @"$s19objc_implementationMXM", i32 0, i32 2) to i64)) to i32) }>, section "__TEXT,__constg_swiftt", align 4
 // CHECK: @"symbolic So9ImplClassC" = linkonce_odr hidden constant <{ [13 x i8], i8 }> <{ [13 x i8] c"So9ImplClassC", i8 0 }>, section "__TEXT,__swift5_typeref, regular"{{.*}}, align 2
 // CHECK: @"$s19objc_implementation13SwiftSubclassCMn" = constant <{ i32, i32, i32, i32, i32, i32, i32, i32, i32, i32, i32 }> <{ i32 80, i32 trunc (i64 sub (i64 ptrtoint (ptr @"$s19objc_implementationMXM" to i64), i64 ptrtoint (ptr getelementptr inbounds (<{ i32, i32, i32, i32, i32, i32, i32, i32, i32, i32, i32 }>, ptr @"$s19objc_implementation13SwiftSubclassCMn", i32 0, i32 1) to i64)) to i32), i32 trunc (i64 sub (i64 ptrtoint (ptr @.str.13.SwiftSubclass to i64), i64 ptrtoint (ptr getelementptr inbounds (<{ i32, i32, i32, i32, i32, i32, i32, i32, i32, i32, i32 }>, ptr @"$s19objc_implementation13SwiftSubclassCMn", i32 0, i32 2) to i64)) to i32), i32 trunc (i64 sub (i64 ptrtoint (ptr @"$s19objc_implementation13SwiftSubclassCMa" to i64), i64 ptrtoint (ptr getelementptr inbounds (<{ i32, i32, i32, i32, i32, i32, i32, i32, i32, i32, i32 }>, ptr @"$s19objc_implementation13SwiftSubclassCMn", i32 0, i32 3) to i64)) to i32), i32 trunc (i64 sub (i64 ptrtoint (ptr @"$s19objc_implementation13SwiftSubclassCMF" to i64), i64 ptrtoint (ptr getelementptr inbounds (<{ i32, i32, i32, i32, i32, i32, i32, i32, i32, i32, i32 }>, ptr @"$s19objc_implementation13SwiftSubclassCMn", i32 0, i32 4) to i64)) to i32), i32 trunc (i64 sub (i64 ptrtoint (ptr @"symbolic So9ImplClassC" to i64), i64 ptrtoint (ptr getelementptr inbounds (<{ i32, i32, i32, i32, i32, i32, i32, i32, i32, i32, i32 }>, ptr @"$s19objc_implementation13SwiftSubclassCMn", i32 0, i32 5) to i64)) to i32), i32 3, i32 10, i32 0, i32 0, i32 10 }>, section "__TEXT,__constg_swiftt", align 4
-// CHECK: @"$s19objc_implementation13SwiftSubclassCMf" = internal global <{ ptr, ptr, ptr, i64, ptr, ptr, ptr, i64, i32, i32, i32, i16, i16, i32, i32, ptr, ptr }> <{ ptr null, ptr @"$s19objc_implementation13SwiftSubclassCfD", ptr @"$sBOWV", i64 ptrtoint (ptr @"OBJC_METACLASS_$__TtC19objc_implementation13SwiftSubclass" to i64), ptr @"OBJC_CLASS_$_ImplClass", ptr @_objc_empty_cache, ptr null, i64 add (i64 ptrtoint (ptr @_DATA__TtC19objc_implementation13SwiftSubclass to i64), i64 1), i32 0, i32 0, i32 24, i16 7, i16 0, i32 104, i32 24, ptr @"$s19objc_implementation13SwiftSubclassCMn", ptr null }>, section "__DATA,__objc_data, regular", align 8
+// CHECK: @"$s19objc_implementation13SwiftSubclassCMf" = internal global <{ ptr, ptr, ptr, i64, ptr, ptr, ptr, i64, i32, i32, i32, i16, i16, i32, i32, ptr, ptr }> <{ ptr null, ptr @"$s19objc_implementation13SwiftSubclassCfD", ptr @"$sBOWV", i64 ptrtoint (ptr @"OBJC_METACLASS_$__TtC19objc_implementation13SwiftSubclass" to i64), ptr @"OBJC_CLASS_$_ImplClass", ptr @_objc_empty_cache, ptr null, i64 add (i64 ptrtoint (ptr @_DATA__TtC19objc_implementation13SwiftSubclass to i64), i64 2), i32 0, i32 0, i32 24, i16 7, i16 0, i32 104, i32 24, ptr @"$s19objc_implementation13SwiftSubclassCMn", ptr null }>, section "__DATA,__objc_data, regular", align 8
 // CHECK: @"symbolic _____ 19objc_implementation13SwiftSubclassC" = linkonce_odr hidden constant <{ i8, i32, i8 }> <{ i8 1, i32 trunc (i64 sub (i64 ptrtoint (ptr @"$s19objc_implementation13SwiftSubclassCMn" to i64), i64 ptrtoint (ptr getelementptr inbounds (<{ i8, i32, i8 }>, ptr @"symbolic _____ 19objc_implementation13SwiftSubclassC", i32 0, i32 1) to i64)) to i32), i8 0 }>, section "__TEXT,__swift5_typeref, regular"{{.*}}, align 2
 // CHECK: @"$s19objc_implementation13SwiftSubclassCMF" = internal constant { i32, i32, i16, i16, i32 } { i32 trunc (i64 sub (i64 ptrtoint (ptr @"symbolic _____ 19objc_implementation13SwiftSubclassC" to i64), i64 ptrtoint (ptr @"$s19objc_implementation13SwiftSubclassCMF" to i64)) to i32), i32 trunc (i64 sub (i64 ptrtoint (ptr @"symbolic So9ImplClassC" to i64), i64 ptrtoint (ptr getelementptr inbounds ({ i32, i32, i16, i16, i32 }, ptr @"$s19objc_implementation13SwiftSubclassCMF", i32 0, i32 1) to i64)) to i32), i16 7, i16 12, i32 0 }, section "__TEXT,__swift5_fieldmd, regular"{{.*}}, align 4
 open class SwiftSubclass: ImplClass {
@@ -142,7 +142,7 @@ open class SwiftSubclass: ImplClass {
 // Class
 // CHECK: @_IVARS_ImplClassWithResilientStoredProperty = internal constant { i32, i32, [4 x { ptr, ptr, ptr, i32, i32 }] } { i32 32, i32 4, [4 x { ptr, ptr, ptr, i32, i32 }] [{ ptr, ptr, ptr, i32, i32 } { ptr @"$sSo36ImplClassWithResilientStoredPropertyC19objc_implementationE9beforeInts5Int32VvpWvd", ptr @.str.9.beforeInt, ptr @.str.0., i32 2, i32 4 }, { ptr, ptr, ptr, i32, i32 } { ptr @"$sSo36ImplClassWithResilientStoredPropertyC19objc_implementationE6mirrors6MirrorVSgvpWvd", ptr @.str.6.mirror, ptr @.str.0., i32 0, i32 0 }, { ptr, ptr, ptr, i32, i32 } { ptr @"$sSo36ImplClassWithResilientStoredPropertyC19objc_implementationE13afterIntFinals5Int32VvpWvd", ptr @.str.13.afterIntFinal, ptr @.str.0., i32 2, i32 4 }, { ptr, ptr, ptr, i32, i32 } { ptr @"$sSo36ImplClassWithResilientStoredPropertyC19objc_implementationE8afterInts5Int32VvpWvd", ptr @.str.8.afterInt, ptr @.str.0., i32 2, i32 4 }] }, section "__DATA, __objc_const", align 8
 // CHECK: @_DATA_ImplClassWithResilientStoredProperty = internal constant { i32, i32, i32, i32, ptr, ptr, ptr, ptr, ptr, ptr, ptr, ptr } { i32 452, i32 8, i32 20, i32 0, ptr null, ptr @.str.36.ImplClassWithResilientStoredProperty, ptr @_INSTANCE_METHODS_ImplClassWithResilientStoredProperty, ptr null, ptr @_IVARS_ImplClassWithResilientStoredProperty, ptr null, ptr @_PROPERTIES_ImplClassWithResilientStoredProperty, ptr @"$sSo36ImplClassWithResilientStoredPropertyCMU" }, section "__DATA, __objc_data", align 8
-// CHECK: @"OBJC_CLASS_$_ImplClassWithResilientStoredProperty" = global <{ i64, ptr, ptr, ptr, i64 }> <{ i64 ptrtoint (ptr @"OBJC_METACLASS_$_ImplClassWithResilientStoredProperty" to i64), ptr @"OBJC_CLASS_$_NSObject", ptr @_objc_empty_cache, ptr null, i64 add (i64 ptrtoint (ptr @_DATA_ImplClassWithResilientStoredProperty to i64), i64 1) }>, section "__DATA,__objc_data, regular", align 8
+// CHECK: @"OBJC_CLASS_$_ImplClassWithResilientStoredProperty" = global <{ i64, ptr, ptr, ptr, i64 }> <{ i64 ptrtoint (ptr @"OBJC_METACLASS_$_ImplClassWithResilientStoredProperty" to i64), ptr @"OBJC_CLASS_$_NSObject", ptr @_objc_empty_cache, ptr null, i64 add (i64 ptrtoint (ptr @_DATA_ImplClassWithResilientStoredProperty to i64), i64 2) }>, section "__DATA,__objc_data, regular", align 8
 
 @_objcImplementation extension ImplClassWithResilientStoredProperty {
   @objc var beforeInt: Int32 = 0

test/IRGen/objc_implementation_deployment_target_resilience.swift

@@ -0,0 +1,25 @@
+// RUN: %target-swift-frontend(mock-sdk: %clang-importer-sdk) -target %target-stable-abi-triple -I %S/Inputs/abi -F %clang-importer-sdk-path/frameworks %s -import-objc-header %S/Inputs/objc_implementation.h -emit-ir -o %t.ir -enable-library-evolution -verify
+// REQUIRES: objc_interop
+
+@_objcImplementation extension ImplClassWithResilientStoredProperty {
+  @objc var beforeInt: Int32 = 0    // no-error
+  final var a: Mirror?              // expected-error {{does not support stored properties whose size can change due to library evolution; raise the minimum deployment target to}}
+  final var b: AnyKeyPath?          // no-error
+  final var c: Int = 0              // no-error
+  final var d: S1?                  // no-error
+  final var e: S2?                  // expected-error {{does not support stored properties whose size can change due to library evolution; raise the minimum deployment target to}}
+  final var f: C1?                  // no-error
+  @objc var afterInt: Int32 = 0     // no-error
+}
+
+public struct S1 {
+  var int = 1
+}
+
+public struct S2 {
+  var mirror: Mirror?
+}
+
+public class C1 {
+  var mirror: Mirror?
+}