Merge pull request #25678 from slavapestov/vtable-thunk-final-bug

SILGen: Fix generated vtable thunk when a final override is more visible than the base

Author: slavapestov

lib/SILGen/SILGenType.cpp

@@ -96,8 +96,9 @@ SILGenModule::emitVTableMethod(ClassDecl *theClass,
   // If the base method is less visible than the derived method, we need
   // a thunk.
   bool baseLessVisibleThanDerived =
-    (derivedDecl->isEffectiveLinkageMoreVisibleThan(baseDecl) &&
-     !usesObjCDynamicDispatch);
+    (!usesObjCDynamicDispatch &&
+     !derivedDecl->isFinal() &&
+     derivedDecl->isEffectiveLinkageMoreVisibleThan(baseDecl));
 
   // Determine the derived thunk type by lowering the derived type against the
   // abstraction pattern of the base.

test/Interpreter/Inputs/vtables_multifile_2.swift

@@ -13,6 +13,12 @@ open class Derived : Base {
   }
 }
 
+public final class FinalDerived : Base {
+  public override func privateMethod() -> Int {
+    return super.privateMethod() + 1
+  }
+}
+
 public func callBaseMethod(_ b: Base) -> Int {
   return b.privateMethod()
 }

test/Interpreter/vtables_multifile.swift

@@ -21,6 +21,12 @@ open class OtherDerived : Derived {
   }
 }
 
+public final class OtherFinalDerived : Derived {
+  public override func privateMethod() -> Int {
+    return super.privateMethod() + 1
+  }
+}
+
 VTableTestSuite.test("Base") {
   expectEqual(1, callBaseMethod(Base()))
 }
@@ -35,4 +41,9 @@ VTableTestSuite.test("OtherDerived") {
   expectEqual(3, callDerivedMethod(OtherDerived()))
 }
 
+VTableTestSuite.test("OtherFinalDerived") {
+  expectEqual(3, callBaseMethod(OtherFinalDerived()))
+  expectEqual(3, callDerivedMethod(OtherFinalDerived()))
+}
+
 runAllTests()

test/SILGen/vtables_multifile.swift

@@ -34,6 +34,13 @@ open class MostDerived : MoreDerived {
   open override func privateMethod4(_: Int) {}
 }
 
+public final class FinalDerived : Base<Int> {
+  internal override func privateMethod1() {}
+  internal override func privateMethod2(_: AnyObject?) {}
+  internal override func privateMethod3(_: Int?) {}
+  internal override func privateMethod4(_: Int) {}
+}
+
 // See Inputs/vtables_multifile_2.swift for overrides in a different file.
 // See Inputs/vtables_multifile_3.swift for overrides in a different module.
 
