[Serialization] Drop overriding initializers with missing bases.

jrose-apple · jrose-apple · commit abdaaefe29ab · 2017-04-24T11:53:54.000-07:00
This is the same as the last few commits, but with the additional
complication of designated initializers affecting other behavior
around the type. In particular, convenience initializers cannot be
invoked on subclasses if the designated initializers are not all
present on the subclass. If a designated initializer is dropped, it's
not possible to satisfy that.

It would be nice to do better here, since a class's initializers are
mostly independent of the superclass's initializers. Unfortunately, it
still affects whether /this/ class can inherit convenience
initializers, as well as vtable layout. This is conservative, at
least.
diff --git a/lib/Serialization/Deserialization.cpp b/lib/Serialization/Deserialization.cpp
@@ -299,17 +299,32 @@ namespace {
   const char XRefError::ID = '\0';
 
   class OverrideError : public llvm::ErrorInfo<OverrideError> {
+  public:
+    enum Kind {
+      Normal,
+      DesignatedInitializer
+    };
+
+  private:
     friend ErrorInfo;
     static const char ID;
 
     DeclName name;
+    Kind kind;
+
   public:
-    explicit OverrideError(DeclName name) : name(name) {}
+
+    explicit OverrideError(DeclName name, Kind kind = Normal)
+        : name(name), kind(kind) {}
 
     void log(raw_ostream &OS) const override {
       OS << "could not find '" << name << "' in parent class";
     }
 
+    Kind getKind() const {
+      return kind;
+    }
+
     std::error_code convertToErrorCode() const override {
       // This is a deprecated part of llvm::Error, so we just return a very
       // generic value.
@@ -2694,6 +2709,29 @@ ModuleFile::getDeclChecked(DeclID DID, Optional<DeclContext *> ForcedContext) {
                                                genericEnvID, interfaceID,
                                                overriddenID, rawAccessLevel,
                                                argNameIDs);
+
+    // Resolve the name ids.
+    SmallVector<Identifier, 2> argNames;
+    for (auto argNameID : argNameIDs)
+      argNames.push_back(getIdentifier(argNameID));
+    DeclName name(ctx, ctx.Id_init, argNames);
+
+    Optional<swift::CtorInitializerKind> initKind =
+        getActualCtorInitializerKind(storedInitKind);
+
+    auto overridden = getDeclChecked(overriddenID);
+    if (!overridden) {
+      llvm::handleAllErrors(overridden.takeError(),
+          [](const XRefError &) { /* expected */ },
+          [this](std::unique_ptr<llvm::ErrorInfoBase> unhandled) {
+        fatal(std::move(unhandled));
+      });
+      auto kind = OverrideError::Normal;
+      if (initKind == CtorInitializerKind::Designated)
+        kind = OverrideError::DesignatedInitializer;
+      return llvm::make_error<OverrideError>(name, kind);
+    }
+
     auto parent = getDeclContext(contextID);
     if (declOrOffset.isComplete())
       return declOrOffset;
@@ -2702,16 +2740,10 @@ ModuleFile::getDeclChecked(DeclID DID, Optional<DeclContext *> ForcedContext) {
     if (declOrOffset.isComplete())
       return declOrOffset;
 
-    // Resolve the name ids.
-    SmallVector<Identifier, 2> argNames;
-    for (auto argNameID : argNameIDs)
-      argNames.push_back(getIdentifier(argNameID));
-
     OptionalTypeKind failability = OTK_None;
     if (auto actualFailability = getActualOptionalTypeKind(rawFailability))
       failability = *actualFailability;
 
-    DeclName name(ctx, ctx.Id_init, argNames);
     auto ctor =
       createDecl<ConstructorDecl>(name, SourceLoc(),
                                   failability, /*FailabilityLoc=*/SourceLoc(),
@@ -2761,11 +2793,10 @@ ModuleFile::getDeclChecked(DeclID DID, Optional<DeclContext *> ForcedContext) {
       ctor->setImplicit();
     if (hasStubImplementation)
       ctor->setStubImplementation(true);
-    if (auto initKind = getActualCtorInitializerKind(storedInitKind))
-      ctor->setInitKind(*initKind);
-    if (auto overridden
-          = dyn_cast_or_null<ConstructorDecl>(getDecl(overriddenID)))
-      ctor->setOverriddenDecl(overridden);
+    if (initKind.hasValue())
+      ctor->setInitKind(initKind.getValue());
+    if (auto overriddenCtor = cast_or_null<ConstructorDecl>(overridden.get()))
+      ctor->setOverriddenDecl(overriddenCtor);
     break;
   }
 
@@ -4429,9 +4460,17 @@ void ModuleFile::loadAllMembers(Decl *container, uint64_t contextData) {
       if (!getContext().LangOpts.EnableDeserializationRecovery)
         fatal(next.takeError());
 
-      // Silently drop the member if it had an override-related problem.
+      // Drop the member if it had an override-related problem.
+      auto handleMissingOverrideBase = [container](const OverrideError &error) {
+        if (error.getKind() != OverrideError::DesignatedInitializer)
+          return;
+        auto *containingClass = dyn_cast<ClassDecl>(container);
+        if (!containingClass)
+          return;
+        containingClass->setHasMissingDesignatedInitializers();
+      };
       llvm::handleAllErrors(next.takeError(),
-          [](const OverrideError &) { /* expected */ },
+          handleMissingOverrideBase,
           [this](std::unique_ptr<llvm::ErrorInfoBase> unhandled) {
         fatal(std::move(unhandled));
       });
diff --git a/test/Serialization/Recovery/Inputs/custom-modules/Overrides.h b/test/Serialization/Recovery/Inputs/custom-modules/Overrides.h
@@ -68,3 +68,50 @@
 // - (nonnull Element)objectAtIndexedSubscript:(nonnull id)key;
 #endif
 @end
+
+
+@interface DesignatedInitDisappearsBase : Object
+- (nonnull instancetype)init __attribute__((objc_designated_initializer));
+- (nonnull instancetype)initConvenience:(long)value;
+#if !BAD
+- (nonnull instancetype)initWithValue:(long)value __attribute__((objc_designated_initializer));
+#else
+//- (nonnull instancetype)initWithValue:(long)value __attribute__((objc_designated_initializer));
+#endif
+@end
+
+@interface OnlyDesignatedInitDisappearsBase : Object
+- (nonnull instancetype)initConvenience:(long)value;
+#if !BAD
+- (nonnull instancetype)initWithValue:(long)value __attribute__((objc_designated_initializer));
+#else
+//- (nonnull instancetype)initWithValue:(long)value __attribute__((objc_designated_initializer));
+#endif
+@end
+
+@interface ConvenienceInitDisappearsBase : Object
+- (nonnull instancetype)init __attribute__((objc_designated_initializer));
+- (nonnull instancetype)initConvenience:(long)value;
+#if !BAD
+- (nonnull instancetype)initWithValue:(long)value;
+#else
+//- (nonnull instancetype)initWithValue:(long)value;
+#endif
+@end
+
+@interface UnknownInitDisappearsBase : Object
+- (nonnull instancetype)init;
+#if !BAD
+- (nonnull instancetype)initWithValue:(long)value;
+#else
+//- (nonnull instancetype)initWithValue:(long)value;
+#endif
+@end
+
+@interface OnlyUnknownInitDisappearsBase : Object
+#if !BAD
+- (nonnull instancetype)initWithValue:(long)value;
+#else
+//- (nonnull instancetype)initWithValue:(long)value;
+#endif
+@end
diff --git a/test/Serialization/Recovery/overrides.swift b/test/Serialization/Recovery/overrides.swift
@@ -5,8 +5,60 @@
 
 // RUN: %target-swift-ide-test -source-filename=x -print-module -module-to-print Lib -I %t -I %S/Inputs/custom-modules -Xcc -DBAD -enable-experimental-deserialization-recovery | %FileCheck -check-prefix CHECK-RECOVERY %s
 
+// RUN: %target-swift-frontend -typecheck %s -I %t -I %S/Inputs/custom-modules -Xcc -DBAD -enable-experimental-deserialization-recovery -D TEST -verify
+
 // REQUIRES: objc_interop
 
+#if TEST
+import Lib
+
+func testInitializers() {
+  _ = D1_DesignatedInitDisappears()
+  _ = D1_DesignatedInitDisappears(value: 0) // expected-error {{incorrect argument label in call}}
+  _ = D1_DesignatedInitDisappears(convenience: 0)
+
+  _ = D2_OnlyDesignatedInitDisappears(value: 0) // expected-error {{cannot be constructed because it has no accessible initializers}}
+  _ = D2_OnlyDesignatedInitDisappears(convenience: 0) // expected-error {{cannot be constructed because it has no accessible initializers}}
+
+  _ = D3_ConvenienceInitDisappears()
+  _ = D3_ConvenienceInitDisappears(value: 0) // expected-error {{incorrect argument label in call}}
+  _ = D3_ConvenienceInitDisappears(convenience: 0)
+
+  _ = D4_UnknownInitDisappears()
+  _ = D4_UnknownInitDisappears(value: 0) // expected-error {{argument passed to call that takes no arguments}}
+
+  // FIXME: Why does 'init()' show up in the generated interface if it can't be
+  // called?
+  _ = D5_OnlyUnknownInitDisappears() // expected-error {{cannot be constructed because it has no accessible initializers}}
+  _ = D5_OnlyUnknownInitDisappears(value: 0) // expected-error {{cannot be constructed because it has no accessible initializers}}
+}
+
+func testSubclassInitializers() {
+  class DesignatedInitDisappearsSub : D1_DesignatedInitDisappears {}
+  _ = DesignatedInitDisappearsSub()
+  _ = DesignatedInitDisappearsSub(value: 0) // expected-error {{argument passed to call that takes no arguments}}
+  _ = DesignatedInitDisappearsSub(convenience: 0) // expected-error {{argument passed to call that takes no arguments}}
+
+  class OnlyDesignatedInitDisappearsSub : D2_OnlyDesignatedInitDisappears {}
+  _ = OnlyDesignatedInitDisappearsSub(value: 0) // expected-error {{cannot be constructed because it has no accessible initializers}}
+  _ = OnlyDesignatedInitDisappearsSub(convenience: 0) // expected-error {{cannot be constructed because it has no accessible initializers}}
+
+  class ConvenienceInitDisappearsSub : D3_ConvenienceInitDisappears {}
+  _ = ConvenienceInitDisappearsSub()
+  _ = ConvenienceInitDisappearsSub(value: 0) // expected-error {{incorrect argument label in call}}
+  _ = ConvenienceInitDisappearsSub(convenience: 0) // still inheritable
+
+  class UnknownInitDisappearsSub : D4_UnknownInitDisappears {}
+  _ = UnknownInitDisappearsSub()
+  _ = UnknownInitDisappearsSub(value: 0) // expected-error {{argument passed to call that takes no arguments}}
+
+  class OnlyUnknownInitDisappearsSub : D5_OnlyUnknownInitDisappears {}
+  _ = OnlyUnknownInitDisappearsSub() // expected-error {{cannot be constructed because it has no accessible initializers}}
+  _ = OnlyUnknownInitDisappearsSub(value: 0) // expected-error {{cannot be constructed because it has no accessible initializers}}
+}
+
+#else // TEST
+
 import Overrides
 
 // Please use prefixes to keep the printed parts of this file in alphabetical
@@ -111,3 +163,74 @@ public class C4_GenericKeyedSubscriptDisappears : GenericKeyedSubscriptDisappear
 // CHECK-RECOVERY-LABEL: class C4_GenericKeyedSubscriptDisappears : GenericKeyedSubscriptDisappearsBase<Base> {
 // CHECK-RECOVERY-NEXT: init()
 // CHECK-RECOVERY-NEXT: {{^}$}}
+
+
+open class D1_DesignatedInitDisappears : DesignatedInitDisappearsBase {
+  public override init() { fatalError() }
+  public override init(value: Int) { fatalError() }
+}
+
+// CHECK-LABEL: class D1_DesignatedInitDisappears : DesignatedInitDisappearsBase {
+// CHECK-NEXT: init()
+// CHECK-NEXT: init(value: Int)
+// CHECK-NEXT: {{^}$}}
+
+// CHECK-RECOVERY-LABEL: class D1_DesignatedInitDisappears : DesignatedInitDisappearsBase {
+// CHECK-RECOVERY-NEXT: init()
+// CHECK-RECOVERY-NEXT: {{^}$}}
+
+
+open class D2_OnlyDesignatedInitDisappears : OnlyDesignatedInitDisappearsBase {
+  public override init(value: Int) { fatalError() }
+}
+
+// CHECK-LABEL: class D2_OnlyDesignatedInitDisappears : OnlyDesignatedInitDisappearsBase {
+// CHECK-NEXT: init(value: Int)
+// CHECK-NEXT: {{^}$}}
+
+// CHECK-RECOVERY-LABEL: class D2_OnlyDesignatedInitDisappears : OnlyDesignatedInitDisappearsBase {
+// CHECK-RECOVERY-NEXT: {{^}$}}
+
+
+open class D3_ConvenienceInitDisappears : ConvenienceInitDisappearsBase {
+  public override init() { fatalError() }
+}
+
+// CHECK-LABEL: class D3_ConvenienceInitDisappears : ConvenienceInitDisappearsBase {
+// CHECK-NEXT: init()
+// CHECK-NEXT: {{^}$}}
+
+// CHECK-RECOVERY-LABEL: class D3_ConvenienceInitDisappears : ConvenienceInitDisappearsBase {
+// CHECK-RECOVERY-NEXT: init()
+// CHECK-RECOVERY-NEXT: {{^}$}}
+
+
+open class D4_UnknownInitDisappears : UnknownInitDisappearsBase {
+  public override init() { fatalError() }
+  public override init(value: Int) { fatalError() }
+}
+
+// CHECK-LABEL: class D4_UnknownInitDisappears : UnknownInitDisappearsBase {
+// CHECK-NEXT: init()
+// CHECK-NEXT: init(value: Int)
+// CHECK-NEXT: {{^}$}}
+
+// CHECK-RECOVERY-LABEL: class D4_UnknownInitDisappears : UnknownInitDisappearsBase {
+// CHECK-RECOVERY-NEXT: init()
+// CHECK-RECOVERY-NEXT: {{^}$}}
+
+
+open class D5_OnlyUnknownInitDisappears : OnlyUnknownInitDisappearsBase {
+  public override init(value: Int) { fatalError() }
+}
+
+// CHECK-LABEL: class D5_OnlyUnknownInitDisappears : OnlyUnknownInitDisappearsBase {
+// CHECK-NEXT: init(value: Int)
+// CHECK-NEXT: init()
+// CHECK-NEXT: {{^}$}}
+
+// CHECK-RECOVERY-LABEL: class D5_OnlyUnknownInitDisappears : OnlyUnknownInitDisappearsBase {
+// CHECK-RECOVERY-NEXT: init()
+// CHECK-RECOVERY-NEXT: {{^}$}}
+
+#endif // TEST
