Merge pull request swiftlang#33214 from jckarter/prune-vtables-internal-method-descriptors

IRGen: Don't drop method descriptors for vtable-elided internal methods.

Author: jckarter

include/swift/IRGen/Linking.h

@@ -575,7 +575,7 @@ class LinkEntity {
   static bool isValidResilientMethodRef(SILDeclRef declRef) {
     if (declRef.isForeign)
       return false;
-
+    
     auto *decl = declRef.getDecl();
     return (isa<ClassDecl>(decl->getDeclContext()) ||
             isa<ProtocolDecl>(decl->getDeclContext()));

lib/IRGen/GenMeta.cpp

@@ -1492,7 +1492,7 @@ namespace {
     void addMethod(SILDeclRef fn) {
       if (!VTable || methodRequiresReifiedVTableEntry(IGM, VTable, fn)) {
         VTableEntries.push_back(fn);
-      } else if (hasPublicVisibility(fn.getLinkage(NotForDefinition))) {
+      } else {
         // Emit a stub method descriptor and lookup function for nonoverridden
         // methods so that resilient code sequences can still use them.
         emitNonoverriddenMethod(fn);
@@ -1609,8 +1609,9 @@ namespace {
     }
 
     void emitMethodDescriptor(SILDeclRef fn) {
+
       // Define the method descriptor to point to the current position in the
-      // nominal type descriptor.
+      // nominal type descriptor, if it has a well-defined symbol name.
       IGM.defineMethodDescriptor(fn, Type,
                       B.getAddrOfCurrentPosition(IGM.MethodDescriptorStructTy));
 
@@ -1628,13 +1629,21 @@ namespace {
     }
 
     void emitNonoverriddenMethod(SILDeclRef fn) {
-      HasNonoverriddenMethods = true;
+      // TODO: Derivative functions do not distinguish themselves in the mangled
+      // names of method descriptor symbols yet, causing symbol name collisions.
+      if (fn.derivativeFunctionIdentifier)
+        return;
+
+     HasNonoverriddenMethods = true;
       // Although this method is non-overridden and therefore left out of the
       // vtable, we still need to maintain the ABI of a potentially-overridden
       // method for external clients.
 
       // Emit method dispatch thunk.
-      IGM.emitDispatchThunk(fn);
+      if (hasPublicVisibility(fn.getLinkage(NotForDefinition))) {
+        IGM.emitDispatchThunk(fn);
+      }
+      
       // Emit a freestanding method descriptor structure. This doesn't have to
       // exist in the table in the class's context descriptor since it isn't
       // in the vtable, but external clients need to be able to link against the

test/IRGen/vtable_non_overridden.sil

@@ -24,6 +24,10 @@ sil_vtable InternalA {
   #InternalA.bas : @InternalA_bas [nonoverridden]
 }
 
+// -- we should still generate method descriptors for the elided methods
+// CHECK-LABEL: @"$s21vtable_non_overridden9InternalAC3fooyyFTq" =
+// CHECK-LABEL: @"$s21vtable_non_overridden9InternalAC3basyyFTq" =
+
 // -- only overridden entries in internal method descriptor table
 // CHECK-LABEL: @"$s21vtable_non_overridden9InternalACMn" =
 // CHECK-SAME:     i32 2, %swift.method_descriptor