@@ -127,7 +134,6 @@ open class MostDerived : MoreDerived {
 // CHECK-NEXT:  return [[RESULT]] : $()
 // CHECK-NEXT: }
 
-// vtable thunk for Derived.privateMethod2(_:) dispatching to MoreDerived.privateMethod2(_:)
 // CHECK-LABEL: sil private [ossa] @$s17vtables_multifile11MoreDerivedC14privateMethod2yyyXlSgFAA0D0CADyyAEFTV : $@convention(method) (@guaranteed Optional<AnyObject>, @guaranteed MoreDerived) -> () {
 // CHECK: bb0(%0 : @guaranteed $Optional<AnyObject>, %1 : @guaranteed $MoreDerived):
 // CHECK-NEXT:  [[METHOD:%.*]] = class_method %1 : $MoreDerived, #MoreDerived.privateMethod2!1 : (MoreDerived) -> (AnyObject?) -> (), $@convention(method) (@guaranteed Optional<AnyObject>, @guaranteed MoreDerived) -> ()
@@ -136,7 +142,6 @@ open class MostDerived : MoreDerived {
 // CHECK-NEXT:  return [[RESULT]] : $()
 // CHECK-NEXT: }
 
-// vtable thunk for Derived.privateMethod3(_:) dispatching to MoreDerived.privateMethod3(_:)
 // CHECK-LABEL: il private [ossa] @$s17vtables_multifile11MoreDerivedC14privateMethod3yySiSgFAA0D0CADyyAEFTV : $@convention(method) (Optional<Int>, @guaranteed MoreDerived) -> () {
 // CHECK: bb0(%0 : $Optional<Int>, %1 : @guaranteed $MoreDerived):
 // CHECK-NEXT:  [[METHOD:%.*]] = class_method %1 : $MoreDerived, #MoreDerived.privateMethod3!1 : (MoreDerived) -> (Int?) -> (), $@convention(method) (Optional<Int>, @guaranteed MoreDerived) -> ()
@@ -145,7 +150,6 @@ open class MostDerived : MoreDerived {
 // CHECK-NEXT:  return [[RESULT]] : $()
 // CHECK-NEXT: }
 
-// vtable thunk for Derived.privateMethod4(_:) dispatching to MoreDerived.privateMethod4(_:)
 // CHECK-LABEL: sil private [ossa] @$s17vtables_multifile11MoreDerivedC14privateMethod4yySiFAA0D0CADyySiFTV : $@convention(method) (Int, @guaranteed MoreDerived) -> () {
 // CHECK: bb0(%0 : $Int, %1 : @guaranteed $MoreDerived):
 // CHECK-NEXT:  [[METHOD:%.*]] = class_method %1 : $MoreDerived, #MoreDerived.privateMethod4!1 : (MoreDerived) -> (Int) -> (), $@convention(method) (Int, @guaranteed MoreDerived) -> ()
@@ -154,6 +158,30 @@ open class MostDerived : MoreDerived {
 // CHECK-NEXT:  return [[RESULT]] : $()
 // CHECK-NEXT: }
 
+// --
+// Thunks for final overrides do not re-dispatch, even if the override is more
+// visible.
+// --
+
+// CHECK-LABEL: sil private [ossa] @$s17vtables_multifile12FinalDerivedC14privateMethod3yySiSgFAA4BaseCAD33_63E5F2521A3C787F5F9EFD57FB9237EALLyySiFTV : $@convention(method) (Int, @guaranteed FinalDerived) -> () {
+// CHECK: bb0(%0 : $Int, %1 : @guaranteed $FinalDerived):
+// CHECK-NEXT:  [[ARG:%.*]] = enum $Optional<Int>, #Optional.some!enumelt.1, %0 : $Int // user: %4
+// CHECK:       [[METHOD:%.*]] = function_ref @$s17vtables_multifile12FinalDerivedC14privateMethod3yySiSgF : $@convention(method) (Optional<Int>, @guaranteed FinalDerived) -> ()
+// CHECK-NEXT:  apply [[METHOD]]([[ARG]], %1) : $@convention(method) (Optional<Int>, @guaranteed FinalDerived) -> ()
+// CHECK-NEXT:  [[RESULT:%.*]] = tuple ()
+// CHECK-NEXT:  return [[RESULT]] : $()
+// CHECK-NEXT: }
+
+// CHECK-LABEL: sil private [ossa] @$s17vtables_multifile12FinalDerivedC14privateMethod4yySiFAA4BaseCAD33_63E5F2521A3C787F5F9EFD57FB9237EALLyyxFTV : $@convention(method) (@in_guaranteed Int, @guaranteed FinalDerived) -> () {
+// CHECK: bb0(%0 : $*Int, %1 : @guaranteed $FinalDerived):
+// CHECK-NEXT:  [[ARG:%.*]] = load [trivial] %0 : $*Int
+// CHECK:       [[METHOD:%.*]] = function_ref @$s17vtables_multifile12FinalDerivedC14privateMethod4yySiF : $@convention(method) (Int, @guaranteed FinalDerived) -> ()
+// CHECK-NEXT:  [[RESULT:%.*]] = apply [[METHOD]]([[ARG]], %1) : $@convention(method) (Int, @guaranteed FinalDerived) -> ()
+// CHECK-NEXT:  [[RESULT:%.*]] = tuple ()
+// CHECK-NEXT:  return [[RESULT]] : $()
+// CHECK-NEXT: }
+
+
 // --
 // VTable for Derived.
 // --
@@ -212,3 +240,16 @@ open class MostDerived : MoreDerived {
 // CHECK-NEXT:   #MoreDerived.privateMethod4!1: (MoreDerived) -> (Int) -> () : @$s17vtables_multifile11MostDerivedC14privateMethod4yySiF [override] // MostDerived.privateMethod4(_:)
 // CHECK-NEXT:   #MostDerived.deinit!deallocator.1: @$s17vtables_multifile11MostDerivedCfD     // MostDerived.__deallocating_deinit
 // CHECK-NEXT: }
+
+// --
+// FinalDerived adds a final override; make sure we handle this correctly.
+// --
+
+// CHECK-LABEL: sil_vtable [serialized] FinalDerived {
+// CHECK-NEXT:   #Base.privateMethod1!1: <T> (Base<T>) -> () -> () : @$s17vtables_multifile12FinalDerivedC14privateMethod1yyF [override]	// FinalDerived.privateMethod1()
+// CHECK-NEXT:   #Base.privateMethod2!1: <T> (Base<T>) -> (AnyObject) -> () : @$s17vtables_multifile12FinalDerivedC14privateMethod2yyyXlSgF [override]	// FinalDerived.privateMethod2(_:)
+// CHECK-NEXT:   #Base.privateMethod3!1: <T> (Base<T>) -> (Int) -> () : @$s17vtables_multifile12FinalDerivedC14privateMethod3yySiSgFAA4BaseCAD33_63E5F2521A3C787F5F9EFD57FB9237EALLyySiFTV [override]	// vtable thunk for Base.privateMethod3(_:) dispatching to FinalDerived.privateMethod3(_:)
+// CHECK-NEXT:   #Base.privateMethod4!1: <T> (Base<T>) -> (T) -> () : @$s17vtables_multifile12FinalDerivedC14privateMethod4yySiFAA4BaseCAD33_63E5F2521A3C787F5F9EFD57FB9237EALLyyxFTV [override]	// vtable thunk for Base.privateMethod4(_:) dispatching to FinalDerived.privateMethod4(_:)
+// CHECK-NEXT:   #Base.init!allocator.1: <T> (Base<T>.Type) -> () -> Base<T> : @$s17vtables_multifile12FinalDerivedCACycfC [override]	// FinalDerived.__allocating_init()
+// CHECK-NEXT:   #FinalDerived.deinit!deallocator.1: @$s17vtables_multifile12FinalDerivedCfD	// FinalDerived.__deallocating_deinit
+// CHECK-NEXT: }