[Serialization] Store whether an override depends on its base for ABI

lib/Serialization/Deserialization.cpp

@@ -2822,7 +2822,7 @@ class swift::DeclDeserializer {
     DeclID associatedDeclID;
     DeclID overriddenID;
     DeclID accessorStorageDeclID;
-    bool needsNewVTableEntry, isTransparent;
+    bool overriddenAffectsABI, needsNewVTableEntry, isTransparent;
     DeclID opaqueReturnTypeID;
     ArrayRef<uint64_t> nameAndDependencyIDs;
 
@@ -2835,6 +2835,7 @@ class swift::DeclDeserializer {
                                           resultInterfaceTypeID,
                                           isIUO,
                                           associatedDeclID, overriddenID,
+                                          overriddenAffectsABI,
                                           numNameComponentsBiased,
                                           rawAccessLevel,
                                           needsNewVTableEntry,
@@ -2849,6 +2850,7 @@ class swift::DeclDeserializer {
                                               resultInterfaceTypeID,
                                               isIUO,
                                               overriddenID,
+                                              overriddenAffectsABI,
                                               accessorStorageDeclID,
                                               rawAccessorKind,
                                               rawAccessLevel,
@@ -2914,20 +2916,11 @@ class swift::DeclDeserializer {
       overridden = overriddenOrError.get();
     } else {
       llvm::consumeError(overriddenOrError.takeError());
-      // There's one case where we know it's safe to ignore a missing override:
-      // if this declaration is '@objc' and 'dynamic'.
-      bool canIgnoreMissingOverriddenDecl = false;
-      if (isObjC && ctx.LangOpts.EnableDeserializationRecovery) {
-        canIgnoreMissingOverriddenDecl =
-            std::any_of(DeclAttributes::iterator(DAttrs),
-                        DeclAttributes::iterator(nullptr),
-                        [](const DeclAttribute *attr) -> bool {
-          return isa<DynamicAttr>(attr);
-        });
-      }
-      if (!canIgnoreMissingOverriddenDecl)
+
+      if (overriddenAffectsABI || !ctx.LangOpts.EnableDeserializationRecovery) {
         return llvm::make_error<OverrideError>(
             name, errorFlags, numVTableEntries);
+      }
 
       overridden = nullptr;
     }

lib/Serialization/ModuleFormat.h

@@ -52,7 +52,7 @@ const uint16_t SWIFTMODULE_VERSION_MAJOR = 0;
 /// describe what change you made. The content of this comment isn't important;
 /// it just ensures a conflict if two people change the module format.
 /// Don't worry about adhering to the 80-column limit for this line.
-const uint16_t SWIFTMODULE_VERSION_MINOR = 519; // SIL function availability
+const uint16_t SWIFTMODULE_VERSION_MINOR = 521; // "overridden affects ABI" flag
 
 /// A standard hash seed used for all string hashes in a serialized module.
 ///
@@ -1200,6 +1200,7 @@ namespace decls_block {
     BCFixed<1>,   // IUO result?
     DeclIDField,  // operator decl
     DeclIDField,  // overridden function
+    BCFixed<1>,   // whether the overridden decl affects ABI
     BCVBR<5>,     // 0 for a simple name, otherwise the number of parameter name
                   // components plus one
     AccessLevelField, // access level
@@ -1241,6 +1242,7 @@ namespace decls_block {
     TypeIDField,  // result interface type
     BCFixed<1>,   // IUO result?
     DeclIDField,  // overridden function
+    BCFixed<1>,   // whether the overridden decl affects ABI
     DeclIDField,  // AccessorStorageDecl
     AccessorKindField, // accessor kind
     AccessLevelField, // access level

lib/Serialization/Serialization.cpp

@@ -2593,6 +2593,24 @@ class Serializer::DeclSerializer : public DeclVisitor<DeclSerializer> {
     return count;
   }
 
+  /// Returns true if a client can still use decls that override \p overridden
+  /// even if \p overridden itself isn't available (isn't found, can't be
+  /// imported, can't be deserialized, whatever).
+  ///
+  /// This should be kept conservative. Compiler crashes are still better than
+  /// miscompiles.
+  static bool overriddenDeclAffectsABI(const ValueDecl *overridden) {
+    if (!overridden)
+      return false;
+    // There's one case where we know a declaration doesn't affect the ABI of
+    // its overrides after they've been compiled: if the declaration is '@objc'
+    // and 'dynamic'. In that case, all accesses to the method or property will
+    // go through the Objective-C method tables anyway.
+    if (overridden->hasClangNode() || overridden->isObjCDynamic())
+      return false;
+    return true;
+  }
+
 public:
   DeclSerializer(Serializer &S, DeclID id) : S(S), id(id) {}
   ~DeclSerializer() {
@@ -3212,6 +3230,7 @@ class Serializer::DeclSerializer : public DeclVisitor<DeclSerializer> {
                            fn->isImplicitlyUnwrappedOptional(),
                            S.addDeclRef(fn->getOperatorDecl()),
                            S.addDeclRef(fn->getOverriddenDecl()),
+                           overriddenDeclAffectsABI(fn->getOverriddenDecl()),
                            fn->getFullName().getArgumentNames().size() +
                              fn->getFullName().isCompoundName(),
                            rawAccessLevel,
@@ -3267,6 +3286,9 @@ class Serializer::DeclSerializer : public DeclVisitor<DeclSerializer> {
     uint8_t rawAccessorKind =
       uint8_t(getStableAccessorKind(fn->getAccessorKind()));
 
+    bool overriddenAffectsABI =
+        overriddenDeclAffectsABI(fn->getOverriddenDecl());
+
     Type ty = fn->getInterfaceType();
     SmallVector<IdentifierID, 4> dependencies;
     for (auto dependency : collectDependenciesFromType(ty->getCanonicalType()))
@@ -3288,6 +3310,7 @@ class Serializer::DeclSerializer : public DeclVisitor<DeclSerializer> {
                                S.addTypeRef(fn->getResultInterfaceType()),
                                fn->isImplicitlyUnwrappedOptional(),
                                S.addDeclRef(fn->getOverriddenDecl()),
+                               overriddenAffectsABI,
                                S.addDeclRef(fn->getStorage()),
                                rawAccessorKind,
                                rawAccessLevel,

test/Serialization/Recovery/Inputs/custom-modules/Overrides.h

@@ -8,11 +8,13 @@
 - (nullable id)nullabilityChangeMethod;
 - (nonnull id)typeChangeMethod;
 @property (readonly) long disappearingProperty;
+@property (readwrite) long disappearingPropertySetter;
 #else
 //- (void)disappearingMethod;
 - (nonnull id)nullabilityChangeMethod;
 - (nonnull Base *)typeChangeMethod;
 // @property (readonly) long disappearingProperty;
+@property (readonly) long disappearingPropertySetter;
 #endif
 @end
 

test/Serialization/Recovery/overrides.swift

@@ -72,19 +72,36 @@ public class A_Sub: Base {
   public override func typeChangeMethod() -> Any { return self }
   public override func disappearingMethodWithOverload() {}
   public override var disappearingProperty: Int { return 0 }
+  public override var disappearingPropertySetter: Int {
+    get { return 0 }
+    set {}
+  }
 }
 
 public class A_Sub2: A_Sub {
   public override func disappearingMethod() {}
 }
 
+public final class A_Sub3Final: Base {
+  public override func disappearingMethod() {}
+  public override func nullabilityChangeMethod() -> Any? { return nil }
+  public override func typeChangeMethod() -> Any { return self }
+  public override func disappearingMethodWithOverload() {}
+  public override var disappearingProperty: Int { return 0 }
+  public override var disappearingPropertySetter: Int {
+    get { return 0 }
+    set {}
+  }
+}
+
 
 // CHECK-LABEL: class A_Sub : Base {
 // CHECK-NEXT: func disappearingMethod()
 // CHECK-NEXT: func nullabilityChangeMethod() -> Any?
 // CHECK-NEXT: func typeChangeMethod() -> Any
 // CHECK-NEXT: func disappearingMethodWithOverload()
 // CHECK-NEXT: var disappearingProperty: Int { get }
+// CHECK-NEXT: var disappearingPropertySetter: Int{{$}}
 // CHECK-NEXT: init()
 // CHECK-NEXT: {{^}$}}
 
@@ -93,12 +110,23 @@ public class A_Sub2: A_Sub {
 // CHECK-NEXT: init()
 // CHECK-NEXT: {{^}$}}
 
+// CHECK-LABEL: final class A_Sub3Final : Base {
+// CHECK-NEXT: func disappearingMethod()
+// CHECK-NEXT: func nullabilityChangeMethod() -> Any?
+// CHECK-NEXT: func typeChangeMethod() -> Any
+// CHECK-NEXT: func disappearingMethodWithOverload()
+// CHECK-NEXT: var disappearingProperty: Int { get }
+// CHECK-NEXT: var disappearingPropertySetter: Int{{$}}
+// CHECK-NEXT: init()
+// CHECK-NEXT: {{^}$}}
+
 // CHECK-RECOVERY-LABEL: class A_Sub : Base {
 // CHECK-RECOVERY-NEXT: func disappearingMethod()
 // CHECK-RECOVERY-NEXT: func nullabilityChangeMethod() -> Any?
 // CHECK-RECOVERY-NEXT: func typeChangeMethod() -> Any
 // CHECK-RECOVERY-NEXT: func disappearingMethodWithOverload()
 // CHECK-RECOVERY-NEXT: /* placeholder for disappearingProperty */
+// CHECK-RECOVERY-NEXT: var disappearingPropertySetter: Int{{$}}
 // CHECK-RECOVERY-NEXT: init()
 // CHECK-RECOVERY-NEXT: {{^}$}}
 
@@ -107,6 +135,16 @@ public class A_Sub2: A_Sub {
 // CHECK-RECOVERY-NEXT: init()
 // CHECK-RECOVERY-NEXT: {{^}$}}
 
+// CHECK-RECOVERY-LABEL: class A_Sub3Final : Base {
+// CHECK-RECOVERY-NEXT: func disappearingMethod()
+// CHECK-RECOVERY-NEXT: func nullabilityChangeMethod() -> Any?
+// CHECK-RECOVERY-NEXT: func typeChangeMethod() -> Any
+// CHECK-RECOVERY-NEXT: func disappearingMethodWithOverload()
+// CHECK-RECOVERY-NEXT: /* placeholder for disappearingProperty */
+// CHECK-RECOVERY-NEXT: var disappearingPropertySetter: Int{{$}}
+// CHECK-RECOVERY-NEXT: init()
+// CHECK-RECOVERY-NEXT: {{^}$}}
+
 extension Base {
   @nonobjc func disappearingMethodWithOverload() -> SwiftOnlyClass? { return nil }
 }