Merge pull request #25538 from jrose-apple/override-the-train-electric-boogaloo

jrose-apple · web-flow · commit c58481fc1cd5 · 2019-06-18T17:37:09.000-07:00
[Serialization] If an override is dynamic, missing the base decl is OK
diff --git a/lib/Serialization/Deserialization.cpp b/lib/Serialization/Deserialization.cpp
@@ -1628,7 +1628,12 @@ ModuleFile::resolveCrossReference(ModuleID MID, uint32_t pathLen) {
             return nullptr;
           }
           values.front() = storage->getAccessor(*actualKind);
-          assert(values.front() && "missing accessor");
+          if (!values.front()) {
+            return llvm::make_error<XRefError>("missing accessor",
+                                               pathTrace,
+                                               getXRefDeclNameForError());
+
+          }
         }
         break;
       }
@@ -2996,10 +3001,27 @@ class swift::DeclDeserializer {
       }
     }
 
-    Expected<Decl *> overridden = MF.getDeclChecked(overriddenID);
-    if (!overridden) {
-      llvm::consumeError(overridden.takeError());
-      return llvm::make_error<OverrideError>(name, errorFlags);
+    Expected<Decl *> overriddenOrError = MF.getDeclChecked(overriddenID);
+    Decl *overridden;
+    if (overriddenOrError) {
+      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)
+        return llvm::make_error<OverrideError>(name, errorFlags);
+
+      overridden = nullptr;
     }
 
     for (TypeID dependencyID : dependencyIDs) {
@@ -3090,7 +3112,7 @@ class swift::DeclDeserializer {
     if (auto bodyText = MF.maybeReadInlinableBodyText())
       fn->setBodyStringRepresentation(*bodyText);
 
-    fn->setOverriddenDecl(cast_or_null<FuncDecl>(overridden.get()));
+    fn->setOverriddenDecl(cast_or_null<FuncDecl>(overridden));
     if (fn->getOverriddenDecl())
       AddAttribute(new (ctx) OverrideAttr(SourceLoc()));
 
diff --git a/test/Serialization/Recovery/Inputs/implementation-only-override/FooKit.h b/test/Serialization/Recovery/Inputs/implementation-only-override/FooKit.h
@@ -0,0 +1,18 @@
+@interface Base
+@end
+
+@interface Parent : Base
+- (nonnull instancetype)init __attribute__((objc_designated_initializer));
+
+// The SECRET is on a non-secret property here because we need to enforce that
+// it's not printed.
+@property (readonly, strong, nullable) Parent *redefinedPropSECRET;
+@end
+
+@interface GenericParent<T: Base *> : Base
+- (nonnull instancetype)init __attribute__((objc_designated_initializer));
+@end
+
+@interface SubscriptParent : Base
+- (nullable Parent *)objectAtIndexedSubscript:(int)index;
+@end
diff --git a/test/Serialization/Recovery/Inputs/implementation-only-override/FooKit_SECRET.h b/test/Serialization/Recovery/Inputs/implementation-only-override/FooKit_SECRET.h
@@ -0,0 +1,31 @@
+#import <FooKit.h>
+
+@interface Parent ()
+- (nonnull instancetype)initWithSECRET:(int)secret __attribute__((objc_designated_initializer, swift_name("init(SECRET:)")));
+
+- (void)methodSECRET;
+
+@property (readonly, strong, nullable) Parent *roPropSECRET;
+@property (readwrite, strong, nullable) Parent *rwPropSECRET;
+
+- (nullable Parent *)objectAtIndexedSubscript:(int)index;
+
+@property (readwrite, strong, nullable) Parent *redefinedPropSECRET;
+@end
+
+@protocol MandatorySecrets
+- (nonnull instancetype)initWithRequiredSECRET:(int)secret;
+@end
+
+@interface Parent () <MandatorySecrets>
+- (nonnull instancetype)initWithRequiredSECRET:(int)secret __attribute__((objc_designated_initializer));
+@end
+
+@interface GenericParent<T: Base *> ()
+@property (readonly, strong, nullable) T roPropSECRET;
+- (nullable Parent *)objectAtIndexedSubscript:(int)index;
+@end
+
+@interface SubscriptParent ()
+- (void)setObject:(nullable Parent *)object atIndexedSubscript:(int)index;
+@end
diff --git a/test/Serialization/Recovery/Inputs/implementation-only-override/module.modulemap b/test/Serialization/Recovery/Inputs/implementation-only-override/module.modulemap
@@ -0,0 +1,9 @@
+module FooKit {
+  header "FooKit.h"
+  export *
+}
+
+module FooKit_SECRET {
+  header "FooKit_SECRET.h"
+  export *
+}
diff --git a/test/Serialization/Recovery/implementation-only-override.swift b/test/Serialization/Recovery/implementation-only-override.swift
@@ -0,0 +1,112 @@
+// RUN: %empty-directory(%t)
+// RUN: %target-swift-frontend -emit-module-path %t/Library.swiftmodule -I %S/Inputs/implementation-only-override -enable-library-evolution -enable-objc-interop -module-name Library %s
+// RUN: %target-swift-ide-test -print-module -module-to-print=Library -I %t -I %S/Inputs/implementation-only-override -source-filename=x -access-filter-public -enable-objc-interop > %t/Library.txt
+// RUN: %FileCheck %s < %t/Library.txt
+
+// CHECK: import FooKit
+// CHECK: import FooKit_SECRET
+
+import FooKit
+@_implementationOnly import FooKit_SECRET
+
+// CHECK-LABEL: class GoodChild : Parent {
+//  CHECK-NEXT:   override init()
+//  CHECK-NEXT:   /* placeholder for init(SECRET:) */
+//  CHECK-NEXT:   /* placeholder for init(requiredSECRET:) */
+//  CHECK-NEXT:   @_implementationOnly func methodSECRET()
+//  CHECK-NEXT:   @_implementationOnly override var redefinedPropSECRET: Parent?
+//  CHECK-NEXT: }
+public class GoodChild: Parent {
+  public override init() { super.init() }
+  // FIXME: @_implementationOnly on an initializer doesn't exactly make sense,
+  // since they're not inherited.
+  @_implementationOnly public override init(SECRET: Int32) { super.init() }
+  @_implementationOnly public required init(requiredSECRET: Int32) { super.init() }
+
+  @_implementationOnly public override func methodSECRET() {}
+  @_implementationOnly public override var roPropSECRET: Parent? { nil }
+  @_implementationOnly public override var rwPropSECRET: Parent? {
+    get { nil }
+    set {}
+  }
+  @_implementationOnly public override subscript(_ index: Int32) -> Parent? { nil }
+  @_implementationOnly public override var redefinedPropSECRET: Parent? {
+    get { nil }
+    set {}
+  }
+}
+
+// CHECK-LABEL: class GoodGenericChild<Toy> : Parent {
+//  CHECK-NEXT:   override init()
+//  CHECK-NEXT:   /* placeholder for init(SECRET:) */
+//  CHECK-NEXT:   /* placeholder for init(requiredSECRET:) */
+//  CHECK-NEXT:   @_implementationOnly func methodSECRET()
+//  CHECK-NEXT:   @_implementationOnly override var redefinedPropSECRET: Parent?
+//  CHECK-NEXT: }
+public class GoodGenericChild<Toy>: Parent {
+  public override init() { super.init() }
+  // FIXME: @_implementationOnly on an initializer doesn't exactly make sense,
+  // since they're not inherited.
+  @_implementationOnly public override init(SECRET: Int32) { super.init() }
+  @_implementationOnly public required init(requiredSECRET: Int32) { super.init() }
+
+  @_implementationOnly public override func methodSECRET() {}
+  @_implementationOnly public override var roPropSECRET: Parent? { nil }
+  @_implementationOnly public override var rwPropSECRET: Parent? {
+    get { nil }
+    set {}
+  }
+  @_implementationOnly public override subscript(_ index: Int32) -> Parent? { nil }
+  @_implementationOnly public override var redefinedPropSECRET: Parent? {
+    get { nil }
+    set {}
+  }
+}
+
+// CHECK-LABEL: class QuietChild : Parent {
+//  CHECK-NEXT:   /* placeholder for init(SECRET:) */
+//  CHECK-NEXT:   /* placeholder for init(requiredSECRET:) */
+//  CHECK-NEXT: }
+public class QuietChild: Parent {
+  internal override init() { super.init() }
+  internal override init(SECRET: Int32) { super.init() }
+  internal required init(requiredSECRET: Int32) { super.init() }
+}
+
+internal class PrivateChild: Parent {
+  override func methodSECRET() {}
+  override var roPropSECRET: Parent? { nil }
+  override var rwPropSECRET: Parent? {
+    get { nil }
+    set {}
+  }
+  override subscript(_ index: Int32) -> Parent? { nil }
+  override var redefinedPropSECRET: Parent? {
+    get { nil }
+    set {}
+  }
+}
+
+internal class PrivateGrandchild: GoodChild {
+  override func methodSECRET() {}
+  override var roPropSECRET: Parent? { nil }
+  override var rwPropSECRET: Parent? {
+    get { nil }
+    set {}
+  }
+  override subscript(_ index: Int32) -> Parent? { nil }
+  override var redefinedPropSECRET: Parent? {
+    get { nil }
+    set {}
+  }
+}
+
+// CHECK-LABEL: class SubscriptChild : SubscriptParent {
+//  CHECK-NEXT:   @_implementationOnly override subscript(index: Int32) -> Parent?
+//  CHECK-NEXT: }
+public class SubscriptChild: SubscriptParent {
+  @_implementationOnly public override subscript(_ index: Int32) -> Parent? {
+    get { nil }
+    set {}
+  }
+}
diff --git a/test/Serialization/Recovery/overrides.swift b/test/Serialization/Recovery/overrides.swift
@@ -94,10 +94,15 @@ public class A_Sub2: A_Sub {
 // 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: init()
 // CHECK-RECOVERY-NEXT: {{^}$}}
 
 // CHECK-RECOVERY-LABEL: class A_Sub2 : A_Sub {
+// CHECK-RECOVERY-NEXT: func disappearingMethod()
 // CHECK-RECOVERY-NEXT: init()
 // CHECK-RECOVERY-NEXT: {{^}$}}
 
@@ -119,6 +124,9 @@ public class B_GenericSub : GenericBase<Base> {
 // CHECK-NEXT: {{^}$}}
 
 // CHECK-RECOVERY-LABEL: class B_GenericSub : GenericBase<Base> {
+// CHECK-RECOVERY-NEXT: func disappearingMethod()
+// CHECK-RECOVERY-NEXT: func nullabilityChangeMethod() -> Base?
+// CHECK-RECOVERY-NEXT: func typeChangeMethod() -> Any
 // CHECK-RECOVERY-NEXT: init()
 // CHECK-RECOVERY-NEXT: {{^}$}}
 
