[Serialization] Drop overridden relationship in constructors when safe

Deserialization recovery lead the compiler to drop public constructors
overridding internal constructors. This limits the logic to dropping the
overriding relationship instead of the whole constructor. This applies
when the overriden constructor fails to deserialize and only when the
overriding relationship was marked as not affecting ABI.

rdar://104704832

Author: xymus

lib/Serialization/Deserialization.cpp

@@ -3094,7 +3094,7 @@ class DeclDeserializer {
     GenericSignatureID genericSigID;
     uint8_t storedInitKind, rawAccessLevel;
     DeclID overriddenID;
-    bool needsNewVTableEntry, firstTimeRequired;
+    bool overriddenAffectsABI, needsNewVTableEntry, firstTimeRequired;
     unsigned numArgNames;
     ArrayRef<uint64_t> argNameAndDependencyIDs;
 
@@ -3104,6 +3104,7 @@ class DeclDeserializer {
                                                async, throws, storedInitKind,
                                                genericSigID,
                                                overriddenID,
+                                               overriddenAffectsABI,
                                                rawAccessLevel,
                                                needsNewVTableEntry,
                                                firstTimeRequired,
@@ -3130,15 +3131,22 @@ class DeclDeserializer {
       attrs.setRawAttributeChain(DAttrs);
     }
 
-    auto overridden = MF.getDeclChecked(overriddenID);
-    if (!overridden) {
+    Expected<Decl *> overriddenOrError = MF.getDeclChecked(overriddenID);
+    Decl *overridden;
+    if (overriddenOrError) {
+      overridden = overriddenOrError.get();
+    } else {
       // Pass through deserialization errors.
-      if (overridden.errorIsA<FatalDeserializationError>())
-        return overridden.takeError();
+      if (overriddenOrError.errorIsA<FatalDeserializationError>())
+        return overriddenOrError.takeError();
 
-      llvm::consumeError(overridden.takeError());
-      return llvm::make_error<OverrideError>(
-          name, errorFlags, numVTableEntries);
+      llvm::consumeError(overriddenOrError.takeError());
+      if (overriddenAffectsABI || !ctx.LangOpts.EnableDeserializationRecovery) {
+        return llvm::make_error<OverrideError>(name, errorFlags,
+                                               numVTableEntries);
+      }
+
+      overridden = nullptr;
     }
 
     for (auto dependencyID : argNameAndDependencyIDs.slice(numArgNames)) {
@@ -3198,7 +3206,7 @@ class DeclDeserializer {
     ctx.evaluator.cacheOutput(NeedsNewVTableEntryRequest{ctor},
                               std::move(needsNewVTableEntry));
 
-    ctor->setOverriddenDecl(cast_or_null<ConstructorDecl>(overridden.get()));
+    ctor->setOverriddenDecl(cast_or_null<ConstructorDecl>(overridden));
     if (auto *overridden = ctor->getOverriddenDecl()) {
       if (!attributeChainContains<RequiredAttr>(DAttrs) ||
           !overridden->isRequired()) {

lib/Serialization/ModuleFormat.h

@@ -58,7 +58,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 = 741; // SIL pack types
+const uint16_t SWIFTMODULE_VERSION_MINOR = 742; // Constructor affects ABI
 
 /// A standard hash seed used for all string hashes in a serialized module.
 ///
@@ -1455,6 +1455,7 @@ namespace decls_block {
     CtorInitializerKindField,  // initializer kind
     GenericSignatureIDField, // generic environment
     DeclIDField, // overridden decl
+    BCFixed<1>,   // whether the overridden decl affects ABI
     AccessLevelField, // access level
     BCFixed<1>,   // requires a new vtable slot
     BCFixed<1>,   // 'required' but overridden is not (used for recovery)

lib/Serialization/Serialization.cpp

@@ -3535,7 +3535,8 @@ class Serializer::DeclSerializer : public DeclVisitor<DeclSerializer> {
     // 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->shouldUseObjCDispatch())
+    if (!isa<ConstructorDecl>(override) &&
+        (overridden->hasClangNode() || overridden->shouldUseObjCDispatch()))
       return false;
 
     // In a public-override-internal case, the override doesn't have ABI
@@ -4499,9 +4500,12 @@ class Serializer::DeclSerializer : public DeclVisitor<DeclSerializer> {
     uint8_t rawAccessLevel = getRawStableAccessLevel(ctor->getFormalAccess());
 
     bool firstTimeRequired = ctor->isRequired();
-    if (auto *overridden = ctor->getOverriddenDecl())
+    auto *overridden = ctor->getOverriddenDecl();
+    if (overridden) {
       if (firstTimeRequired && overridden->isRequired())
         firstTimeRequired = false;
+    }
+    bool overriddenAffectsABI = overriddenDeclAffectsABI(ctor, overridden);
 
     unsigned abbrCode = S.DeclTypeAbbrCodes[ConstructorLayout::Code];
     ConstructorLayout::emitRecord(S.Out, S.ScratchRecord, abbrCode,
@@ -4517,7 +4521,8 @@ class Serializer::DeclSerializer : public DeclVisitor<DeclSerializer> {
                                     ctor->getInitKind()),
                                   S.addGenericSignatureRef(
                                                  ctor->getGenericSignature()),
-                                  S.addDeclRef(ctor->getOverriddenDecl()),
+                                  S.addDeclRef(overridden),
+                                  overriddenAffectsABI,
                                   rawAccessLevel,
                                   ctor->needsNewVTableEntry(),
                                   firstTimeRequired,

test/Serialization/Safety/override-internal-func.swift

@@ -19,6 +19,8 @@
 //--- Lib.swift
 
 open class Base {
+  internal init() {}
+
   public func publicMethod() -> Int {
     return 1
   }
@@ -33,6 +35,10 @@ open class Base {
 }
 
 open class Derived : Base {
+  public override init() {
+      super.init()
+  }
+
   open override func publicMethod() -> Int {
     return super.publicMethod() + 1
   }
@@ -63,3 +69,7 @@ public class OtherFinalDerived : Derived {
     return super.internalMethod() + 1
   }
 }
+
+func foo() {
+    let a = Derived()
+}