SILGen: Fix inconsistency between vtable layout and vtable entry emission.

As part of the criteria to determine whether a method override requires a new
vtable entry in addition to overriding the base class's vtable entry for the
method, we compare the visibility of the base class to that of the derived
class, since if further subclasses of the derived class cannot see the original
base class's method descriptor, they would not be able to directly override its
vtable entry.

However, there was a subtle inconsistency in our checks in two different
places: `NeedsNewVTableEntryRequest::evaluate` compared the visibility of the
derived method to its immediate override method, whereas
`SILGenModule::emitVTableMethod` compared visibility with the ultimate base
method that originated the vtable entry in question. Internal imports create
a situation where this leads to inconsistent answers, causing us to emit
a vtable thunk but without a corresponding new vtable entry, causing the thunk
to become self-recursive (rdar://147360093). If the base class is in a separate
module:

```
// Module1
open class Base {
    public required init() {}
}
```

and an intermediate class inherits that base class, it must `public import`
the module in the file defining the class to do so:

```
// Module2

public import Module1

open class MidDerived: Base { }
```

However, another file in the same module can further inherit that class, but
with only an `internal import` of the base module:

```
// Also Module2

import Module1

open class MostDerived: MidDerived { }
```

The implicit `init()` is `public` in each class, but from the perspective of
`MostDerived.init`, `Base.init` is `internal` because of the import. However,
the fact that its immediate parent is `public` should be sufficient evidence
that its original definition had visibility to `Base`, so no thunk should
be necessary.

Author: jckarter

lib/SILGen/SILGenType.cpp

@@ -112,10 +112,18 @@ SILGenModule::emitVTableMethod(ClassDecl *theClass, SILDeclRef derived,
 
   // If the base method is less visible than the derived method, we need
   // a thunk.
+  //
+  // Note that we check the visibility of the derived method's immediate base
+  // here, rather than the ultimate base of the vtable entry, because it is
+  // possible through import visibility for an intermediate base class in one
+  // file to have public visibility to the ultimate base via a public import,
+  // but then in turn be overridden by a derived class in another file in
+  // the same module that either doesn't import the ultimate base class's
+  // module or else imports it non-publicly in its file.
   bool baseLessVisibleThanDerived =
     (!usesObjCDynamicDispatch &&
      !derivedDecl->isFinal() &&
-     derivedDecl->isMoreVisibleThan(baseDecl));
+     derivedDecl->isMoreVisibleThan(derivedDecl->getOverriddenDecl()));
 
   // Determine the derived thunk type by lowering the derived type against the
   // abstraction pattern of the base.

test/SILGen/Inputs/vtable_internal_imported_ancestor/Repro1.swift

@@ -0,0 +1,3 @@
+open class Base {
+    public required init() {}
+}

test/SILGen/Inputs/vtable_internal_imported_ancestor/Repro2.swift

@@ -0,0 +1,4 @@
+public import Repro1
+
+open class MidDerived: Base {
+}

test/SILGen/vtable_internal_imported_ancestor.swift

@@ -0,0 +1,14 @@
+// RUN: %empty-directory(%t)
+// RUN: %target-swift-frontend -enable-library-evolution -emit-module-path %t/Repro1.swiftmodule -module-name Repro1 %S/Inputs/vtable_internal_imported_ancestor/Repro1.swift
+// RUN: %target-swift-frontend -enable-upcoming-feature InternalImportsByDefault -I %t -emit-silgen %S/Inputs/vtable_internal_imported_ancestor/Repro2.swift -primary-file %s | %FileCheck %s
+
+// REQUIRES: swift_feature_InternalImportsByDefault
+
+import Repro1
+
+public class MostDerived: MidDerived {
+}
+
+// CHECK-NOT: vtable thunk
+// CHECK-LABEL: sil_vtable{{.*}} MostDerived {
+// CHECK:         #Base.init!allocator: {{.*}} @$s6Repro211MostDerivedCACycfC [override]

test/SILGen/vtables_multifile.swift

@@ -225,15 +225,15 @@ public final class FinalDerived : Base<Int> {
 // --
 
 // CHECK-LABEL: sil_vtable [serialized] MostDerived {
-// CHECK-NEXT:   #Base.privateMethod1: <T> (Base<T>) -> () -> () : @$s17vtables_multifile11MostDerivedC14privateMethod1yyFAA4BaseCAD33_63E5F2521A3C787F5F9EFD57FB9237EALLyyFTV [override]     // vtable thunk for Base.privateMethod1() dispatching to MostDerived.privateMethod1()
-// CHECK-NEXT:   #Base.privateMethod2: <T> (Base<T>) -> (AnyObject) -> () : @$s17vtables_multifile11MostDerivedC14privateMethod2yyyXlSgFAA4BaseCAD33_63E5F2521A3C787F5F9EFD57FB9237EALLyyyXlFTV [override]    // vtable thunk for Base.privateMethod2(_:) dispatching to MostDerived.privateMethod2(_:)
-// CHECK-NEXT:   #Base.privateMethod3: <T> (Base<T>) -> (Int) -> () : @$s17vtables_multifile11MostDerivedC14privateMethod3yySiSgFAA4BaseCAD33_63E5F2521A3C787F5F9EFD57FB9237EALLyySiFTV [override]    // vtable thunk for Base.privateMethod3(_:) dispatching to MostDerived.privateMethod3(_:)
-// CHECK-NEXT:   #Base.privateMethod4: <T> (Base<T>) -> (T) -> () : @$s17vtables_multifile11MostDerivedC14privateMethod4yySiFAA4BaseCAD33_63E5F2521A3C787F5F9EFD57FB9237EALLyyxFTV [override] // vtable thunk for Base.privateMethod4(_:) dispatching to MostDerived.privateMethod4(_:)
+// CHECK-NEXT:   #Base.privateMethod1: <T> (Base<T>) -> () -> () : @$s17vtables_multifile11MostDerivedC14privateMethod1yyF
+// CHECK-NEXT:   #Base.privateMethod2: <T> (Base<T>) -> (AnyObject) -> () : @$s17vtables_multifile11MostDerivedC14privateMethod2yyyXlSgF
+// CHECK-NEXT:   #Base.privateMethod3: <T> (Base<T>) -> (Int) -> () : @$s17vtables_multifile11MostDerivedC14privateMethod3yySiSgF
+// CHECK-NEXT:   #Base.privateMethod4: <T> (Base<T>) -> (T) -> () : @$s17vtables_multifile11MostDerivedC14privateMethod4yySiF
 // CHECK-NEXT:   #Base.init!allocator: <T> (Base<T>.Type) -> () -> Base<T> : @$s17vtables_multifile11MostDerivedCACycfC [override]   // MostDerived.__allocating_init()
-// CHECK-NEXT:   #Derived.privateMethod1: (Derived) -> () -> () : @$s17vtables_multifile11MostDerivedC14privateMethod1yyFAA0D0CADyyFTV [override]     // vtable thunk for Derived.privateMethod1() dispatching to MostDerived.privateMethod1()
-// CHECK-NEXT:   #Derived.privateMethod2: (Derived) -> (AnyObject?) -> () : @$s17vtables_multifile11MostDerivedC14privateMethod2yyyXlSgFAA0D0CADyyAEFTV [override]    // vtable thunk for Derived.privateMethod2(_:) dispatching to MostDerived.privateMethod2(_:)
-// CHECK-NEXT:   #Derived.privateMethod3: (Derived) -> (Int?) -> () : @$s17vtables_multifile11MostDerivedC14privateMethod3yySiSgFAA0D0CADyyAEFTV [override]   // vtable thunk for Derived.privateMethod3(_:) dispatching to MostDerived.privateMethod3(_:)
-// CHECK-NEXT:   #Derived.privateMethod4: (Derived) -> (Int) -> () : @$s17vtables_multifile11MostDerivedC14privateMethod4yySiFAA0D0CADyySiFTV [override]      // vtable thunk for Derived.privateMethod4(_:) dispatching to MostDerived.privateMethod4(_:)
+// CHECK-NEXT:   #Derived.privateMethod1: (Derived) -> () -> () : @$s17vtables_multifile11MostDerivedC14privateMethod1yyF
+// CHECK-NEXT:   #Derived.privateMethod2: (Derived) -> (AnyObject?) -> () : @$s17vtables_multifile11MostDerivedC14privateMethod2yyyXlSgF
+// CHECK-NEXT:   #Derived.privateMethod3: (Derived) -> (Int?) -> () : @$s17vtables_multifile11MostDerivedC14privateMethod3yySiSgF
+// CHECK-NEXT:   #Derived.privateMethod4: (Derived) -> (Int) -> () : @$s17vtables_multifile11MostDerivedC14privateMethod4yySiF
 // CHECK-NEXT:   #MoreDerived.privateMethod1: (MoreDerived) -> () -> () : @$s17vtables_multifile11MostDerivedC14privateMethod1yyF [override] // MostDerived.privateMethod1()
 // CHECK-NEXT:   #MoreDerived.privateMethod2: (MoreDerived) -> (AnyObject?) -> () : @$s17vtables_multifile11MostDerivedC14privateMethod2yyyXlSgF [override] // MostDerived.privateMethod2(_:)
 // CHECK-NEXT:   #MoreDerived.privateMethod3: (MoreDerived) -> (Int?) -> () : @$s17vtables_multifile11MostDerivedC14privateMethod3yySiSgF [override] // MostDerived.privateMethod3(_:)