[SIL] NFC: Make SILVTable follow C++ and LLVM best practices

1) Convert the `Entry` type to a class with getters/setters
2) Use llvm::TrailingObjects
3) Use llvm::PointerIntPair

Author: davezarzycki

include/swift/SIL/SILVTable.h

@@ -39,43 +39,54 @@ enum IsSerialized_t : unsigned char;
 class SILFunction;
 class SILModule;
 
+// TODO: Entry should include substitutions needed to invoke an overridden
+// generic base class method.
+class SILVTableEntry {
+  /// The declaration reference to the least-derived method visible through
+  /// the class.
+  SILDeclRef Method;
+
+  /// The function which implements the method for the class and the entry kind.
+  llvm::PointerIntPair<SILFunction *, 2, unsigned> ImplAndKind;
+
+public:
+  enum Kind : uint8_t {
+    /// The vtable entry is for a method defined directly in this class.
+    Normal,
+    /// The vtable entry is for a method defined directly in this class, and is
+    /// never overridden by subclasses.
+    NormalNonOverridden,
+    /// The vtable entry is inherited from the superclass.
+    Inherited,
+    /// The vtable entry is inherited from the superclass, and overridden
+    /// in this class.
+    Override,
+
+    // Please update the PointerIntPair above if you add/remove enums.
+  };
+
+  SILVTableEntry() : ImplAndKind(nullptr, Kind::Normal) {}
+
+  SILVTableEntry(SILDeclRef Method, SILFunction *Implementation, Kind TheKind)
+      : Method(Method), ImplAndKind(Implementation, TheKind) {}
+
+  SILDeclRef getMethod() const { return Method; }
+
+  Kind getKind() const { return Kind(ImplAndKind.getInt()); }
+  void setKind(Kind kind) { ImplAndKind.setInt(kind); }
+
+  SILFunction *getImplementation() const { return ImplAndKind.getPointer(); }
+};
+
 /// A mapping from each dynamically-dispatchable method of a class to the
 /// SILFunction that implements the method for that class.
 /// Note that dead methods are completely removed from the vtable.
