Merge pull request #22989 from slavapestov/ctor-subclass-scope

SIL: Fix subclass scope calculation for constructors

Author: slavapestov

lib/SIL/SILDeclRef.cpp

@@ -944,47 +944,62 @@ SubclassScope SILDeclRef::getSubclassScope() const {
   if (!hasDecl())
     return SubclassScope::NotApplicable;
 
+  auto *decl = getDecl();
+
+  if (!isa<AbstractFunctionDecl>(decl))
+    return SubclassScope::NotApplicable;
+
   // If this declaration is a function which goes into a vtable, then it's
   // symbol must be as visible as its class, because derived classes have to put
   // all less visible methods of the base class into their vtables.
 
-  if (auto *CD = dyn_cast<ConstructorDecl>(getDecl()))
-    if (!CD->isRequired())
+  if (auto *CD = dyn_cast<ConstructorDecl>(decl)) {
+    // Initializing entry points do not appear in the vtable.
+    if (kind == SILDeclRef::Kind::Initializer)
       return SubclassScope::NotApplicable;
-
-  auto *FD = dyn_cast<FuncDecl>(getDecl());
-  if (!FD)
+    // Non-required convenience inits do not apper in the vtable.
+    if (!CD->isRequired() && !CD->isDesignatedInit())
+      return SubclassScope::NotApplicable;
+  } else if (isa<DestructorDecl>(decl)) {
+    // Detructors do not appear in the vtable.
     return SubclassScope::NotApplicable;
+  } else {
+    assert(isa<FuncDecl>(decl));
+  }
 
-  DeclContext *context = FD->getDeclContext();
+  DeclContext *context = decl->getDeclContext();
 
-  // Methods from extensions don't go into vtables (yet).
+  // Methods from extensions don't go in the vtable.
   if (isa<ExtensionDecl>(context))
     return SubclassScope::NotApplicable;
 
   // Various forms of thunks don't either.
   if (isThunk() || isForeign)
     return SubclassScope::NotApplicable;
 
-  // Default arg generators are not visible.
+  // Default arg generators don't go in the vtable.
   if (isDefaultArgGenerator())
     return SubclassScope::NotApplicable;
 
+  // Only non-final methods in non-final classes go in the vtable.
   auto *classType = context->getSelfClassDecl();
   if (!classType || classType->isFinal())
     return SubclassScope::NotApplicable;
 
-  if (FD->isFinal())
+  if (decl->isFinal())
     return SubclassScope::NotApplicable;
 
-  assert(FD->getEffectiveAccess() <= classType->getEffectiveAccess() &&
+  assert(decl->getEffectiveAccess() <= classType->getEffectiveAccess() &&
          "class must be as visible as its members");
 
   // FIXME: This is too narrow. Any class with resilient metadata should
   // probably have this, at least for method overrides that don't add new
   // vtable entries.
-  if (classType->isResilient())
+  if (classType->isResilient()) {
+    if (isa<ConstructorDecl>(decl))
+      return SubclassScope::NotApplicable;
     return SubclassScope::Resilient;
+  }
 
   switch (classType->getEffectiveAccess()) {
   case AccessLevel::Private:

test/IRGen/Inputs/vtable_symbol_linkage_base.swift

@@ -11,6 +11,9 @@ open class Base {
   internal var internalVar: Int = 30
 }
 
+open class Middle : Base {
+  public init(x: Int) {}
+}
 
 public struct Namespace {
   open class Nested {

test/IRGen/vtable_symbol_linkage.swift

@@ -10,8 +10,9 @@
 
 import BaseModule
 
-public class Derived : Base {
-}
+public class Derived : Base {}
+
+public class MostDerived : Middle {}
 
 public class DerivedNested : Namespace.Nested {}
 public class DerivedExtNested : Namespace.ExtNested {}