SILGen: Use SILVTableVisitor instead of bespoke member traversal

This changes the order in which declarations are emitted.
It also means we no longer emit a vtable entry for the
materializeForSet of dynamic storage. Neither of these are
intended to have any functional effect.

Author: slavapestov

lib/SIL/SILDeclRef.cpp

@@ -772,12 +772,14 @@ SILDeclRef SILDeclRef::getNextOverriddenVTableEntry() const {
     // @NSManaged property, then it won't be in the vtable.
     if (overridden.getDecl()->hasClangNode())
       return SILDeclRef();
-    if (overridden.getDecl()->getAttrs().hasAttribute<DynamicAttr>())
+    if (overridden.getDecl()->isDynamic())
       return SILDeclRef();
     if (auto *ovFD = dyn_cast<FuncDecl>(overridden.getDecl()))
       if (auto *asd = ovFD->getAccessorStorageDecl()) {
         if (asd->hasClangNode())
           return SILDeclRef();
+        if (asd->isDynamic())
+          return SILDeclRef();
       }
 
     // If we overrode a decl from an extension, it won't be in a vtable

lib/SILGen/SILGenType.cpp

@@ -24,6 +24,7 @@
 #include "swift/AST/ProtocolConformance.h"
 #include "swift/AST/TypeMemberVisitor.h"
 #include "swift/SIL/SILArgument.h"
+#include "swift/SIL/SILVTableVisitor.h"
 #include "swift/SIL/TypeLowering.h"
 
 using namespace swift;
@@ -146,154 +147,83 @@ bool SILGenModule::requiresObjCMethodEntryPoint(ConstructorDecl *constructor) {
 namespace {
 
 /// An ASTVisitor for populating SILVTable entries from ClassDecl members.
-class SILGenVTable : public Lowering::ASTVisitor<SILGenVTable> {
+class SILGenVTable : public SILVTableVisitor<SILGenVTable> {
 public:
   SILGenModule &SGM;
   ClassDecl *theClass;
   std::vector<SILVTable::Entry> vtableEntries;
 
   SILGenVTable(SILGenModule &SGM, ClassDecl *theClass)
     : SGM(SGM), theClass(theClass)
-  {
-    // Populate the superclass members, if any.
-    Type super = theClass->getSuperclass();
-    if (super && super->getClassOrBoundGenericClass())
-      visitAncestor(super->getClassOrBoundGenericClass());
-  }
+  { }
 
-  ~SILGenVTable() {
-    // Create the vtable.
-    SILVTable::create(SGM.M, theClass, vtableEntries);
-  }
+  void emitVTable() {
+    // Populate the superclass members, if any.
+    visitAncestor(theClass);
 
-  void visitAncestor(ClassDecl *ancestor) {
-    // Recursively visit all our ancestors.
-    Type super = ancestor->getSuperclass();
-    if (super && super->getClassOrBoundGenericClass())
-      visitAncestor(super->getClassOrBoundGenericClass());
-
-    // Only visit the members for a class defined natively.
-    if (!ancestor->hasClangNode()) {
-      for (auto member : ancestor->getMembers())
-        visit(member);
-    }
-  }
+    auto *dtor = theClass->getDestructor();
+    assert(dtor);
 
-  // Add an entry to the vtable.
-  void addEntry(SILDeclRef member) {
-    /// Get the function to reference from the vtable.
-    auto getVtableEntry = [&](SILDeclRef entry) -> SILVTable::Entry {
-      // If the member is dynamic, reference its dynamic dispatch thunk so that
-      // it will be redispatched, funneling the method call through the runtime
-      // hook point.
-      // TODO: Dynamic thunks could conceivably require reabstraction too.
-      if (member.getDecl()->getAttrs().hasAttribute<DynamicAttr>())
-        return { entry,
-                 SGM.getDynamicThunk(member, SGM.Types.getConstantInfo(member)),
-                 SILLinkage::Public };
-
-      // The derived method may require thunking to match up to the ABI of the
-      // base method.
-      SILLinkage implLinkage;
-      SILFunction *implFn = SGM.emitVTableMethod(member, entry, implLinkage);
-      return { entry, implFn, stripExternalFromLinkage(implLinkage) };
-    };
-
-    // Try to find an overridden entry.
-    // NB: Mutates vtableEntries in-place
-    // FIXME: O(n^2)
-    if (auto overridden = member.getNextOverriddenVTableEntry()) {
-      for (SILVTable::Entry &entry : vtableEntries) {
-        SILDeclRef ref = overridden;
-
-        do {
-          // Replace the overridden member.
-          if (entry.Method == ref) {
-            // The entry is keyed by the least derived method.
-            entry = getVtableEntry(ref);
-            return;
-          }
-        } while ((ref = ref.getOverridden()));
-      }
-      llvm_unreachable("no overridden vtable entry?!");
-    }
+    // Add the deallocating destructor to the vtable just for the purpose
+    // that it is referenced and cannot be eliminated by dead function removal.
+    // In reality, the deallocating destructor is referenced directly from
+    // the HeapMetadata for the class.
+    if (!dtor->hasClangNode())
+      addMethod(SILDeclRef(dtor, SILDeclRef::Kind::Deallocator));
 
-    // If this is a final member and isn't overriding something, we don't need
-    // to add it to the vtable.
-    if (member.getDecl()->isFinal())
-      return;
-    // If this is dynamic and isn't overriding a non-dynamic method, it'll
-    // always be accessed by objc_msgSend, so we don't need to add it to the
-    // vtable.
-    if (member.getDecl()->getAttrs().hasAttribute<DynamicAttr>())
-      return;
+    if (SGM.requiresIVarDestroyer(theClass))
+      addMethod(SILDeclRef(theClass, SILDeclRef::Kind::IVarDestroyer));
 
-    // Otherwise, introduce a new vtable entry.
-    vtableEntries.push_back(getVtableEntry(member));
+    // Create the vtable.
+    SILVTable::create(SGM.M, theClass, vtableEntries);
   }
 
-  // Default for members that don't require vtable entries.
-  void visitDecl(Decl*) {}
-
-  void visitFuncDecl(FuncDecl *fd) {
-    // ObjC decls don't go in vtables.
-    if (fd->hasClangNode())
-      return;
-
-    // Observers don't get separate vtable entries.
-    if (fd->isObservingAccessor())
-      return;
+  void visitAncestor(ClassDecl *ancestor) {
+    auto superTy = ancestor->getSuperclass();
+    if (superTy)
+      visitAncestor(superTy->getClassOrBoundGenericClass());
 
-    addEntry(SILDeclRef(fd));
+    addVTableEntries(ancestor);
   }
 
-  void visitConstructorDecl(ConstructorDecl *cd) {
-    // Stub constructors don't get an entry, unless they were synthesized to
-    // override a non-required designated initializer in the superclass.
-    if (cd->hasStubImplementation() && !cd->getOverriddenDecl())
-      return;
-
-    // Required constructors (or overrides thereof) have their allocating entry
-    // point in the vtable.
-    bool isRequired = false;
-    auto override = cd;
-    while (override) {
-      if (override->isRequired()) {
-        isRequired = true;
-        break;
-      }
-      override = override->getOverriddenDecl();
+  SILVTable::Entry getVTableEntry(SILDeclRef baseRef, SILDeclRef declRef) {
+    // If the member is dynamic, reference its dynamic dispatch thunk so that
+    // it will be redispatched, funneling the method call through the runtime
+    // hook point.
+    // TODO: Dynamic thunks could conceivably require reabstraction too.
+    if (declRef.getDecl()->isDynamic()) {
+      return { baseRef,
+          SGM.getDynamicThunk(declRef, SGM.Types.getConstantInfo(declRef)),
+          SILLinkage::Public };
     }
-    if (isRequired) {
-      addEntry(SILDeclRef(cd, SILDeclRef::Kind::Allocator));
-    }
-
-    // All constructors have their initializing constructor in the
-    // vtable, which can be used by a convenience initializer.
-    addEntry(SILDeclRef(cd, SILDeclRef::Kind::Initializer));
-  }
 
-  void visitVarDecl(VarDecl *vd) {
-    // Note: dynamically-dispatched properties have their getter and setter
-    // added to the vtable when they are visited.
+    // The derived method may require thunking to match up to the ABI of the
+    // base method.
+    SILLinkage implLinkage;
+    SILFunction *implFn = SGM.emitVTableMethod(declRef, baseRef, implLinkage);
+    return { baseRef, implFn, stripExternalFromLinkage(implLinkage) };
   }
 
-  void visitDestructorDecl(DestructorDecl *dd) {
-    if (dd->getParent()->getAsClassOrClassExtensionContext() == theClass) {
-      // Add the deallocating destructor to the vtable just for the purpose
-      // that it is referenced and cannot be eliminated by dead function removal.
-      // In reality, the deallocating destructor is referenced directly from
-      // the HeapMetadata for the class.
-      addEntry(SILDeclRef(dd, SILDeclRef::Kind::Deallocator));
-
-      if (SGM.requiresIVarDestroyer(theClass))
-        addEntry(SILDeclRef(theClass, SILDeclRef::Kind::IVarDestroyer));
+  // Try to find an overridden entry.
+  void addMethodOverride(SILDeclRef baseRef, SILDeclRef declRef) {
+    // NB: Mutates vtableEntries in-place
+    // FIXME: O(n^2)
+    for (SILVTable::Entry &entry : vtableEntries) {
+      // Replace the overridden member.
+      if (entry.Method == baseRef) {
+        // The entry is keyed by the least derived method.
+        entry = getVTableEntry(baseRef, declRef);
+        return;
+      }
     }
+    baseRef.dump();
+    declRef.dump();
+    llvm_unreachable("no overridden vtable entry?!");
   }
 
-  void visitSubscriptDecl(SubscriptDecl *sd) {
-    // Note: dynamically-dispatched properties have their getter and setter
-    // added to the vtable when they are visited.
+  // Add an entry to the vtable.
+  void addMethod(SILDeclRef member) {
+    vtableEntries.push_back(getVTableEntry(member, member));
   }
 };
 
@@ -319,24 +249,22 @@ class SILGenType : public TypeMemberVisitor<SILGenType> {
 public:
   SILGenModule &SGM;
   NominalTypeDecl *theType;
-  Optional<SILGenVTable> genVTable;
 
   SILGenType(SILGenModule &SGM, NominalTypeDecl *theType)
     : SGM(SGM), theType(theType) {}
 
   /// Emit SIL functions for all the members of the type.
   void emitType() {
-    // Start building a vtable if this is a class.
-    if (auto theClass = dyn_cast<ClassDecl>(theType))
-      genVTable.emplace(SGM, theClass);
-
-    for (Decl *member : theType->getMembers()) {
-      if (genVTable)
-        genVTable->visit(member);
-
+    for (Decl *member : theType->getMembers())
       visit(member);
+
+    // Build a vtable if this is a class.
+    if (auto theClass = dyn_cast<ClassDecl>(theType)) {
+      SILGenVTable genVTable(SGM, theClass);
+      genVTable.emitVTable();
     }
 
+    // Build a default witness table if this is a protocol.
     if (auto protocol = dyn_cast<ProtocolDecl>(theType)) {
       if (!protocol->isObjC())
         SGM.emitDefaultWitnessTable(protocol);

test/DebugInfo/initializer.swift

@@ -6,13 +6,13 @@ protocol Named {
     var name : String { get }
 }
 
-// initializer.Person.init (initializer.Person.Type)() -> initializer.Person
-// CHECK: define hidden {{.*}}%T11initializer6PersonC* @_T011initializer6PersonCACycfc(%T11initializer6PersonC*{{.*}}) {{.*}} {
-
 // initializer.Person.__allocating_init (initializer.Person.Type)() -> initializer.Person
 // CHECK: define hidden {{.*}}%T11initializer6PersonC* @_T011initializer6PersonCACycfC(%swift.type*{{.*}}) {{.*}} {
 // CHECK:  call {{.*}}%T11initializer6PersonC* @_T011initializer6PersonCACycfc(%T11initializer6PersonC* {{.*}}%3), !dbg ![[ALLOCATING_INIT:.*]]
 
+// initializer.Person.init (initializer.Person.Type)() -> initializer.Person
+// CHECK: define hidden {{.*}}%T11initializer6PersonC* @_T011initializer6PersonCACycfc(%T11initializer6PersonC*{{.*}}) {{.*}} {
+
 // CHECK-DAG: ![[ALLOCATING_INIT]]  = !DILocation(line: 0, scope
 class Person : Named {
     var name : String { get { return "No Name" } }

test/SILGen/SILDeclRef.swift

@@ -55,16 +55,16 @@ public func testBase(b: Base) -> Int32 {
 // Check that vtables and witness tables contain SILDeclRefs with signatures.
 
 // CHECK: sil_vtable Base {
-// CHECK:  #Base.foo!1: (Base) -> () -> Int32 : _T010SILDeclRef4BaseC3foos5Int32VyF	// Base.foo() -> Int32
-// CHECK:  #Base.foo!1: (Base) -> (Int32) -> () : _T010SILDeclRef4BaseC3fooys5Int32V1n_tF	// Base.foo(n : Int32) -> ()
-// CHECK:  #Base.foo!1: (Base) -> (Float) -> Int32 : _T010SILDeclRef4BaseC3foos5Int32VSf1f_tF	// Base.foo(f : Float) -> Int32
-// CHECK:  #Base.deinit!deallocator: _T010SILDeclRef4BaseCfD	// Base.__deallocating_deinit
-// CHECK:  #Base.init!initializer.1: (Base.Type) -> () -> Base : _T010SILDeclRef4BaseCACycfc	// Base.init() -> Base
-// CHECK: }
+// CHECK-NEXT:  #Base.foo!1: (Base) -> () -> Int32 : _T010SILDeclRef4BaseC3foos5Int32VyF	// Base.foo() -> Int32
+// CHECK-NEXT:  #Base.foo!1: (Base) -> (Int32) -> () : _T010SILDeclRef4BaseC3fooys5Int32V1n_tF	// Base.foo(n : Int32) -> ()
+// CHECK-NEXT:  #Base.foo!1: (Base) -> (Float) -> Int32 : _T010SILDeclRef4BaseC3foos5Int32VSf1f_tF	// Base.foo(f : Float) -> Int32
+// CHECK-NEXT:  #Base.init!initializer.1: (Base.Type) -> () -> Base : _T010SILDeclRef4BaseCACycfc	// Base.init() -> Base
+// CHECK-NEXT:  #Base.deinit!deallocator: _T010SILDeclRef4BaseCfD	// Base.__deallocating_deinit
+// CHECK-NEXT: }
 
 // CHECK:sil_witness_table [fragile] Base: P module SILDeclRef {
-// CHECK: method #P.foo!1: <Self where Self : P> (Self) -> () -> Int32 : @_T010SILDeclRef4BaseCAA1PA2aDP3foos5Int32VyFTW	// protocol witness for P.foo() -> Int32 in conformance Base
-// CHECK: method #P.foo!1: <Self where Self : P> (Self) -> (Int32) -> () : @_T010SILDeclRef4BaseCAA1PA2aDP3fooys5Int32V1n_tFTW	// protocol witness for P.foo(n : Int32) -> () in conformance Base
-// CHECK: }
+// CHECK-NEXT: method #P.foo!1: <Self where Self : P> (Self) -> () -> Int32 : @_T010SILDeclRef4BaseCAA1PA2aDP3foos5Int32VyFTW	// protocol witness for P.foo() -> Int32 in conformance Base
+// CHECK-NEXT: method #P.foo!1: <Self where Self : P> (Self) -> (Int32) -> () : @_T010SILDeclRef4BaseCAA1PA2aDP3fooys5Int32V1n_tFTW	// protocol witness for P.foo(n : Int32) -> () in conformance Base
+// CHECK-NEXT: }
 
 

test/SILGen/complete_object_init.swift

@@ -3,7 +3,14 @@
 struct X { }
 
 class A {
-  // CHECK-LABEL: sil hidden @_T020complete_object_init1AC{{[_0-9a-zA-Z]*}}fc : $@convention(method) (@owned A) -> @owned A
+// CHECK-LABEL: sil hidden @_T020complete_object_init1AC{{[_0-9a-zA-Z]*}}fC : $@convention(method) (@thick A.Type) -> @owned A
+// CHECK: bb0([[SELF_META:%[0-9]+]] : $@thick A.Type):
+// CHECK:   [[SELF:%[0-9]+]] = alloc_ref_dynamic [[SELF_META]] : $@thick A.Type, $A
+// CHECK:   [[OTHER_INIT:%[0-9]+]] = function_ref @_T020complete_object_init1AC{{[_0-9a-zA-Z]*}}fc : $@convention(method) (@owned A) -> @owned A
+// CHECK:   [[RESULT:%[0-9]+]] = apply [[OTHER_INIT]]([[SELF]]) : $@convention(method) (@owned A) -> @owned A
+// CHECK:   return [[RESULT]] : $A
+
+// CHECK-LABEL: sil hidden @_T020complete_object_init1AC{{[_0-9a-zA-Z]*}}fc : $@convention(method) (@owned A) -> @owned A
 // CHECK: bb0([[SELF_PARAM:%[0-9]+]] : $A):
 // CHECK:   [[SELF_BOX:%[0-9]+]] = alloc_box ${ var A }
 // CHECK:   [[PB:%.*]] = project_box [[SELF_BOX]]
@@ -20,13 +27,7 @@ class A {
 // CHECK:   destroy_value [[SELF_BOX]] : ${ var A }
 // CHECK:   return [[RESULT]] : $A
 
-  // CHECK-LABEL: sil hidden @_T020complete_object_init1AC{{[_0-9a-zA-Z]*}}fC : $@convention(method) (@thick A.Type) -> @owned A
   convenience init() {
-    // CHECK: bb0([[SELF_META:%[0-9]+]] : $@thick A.Type):
-    // CHECK:   [[SELF:%[0-9]+]] = alloc_ref_dynamic [[SELF_META]] : $@thick A.Type, $A
-    // CHECK:   [[OTHER_INIT:%[0-9]+]] = function_ref @_T020complete_object_init1AC{{[_0-9a-zA-Z]*}}fc : $@convention(method) (@owned A) -> @owned A
-    // CHECK:   [[RESULT:%[0-9]+]] = apply [[OTHER_INIT]]([[SELF]]) : $@convention(method) (@owned A) -> @owned A
-    // CHECK:   return [[RESULT]] : $A
     self.init(x: X())
   }
 

test/SILGen/dynamic.swift

@@ -448,6 +448,21 @@ public class Sub : Base {
   override var x: Bool { return false || super.x }
 }
 
+public class BaseExt : NSObject {}
+
+extension BaseExt {
+  public var count: Int {
+    return 0
+  }
+}
+
+public class SubExt : BaseExt {
+  public override var count: Int {
+    return 1
+  }
+}
+
+// Vtable contains entries for native and @objc methods, but not dynamic ones
 // CHECK-LABEL: sil_vtable Foo {
 // CHECK-NEXT:   #Foo.init!initializer.1: {{.*}} :   _T07dynamic3FooCACSi6native_tcfc
 // CHECK-NEXT:   #Foo.nativeMethod!1: {{.*}} :       _T07dynamic3FooC12nativeMethodyyF
@@ -465,13 +480,17 @@ public class Sub : Base {
 // CHECK-NEXT:   #Foo.subscript!getter.1: {{.*}} : _T07dynamic3FooC9subscriptSis9AnyObject_p4objc_tcfg // dynamic.Foo.subscript.getter : (objc : Swift.AnyObject) -> Swift.Int
 // CHECK-NEXT:   #Foo.subscript!setter.1: {{.*}} : _T07dynamic3FooC9subscriptSis9AnyObject_p4objc_tcfs // dynamic.Foo.subscript.setter : (objc : Swift.AnyObject) -> Swift.Int
 // CHECK-NEXT:   #Foo.subscript!materializeForSet
-// CHECK-NEXT:   #Foo.dynamicProp!materializeForSet.1: (Foo) -> (Builtin.RawPointer, inout Builtin.UnsafeValueBuffer) -> (Builtin.RawPointer, Builtin.RawPointer?) : _T07dynamic3FooC0A4PropSifm	// dynamic.Foo.dynamicProp.materializeForSet : Swift.Int
-// CHECK-NEXT:   #Foo.subscript!materializeForSet.1: (Foo) -> (Builtin.RawPointer, inout Builtin.UnsafeValueBuffer, Int) -> (Builtin.RawPointer, Builtin.RawPointer?) : _T07dynamic3FooC9subscriptS2iAA_tcfm	// dynamic.Foo.subscript.materializeForSet : (dynamic : Swift.Int) -> Swift.Int
 // CHECK-NEXT:   #Foo.overriddenByDynamic!1: {{.*}} : _T07dynamic3FooC19overriddenByDynamic{{[_0-9a-zA-Z]*}}
 // CHECK-NEXT:   #Foo.deinit!deallocator: {{.*}}
 // CHECK-NEXT: }
 
 // Vtable uses a dynamic thunk for dynamic overrides
 // CHECK-LABEL: sil_vtable Subclass {
-// CHECK-LABEL:   #Foo.overriddenByDynamic!1: {{.*}} : public _T07dynamic8SubclassC19overriddenByDynamic{{[_0-9a-zA-Z]*}}FTD
+// CHECK:   #Foo.overriddenByDynamic!1: {{.*}} : public _T07dynamic8SubclassC19overriddenByDynamic{{[_0-9a-zA-Z]*}}FTD
+// CHECK: }
 
+// No vtable entry for override of @objc extension property
+// CHECK-LABEL: sil_vtable SubExt {
+// CHECK-NEXT: #BaseExt.init!initializer.1: (BaseExt.Type) -> () -> BaseExt : _T07dynamic6SubExtCACycfc // dynamic.SubExt.init () -> dynamic.SubExt
+// CHECK-NEXT: #SubExt.deinit!deallocator: _T07dynamic6SubExtCfD // dynamic.SubExt.__deallocating_deinit
+// CHECK-NEXT: }