-class SILVTable : public SILAllocated<SILVTable> {
+class SILVTable final : public SILAllocated<SILVTable>,
+                        llvm::TrailingObjects<SILVTable, SILVTableEntry> {
+  friend TrailingObjects;
+
 public:
-  // TODO: Entry should include substitutions needed to invoke an overridden
-  // generic base class method.
-  struct Entry {
-    enum Kind : uint8_t {
-      /// The vtable entry is for a method defined directly in this class.
-      Normal,
-      /// The vtable entry is for a method defined directly in this class, and is never overridden
-      /// by subclasses.
-      NormalNonOverridden,
-      /// The vtable entry is inherited from the superclass.
-      Inherited,
-      /// The vtable entry is inherited from the superclass, and overridden
-      /// in this class.
-      Override,
-    };
-
-    Entry()
-      : Implementation(nullptr), TheKind(Kind::Normal) { }
-
-    Entry(SILDeclRef Method, SILFunction *Implementation, Kind TheKind)
-      : Method(Method), Implementation(Implementation), TheKind(TheKind) { }
-
-    /// The declaration reference to the least-derived method visible through
-    /// the class.
-    SILDeclRef Method;
-
-    /// The function which implements the method for the class.
-    SILFunction *Implementation;
-
-    /// The entry kind.
-    Kind TheKind;
-  };
+  using Entry = SILVTableEntry;
 
   // Disallow copying into temporary objects.
   SILVTable(const SILVTable &other) = delete;
@@ -92,9 +103,6 @@ class SILVTable : public SILAllocated<SILVTable> {
   /// The number of SILVTables entries.
   unsigned NumEntries : 31;
 
-  /// Tail-allocated SILVTable entries.
-  Entry Entries[1];
-
   /// Private constructor. Create SILVTables by calling SILVTable::create.
   SILVTable(ClassDecl *c, IsSerialized_t serialized, ArrayRef<Entry> entries);
 
@@ -123,29 +131,34 @@ class SILVTable : public SILAllocated<SILVTable> {
   }
 
   /// Return all of the method entries.
-  ArrayRef<Entry> getEntries() const { return {Entries, NumEntries}; }
+  ArrayRef<Entry> getEntries() const {
+    return {getTrailingObjects<SILVTableEntry>(), NumEntries};
+  }
 
   /// Return all of the method entries mutably.
-  MutableArrayRef<Entry> getMutableEntries() { return {Entries, NumEntries}; }
+  MutableArrayRef<Entry> getMutableEntries() {
+    return {getTrailingObjects<SILVTableEntry>(), NumEntries};
+  }
 
   /// Look up the implementation function for the given method.
   Optional<Entry> getEntry(SILModule &M, SILDeclRef method) const;
 
   /// Removes entries from the vtable.
   /// \p predicate Returns true if the passed entry should be removed.
   template <typename Predicate> void removeEntries_if(Predicate predicate) {
-    Entry *end = std::remove_if(Entries, Entries + NumEntries,
-                                [&](Entry &entry) -> bool {
-      if (predicate(entry)) {
-        entry.Implementation->decrementRefCount();
-        removeFromVTableCache(entry);
-        return true;
-      }
-      return false;
-    });
-    NumEntries = end - Entries;
+    auto Entries = getMutableEntries();
+    Entry *end = std::remove_if(
+        Entries.begin(), Entries.end(), [&](Entry &entry) -> bool {
+          if (predicate(entry)) {
+            entry.getImplementation()->decrementRefCount();
+            removeFromVTableCache(entry);
+            return true;
+          }
+          return false;
+        });
+    NumEntries = std::distance(Entries.begin(), end);
   }
-                    
+
   /// Verify that the vtable is well-formed for the given class.
   void verify(const SILModule &M) const;
 

lib/IRGen/GenMeta.cpp

@@ -1577,8 +1577,8 @@ namespace {
       descriptor.addInt(IGM.Int32Ty, flags.getIntValue());
 
       if (auto entry = VTable->getEntry(IGM.getSILModule(), fn)) {
-        assert(entry->TheKind == SILVTable::Entry::Kind::Normal);
-        auto *implFn = IGM.getAddrOfSILFunction(entry->Implementation,
+        assert(entry->getKind() == SILVTable::Entry::Kind::Normal);
+        auto *implFn = IGM.getAddrOfSILFunction(entry->getImplementation(),
                                                 NotForDefinition);
         descriptor.addRelativeAddress(implFn);
       } else {
@@ -1625,8 +1625,8 @@ namespace {
 
       // The implementation of the override.
       if (auto entry = VTable->getEntry(IGM.getSILModule(), baseRef)) {
-        assert(entry->TheKind == SILVTable::Entry::Kind::Override);
-        auto *implFn = IGM.getAddrOfSILFunction(entry->Implementation,
+        assert(entry->getKind() == SILVTable::Entry::Kind::Override);
+        auto *implFn = IGM.getAddrOfSILFunction(entry->getImplementation(),
                                                 NotForDefinition);
         descriptor.addRelativeAddress(implFn);
       } else {
@@ -2981,7 +2981,7 @@ namespace {
       // The class is fragile. Emit a direct reference to the vtable entry.
       llvm::Constant *ptr;
       if (entry) {
-        ptr = IGM.getAddrOfSILFunction(entry->Implementation,
+        ptr = IGM.getAddrOfSILFunction(entry->getImplementation(),
                                        NotForDefinition);
       } else {
         // The method is removed by dead method elimination.

lib/SIL/IR/Linker.cpp

@@ -154,7 +154,7 @@ void SILLinkerVisitor::linkInVTable(ClassDecl *D) {
   for (auto P : Vtbl->getEntries()) {
     // Deserialize and recursively walk any vtable entries that do not have
     // bodies yet.
-    maybeAddFunctionToWorklist(P.Implementation);
+    maybeAddFunctionToWorklist(P.getImplementation());
   }
 }
 

lib/SIL/IR/SILModule.cpp

@@ -593,7 +593,7 @@ lookUpFunctionInVTable(ClassDecl *Class, SILDeclRef Member) {
   // Ok, we have a VTable. Try to lookup the SILFunction implementation from
   // the VTable.
   if (auto E = Vtbl->getEntry(*this, Member))
-    return E->Implementation;
+    return E->getImplementation();
 
   return nullptr;
 }

lib/SIL/IR/SILPrinter.cpp

@@ -3106,11 +3106,11 @@ void SILVTable::print(llvm::raw_ostream &OS, bool Verbose) const {
   PrintOptions QualifiedSILTypeOptions = PrintOptions::printQualifiedSILType();
   for (auto &entry : getEntries()) {
     OS << "  ";
-    entry.Method.print(OS);
+    entry.getMethod().print(OS);
     OS << ": ";
 
     bool HasSingleImplementation = false;
-    switch (entry.Method.kind) {
+    switch (entry.getMethod().kind) {
     default:
       break;
     case SILDeclRef::Kind::IVarDestroyer:
@@ -3122,13 +3122,13 @@ void SILVTable::print(llvm::raw_ostream &OS, bool Verbose) const {
     // single implementation, e.g. for destructors.
     if (!HasSingleImplementation) {
       QualifiedSILTypeOptions.CurrentModule =
-          entry.Method.getDecl()->getDeclContext()->getParentModule();
-      entry.Method.getDecl()->getInterfaceType().print(OS,
-                                                       QualifiedSILTypeOptions);
+          entry.getMethod().getDecl()->getDeclContext()->getParentModule();
+      entry.getMethod().getDecl()->getInterfaceType().print(
+          OS, QualifiedSILTypeOptions);
       OS << " : ";
     }
-    OS << '@' << entry.Implementation->getName();
-    switch (entry.TheKind) {
+    OS << '@' << entry.getImplementation()->getName();
+    switch (entry.getKind()) {
     case SILVTable::Entry::Kind::Normal:
       break;
     case SILVTable::Entry::Kind::NormalNonOverridden:
@@ -3141,7 +3141,7 @@ void SILVTable::print(llvm::raw_ostream &OS, bool Verbose) const {
       OS << " [override]";
       break;
     }
-    OS << "\t// " << demangleSymbol(entry.Implementation->getName());
+    OS << "\t// " << demangleSymbol(entry.getImplementation()->getName());
     OS << "\n";
   }
   OS << "}\n\n";

lib/SIL/IR/SILVTable.cpp

@@ -26,17 +26,14 @@ using namespace swift;
 SILVTable *SILVTable::create(SILModule &M, ClassDecl *Class,
                              IsSerialized_t Serialized,
                              ArrayRef<Entry> Entries) {
-  // SILVTable contains one element declared in Entries.  We must allocate
-  // space for it, because its default ctor will write to it.
-  unsigned NumTailElements = std::max((unsigned)Entries.size(), 1U)-1;
-  void *buf = M.allocate(sizeof(SILVTable) + sizeof(Entry) * NumTailElements,
-                         alignof(SILVTable));
+  auto size = totalSizeToAlloc<Entry>(Entries.size());
+  auto buf = M.allocate(size, alignof(SILVTable));
   SILVTable *vt = ::new (buf) SILVTable(Class, Serialized, Entries);
   M.vtables.push_back(vt);
   M.VTableMap[Class] = vt;
   // Update the Module's cache with new vtable + vtable entries:
   for (const Entry &entry : Entries) {
-    M.VTableEntryCache.insert({{vt, entry.Method}, entry});
+    M.VTableEntryCache.insert({{vt, entry.getMethod()}, entry});
   }
   return vt;
 }
@@ -54,24 +51,25 @@ SILVTable::getEntry(SILModule &M, SILDeclRef method) const {
 }
 
 void SILVTable::removeFromVTableCache(Entry &entry) {
-  SILModule &M = entry.Implementation->getModule();
-  M.VTableEntryCache.erase({this, entry.Method});
+  SILModule &M = entry.getImplementation()->getModule();
+  M.VTableEntryCache.erase({this, entry.getMethod()});
 }
 
 SILVTable::SILVTable(ClassDecl *c, IsSerialized_t serialized,
                      ArrayRef<Entry> entries)
   : Class(c), Serialized(serialized), NumEntries(entries.size()) {
-  memcpy(Entries, entries.begin(), sizeof(Entry) * NumEntries);
-  
+  std::uninitialized_copy(entries.begin(), entries.end(),
+                          getTrailingObjects<Entry>());
+
   // Bump the reference count of functions referenced by this table.
   for (const Entry &entry : getEntries()) {
-    entry.Implementation->incrementRefCount();
+    entry.getImplementation()->incrementRefCount();
   }
 }
 
 SILVTable::~SILVTable() {
   // Drop the reference count of functions referenced by this table.
   for (const Entry &entry : getEntries()) {
-    entry.Implementation->decrementRefCount();
+    entry.getImplementation()->decrementRefCount();
   }
 }

lib/SIL/Verifier/SILVerifier.cpp

@@ -5378,18 +5378,18 @@ void SILVTable::verify(const SILModule &M) const {
     auto &entry = getEntries()[i];
 
     // All vtable entries must be decls in a class context.
-    assert(entry.Method.hasDecl() && "vtable entry is not a decl");
-    auto baseInfo =
-        M.Types.getConstantInfo(TypeExpansionContext::minimal(), entry.Method);
-    ValueDecl *decl = entry.Method.getDecl();
+    assert(entry.getMethod().hasDecl() && "vtable entry is not a decl");
+    auto baseInfo = M.Types.getConstantInfo(TypeExpansionContext::minimal(),
+                                            entry.getMethod());
+    ValueDecl *decl = entry.getMethod().getDecl();
 
     assert((!isa<AccessorDecl>(decl)
             || !cast<AccessorDecl>(decl)->isObservingAccessor())
            && "observing accessors shouldn't have vtable entries");
 
     // For ivar destroyers, the decl is the class itself.
     ClassDecl *theClass;
-    if (entry.Method.kind == SILDeclRef::Kind::IVarDestroyer)
+    if (entry.getMethod().kind == SILDeclRef::Kind::IVarDestroyer)
       theClass = dyn_cast<ClassDecl>(decl);
     else
       theClass = dyn_cast<ClassDecl>(decl->getDeclContext());
@@ -5401,29 +5401,29 @@ void SILVTable::verify(const SILModule &M) const {
            "vtable entry must refer to a member of the vtable's class");
 
     // Foreign entry points shouldn't appear in vtables.
-    assert(!entry.Method.isForeign && "vtable entry must not be foreign");
-    
+    assert(!entry.getMethod().isForeign && "vtable entry must not be foreign");
+
     // The vtable entry must be ABI-compatible with the overridden vtable slot.
     SmallString<32> baseName;
     {
       llvm::raw_svector_ostream os(baseName);
-      entry.Method.print(os);
+      entry.getMethod().print(os);
     }
 
     if (M.getStage() != SILStage::Lowered) {
-      SILVerifier(*entry.Implementation)
+      SILVerifier(*entry.getImplementation())
           .requireABICompatibleFunctionTypes(
               baseInfo.getSILType().castTo<SILFunctionType>(),
-              entry.Implementation->getLoweredFunctionType(),
+              entry.getImplementation()->getLoweredFunctionType(),
               "vtable entry for " + baseName + " must be ABI-compatible",
-              *entry.Implementation);
+              *entry.getImplementation());
     }
 
     // Validate the entry against its superclass vtable.
     if (!superclass) {
       // Root methods should not have inherited or overridden entries.
       bool validKind;
-      switch (entry.TheKind) {
+      switch (entry.getKind()) {
       case Entry::Normal:
       case Entry::NormalNonOverridden:
         validKind = true;
@@ -5441,13 +5441,14 @@ void SILVTable::verify(const SILModule &M) const {
 
       const Entry *superEntry = nullptr;
       for (auto &se : superVTable->getEntries()) {
-        if (se.Method.getOverriddenVTableEntry() == entry.Method.getOverriddenVTableEntry()) {
+        if (se.getMethod().getOverriddenVTableEntry() ==
+            entry.getMethod().getOverriddenVTableEntry()) {
           superEntry = &se;
           break;
         }
       }
-      
-      switch (entry.TheKind) {
+
+      switch (entry.getKind()) {
       case Entry::Normal:
       case Entry::NormalNonOverridden:
         assert(!superEntry && "non-root vtable entry must be inherited or override");
@@ -5461,8 +5462,9 @@ void SILVTable::verify(const SILModule &M) const {
           break;
 
         // The superclass entry must not prohibit overrides.
-        assert(superEntry->TheKind != Entry::NormalNonOverridden
-               && "vtable entry overrides an entry that claims to have no overrides");
+        assert(
+            superEntry->getKind() != Entry::NormalNonOverridden &&
+            "vtable entry overrides an entry that claims to have no overrides");
         // TODO: Check the root vtable entry for the method too.
         break;
       }
@@ -5590,9 +5592,11 @@ void SILModule::verify() const {
     vt->verify(*this);
     // Check if there is a cache entry for each vtable entry
     for (auto entry : vt->getEntries()) {
-      if (VTableEntryCache.find({vt, entry.Method}) == VTableEntryCache.end()) {
+      if (VTableEntryCache.find({vt, entry.getMethod()}) ==
+          VTableEntryCache.end()) {
         llvm::errs() << "Vtable entry for function: "
-                     << entry.Implementation->getName() << "not in cache!\n";
+                     << entry.getImplementation()->getName()
+                     << "not in cache!\n";
         assert(false && "triggering standard assertion failure routine");
       }
       ++EntriesSZ;