Merge pull request #9309 from jrose-apple/ast-vtables

Lift the decision of whether a method needs a vtable slot up to AST.

Author: jrose-apple

include/swift/AST/Decl.h

@@ -365,8 +365,14 @@ class alignas(1 << DeclAlignInBits) Decl {
 
     /// Whether the function body throws.
     unsigned Throws : 1;
+
+    /// Whether this function requires a new vtable entry.
+    unsigned NeedsNewVTableEntry : 1;
+
+    /// Whether NeedsNewVTableEntry is valid.
+    unsigned HasComputedNeedsNewVTableEntry : 1;
   };
-  enum { NumAbstractFunctionDeclBits = NumValueDeclBits + 11 };
+  enum { NumAbstractFunctionDeclBits = NumValueDeclBits + 13 };
   static_assert(NumAbstractFunctionDeclBits <= 32, "fits in an unsigned");
 
   class FuncDeclBitfields {
@@ -378,15 +384,8 @@ class alignas(1 << DeclAlignInBits) Decl {
 
     /// \brief Whether 'static' or 'class' was used.
     unsigned StaticSpelling : 2;
-
-    /// Whether this function is a 'mutating' method.
-    unsigned Mutating : 1;
-
-    /// Whether this function has a dynamic Self return type.
-    unsigned HasDynamicSelf : 1;
-    
   };
-  enum { NumFuncDeclBits = NumAbstractFunctionDeclBits + 5 };
+  enum { NumFuncDeclBits = NumAbstractFunctionDeclBits + 3 };
   static_assert(NumFuncDeclBits <= 32, "fits in an unsigned");
 
   class ConstructorDeclBitfields {
@@ -399,17 +398,8 @@ class alignas(1 << DeclAlignInBits) Decl {
     /// of the definition of the constructor that is useful only to semantic
     /// analysis and SIL generation.
     unsigned ComputedBodyInitKind : 3;
-
-    /// Whether this initializer is a stub placed into a subclass to
-    /// catch invalid delegations to a designated initializer not
-    /// overridden by the subclass. A stub will always trap at runtime.
-    ///
-    /// Initializer stubs can be invoked from Objective-C or through
-    /// the Objective-C runtime; there is no way to directly express
-    /// an object construction that will invoke a stub.
-    unsigned HasStubImplementation : 1;
   };
-  enum { NumConstructorDeclBits = NumAbstractFunctionDeclBits + 4 };
+  enum { NumConstructorDeclBits = NumAbstractFunctionDeclBits + 3 };
   static_assert(NumConstructorDeclBits <= 32, "fits in an unsigned");
 
   class TypeDeclBitfields {
@@ -4790,6 +4780,8 @@ class AbstractFunctionDecl : public ValueDecl, public GenericContext {
     AbstractFunctionDeclBits.NumParameterLists = NumParameterLists;
     AbstractFunctionDeclBits.Overridden = false;
     AbstractFunctionDeclBits.Throws = Throws;
+    AbstractFunctionDeclBits.NeedsNewVTableEntry = false;
+    AbstractFunctionDeclBits.HasComputedNeedsNewVTableEntry = false;
 
     // Verify no bitfield truncation.
     assert(AbstractFunctionDeclBits.NumParameterLists == NumParameterLists);
@@ -4903,6 +4895,21 @@ class AbstractFunctionDecl : public ValueDecl, public GenericContext {
     return getBodyKind() == BodyKind::MemberwiseInitializer;
   }
 
+  void setNeedsNewVTableEntry(bool value) {
+    AbstractFunctionDeclBits.HasComputedNeedsNewVTableEntry = true;
+    AbstractFunctionDeclBits.NeedsNewVTableEntry = value;
+  }
+
+  bool needsNewVTableEntry() const {
+    if (!AbstractFunctionDeclBits.HasComputedNeedsNewVTableEntry)
+      const_cast<AbstractFunctionDecl *>(this)->computeNeedsNewVTableEntry();
+    return AbstractFunctionDeclBits.NeedsNewVTableEntry;
+  }
+
+private:
+  void computeNeedsNewVTableEntry();
+
+public:
   /// Retrieve the source range of the function body.
   SourceRange getBodySourceRange() const;
 
@@ -5059,6 +5066,12 @@ class FuncDecl final : public AbstractFunctionDecl,
   /// Whether we are statically dispatched even if overridable
   unsigned ForcedStaticDispatch : 1;
 
+  /// Whether this function has a dynamic Self return type.
+  unsigned HasDynamicSelf : 1;
+
+  /// Whether this function is a 'mutating' method.
+  unsigned Mutating : 1;
+
   /// \brief If this FuncDecl is an accessor for a property, this indicates
   /// which property and what kind of accessor.
   llvm::PointerIntPair<AbstractStorageDecl*, 3, AccessorKind> AccessorDecl;
@@ -5086,9 +5099,9 @@ class FuncDecl final : public AbstractFunctionDecl,
       StaticLoc.isValid() || StaticSpelling != StaticSpellingKind::None;
     FuncDeclBits.StaticSpelling = static_cast<unsigned>(StaticSpelling);
     assert(NumParameterLists > 0 && "Must have at least an empty tuple arg");
-    FuncDeclBits.Mutating = false;
-    FuncDeclBits.HasDynamicSelf = false;
 
+    Mutating = false;
+    HasDynamicSelf = false;
     ForcedStaticDispatch = false;
     HaveSearchedForCommonOverloadReturnType = false;
     HaveFoundCommonOverloadReturnType = false;
@@ -5141,10 +5154,10 @@ class FuncDecl final : public AbstractFunctionDecl,
     FuncDeclBits.IsStatic = IsStatic;
   }
   bool isMutating() const {
-    return FuncDeclBits.Mutating;
+    return Mutating;
   }
-  void setMutating(bool Mutating = true) {
-    FuncDeclBits.Mutating = Mutating;
+  void setMutating(bool mutating = true) {
+    Mutating = mutating;
   }
 
   /// \brief Returns the parameter lists(s) for the function definition.
@@ -5266,11 +5279,11 @@ class FuncDecl final : public AbstractFunctionDecl,
 
   /// Determine whether this function has a dynamic \c Self return
   /// type.
-  bool hasDynamicSelf() const { return FuncDeclBits.HasDynamicSelf; }
+  bool hasDynamicSelf() const { return HasDynamicSelf; }
 
   /// Set whether this function has a dynamic \c Self return or not.
   void setDynamicSelf(bool hasDynamicSelf) { 
-    FuncDeclBits.HasDynamicSelf = hasDynamicSelf;
+    HasDynamicSelf = hasDynamicSelf;
   }
 
   void getLocalCaptures(SmallVectorImpl<CapturedValue> &Result) const {
@@ -5549,6 +5562,15 @@ class ConstructorDecl : public AbstractFunctionDecl {
   /// The failability of this initializer, which is an OptionalTypeKind.
   unsigned Failability : 2;
 
+  /// Whether this initializer is a stub placed into a subclass to
+  /// catch invalid delegations to a designated initializer not
+  /// overridden by the subclass. A stub will always trap at runtime.
+  ///
+  /// Initializer stubs can be invoked from Objective-C or through
+  /// the Objective-C runtime; there is no way to directly express
+  /// an object construction that will invoke a stub.
+  unsigned HasStubImplementation : 1;
+
   /// The location of the '!' or '?' for a failable initializer.
   SourceLoc FailabilityLoc;
 
@@ -5708,13 +5730,13 @@ class ConstructorDecl : public AbstractFunctionDecl {
 
   /// Whether the implementation of this method is a stub that traps at runtime.
   bool hasStubImplementation() const {
-    return ConstructorDeclBits.HasStubImplementation;
+    return HasStubImplementation;
   }
 
   /// Set whether the implementation of this method is a stub that
   /// traps at runtime.
   void setStubImplementation(bool stub) {
-    ConstructorDeclBits.HasStubImplementation = stub;
+    HasStubImplementation = stub;
   }
 
   ConstructorDecl *getOverriddenDecl() const { return OverriddenDecl; }

include/swift/AST/Types.h

@@ -221,19 +221,22 @@ enum class TypeTraitResult {
 
 /// Specifies which normally-unsafe type mismatches should be accepted when
 /// checking overrides.
-enum class OverrideMatchMode {
-  /// Only accept overrides that are properly covariant.
-  Strict,
+enum class TypeMatchFlags {
+  /// Allow properly-covariant overrides.
+  AllowOverride = 1 << 0,
   /// Allow a parameter with IUO type to be overridden by a parameter with non-
   /// optional type.
-  AllowNonOptionalForIUOParam,
+  AllowNonOptionalForIUOParam = 1 << 1,
   /// Allow any mismatches of Optional or ImplicitlyUnwrappedOptional at the
   /// top level of a type.
   ///
   /// This includes function parameters and result types as well as tuple
   /// elements, but excludes generic parameters.
-  AllowTopLevelOptionalMismatch
+  AllowTopLevelOptionalMismatch = 1 << 2,
+  /// Allow any ABI-compatible types to be considered matching.
+  AllowABICompatible = 1 << 3,
 };
+using TypeMatchOptions = OptionSet<TypeMatchFlags>;
 
 /// TypeBase - Base class for all types in Swift.
 class alignas(1 << TypeAlignInBits) TypeBase {
@@ -702,10 +705,9 @@ class alignas(1 << TypeAlignInBits) TypeBase {
   /// concrete types to form the argument type.
   bool isBindableTo(Type ty);
 
-  /// \brief Determines whether this type is permitted as a method override
-  /// of the \p other.
-  bool canOverride(Type other, OverrideMatchMode matchMode,
-                   LazyResolver *resolver);
+  /// \brief Determines whether this type is similar to \p other as defined by
+  /// \p matchOptions.
+  bool matches(Type other, TypeMatchOptions matchOptions, LazyResolver *resolver);
 
   /// \brief Determines whether this type has a retainable pointer
   /// representation, i.e. whether it is representable as a single,

include/swift/SIL/SILVTableVisitor.h

@@ -43,38 +43,33 @@ template <class T> class SILVTableVisitor {
   void maybeAddMethod(FuncDecl *fd) {
     assert(!fd->hasClangNode());
 
-    // Observing accessors and addressors don't get vtable entries.
-    if (fd->isObservingAccessor() ||
-        fd->getAddressorKind() != AddressorKind::NotAddressor)
-      return;
-
-    maybeAddEntry(SILDeclRef(fd, SILDeclRef::Kind::Func));
+    maybeAddEntry(SILDeclRef(fd, SILDeclRef::Kind::Func),
+                  fd->needsNewVTableEntry());
   }
 
   void maybeAddConstructor(ConstructorDecl *cd) {
     assert(!cd->hasClangNode());
 
-    SILDeclRef initRef(cd, SILDeclRef::Kind::Initializer);
-
-    // Stub constructors don't get a vtable entry unless they were synthesized
-    // to override a base class initializer.
-    if (cd->hasStubImplementation() &&
-        !initRef.getNextOverriddenVTableEntry())
-      return;
-
     // Required constructors (or overrides thereof) have their allocating entry
     // point in the vtable.
-    if (cd->isRequired())
-      maybeAddEntry(SILDeclRef(cd, SILDeclRef::Kind::Allocator));
+    if (cd->isRequired()) {
+      bool needsAllocatingEntry = cd->needsNewVTableEntry();
+      if (!needsAllocatingEntry)
+        if (auto *baseCD = cd->getOverriddenDecl())
+          needsAllocatingEntry = !baseCD->isRequired();
+      maybeAddEntry(SILDeclRef(cd, SILDeclRef::Kind::Allocator),
+                    needsAllocatingEntry);
+    }
 
     // All constructors have their initializing constructor in the
     // vtable, which can be used by a convenience initializer.
-    maybeAddEntry(SILDeclRef(cd, SILDeclRef::Kind::Initializer));
+    maybeAddEntry(SILDeclRef(cd, SILDeclRef::Kind::Initializer),
+                  cd->needsNewVTableEntry());
   }
 
-  void maybeAddEntry(SILDeclRef declRef) {
+  void maybeAddEntry(SILDeclRef declRef, bool needsNewEntry) {
     // Introduce a new entry if required.
-    if (Types.requiresNewVTableEntry(declRef))
+    if (needsNewEntry)
       asDerived().addMethod(declRef);
 
     // Update any existing entries that it overrides.

include/swift/SIL/TypeLowering.h

@@ -511,8 +511,6 @@ class TypeConverter {
 
   llvm::DenseMap<OverrideKey, SILConstantInfo> ConstantOverrideTypes;
 
-  llvm::DenseMap<SILDeclRef, bool> RequiresVTableEntry;
-
   llvm::DenseMap<AnyFunctionRef, CaptureInfo> LoweredCaptures;
 
   CanAnyFunctionType makeConstantInterfaceType(SILDeclRef constant);
@@ -665,11 +663,6 @@ class TypeConverter {
   /// `constant` must refer to a method.
   SILParameterInfo getConstantSelfParameter(SILDeclRef constant);
 
-  /// Return if this method introduces a new vtable entry. This will be true
-  /// if the method does not override any method of its base class, or if it
-  /// overrides a method but has a more general AST type.
-  bool requiresNewVTableEntry(SILDeclRef method);
-
   /// Return the most derived override which requires a new vtable entry.
   /// If the method does not override anything or no override is vtable
   /// dispatched, will return the least derived method.
@@ -852,8 +845,6 @@ class TypeConverter {
   CanAnyFunctionType getBridgedFunctionType(AbstractionPattern fnPattern,
                                             CanAnyFunctionType fnType,
                                             AnyFunctionType::ExtInfo extInfo);
-
-  bool requiresNewVTableEntryUncached(SILDeclRef method);
 };
 
 inline const TypeLowering &

include/swift/Serialization/ModuleFormat.h

@@ -54,7 +54,7 @@ const uint16_t VERSION_MAJOR = 0;
 /// in source control, you should also update the comment to briefly
 /// describe what change you made. The content of this comment isn't important;
 /// it just ensures a conflict if two people change the module format.
-const uint16_t VERSION_MINOR = 342; // Last change: keypath instruction
+const uint16_t VERSION_MINOR = 343; // Last change: new vtable entry flag
 
 using DeclID = PointerEmbeddedInt<unsigned, 31>;
 using DeclIDField = BCFixed<31>;
@@ -858,6 +858,7 @@ namespace decls_block {
     TypeIDField, // canonical interface type
     DeclIDField, // overridden decl
     AccessibilityKindField, // accessibility
+    BCFixed<1>,   // requires a new vtable slot
     BCArray<IdentifierIDField> // argument names
     // Trailed by its generic parameters, if any, followed by the parameter
     // patterns.
@@ -916,6 +917,7 @@ namespace decls_block {
     BCFixed<1>,   // name is compound?
     AddressorKindField, // addressor kind
     AccessibilityKindField, // accessibility
+    BCFixed<1>,   // requires a new vtable slot
     BCArray<IdentifierIDField> // name components
     // The record is trailed by:
     // - its _silgen_name, if any

lib/AST/Decl.cpp

@@ -4608,6 +4608,63 @@ AbstractFunctionDecl *AbstractFunctionDecl::getOverriddenDecl() const {
   return nullptr;
 }
 
+static bool requiresNewVTableEntry(const AbstractFunctionDecl *decl) {
+  assert(isa<FuncDecl>(decl) || isa<ConstructorDecl>(decl));
+
+  // Final members are always be called directly.
+  // Dynamic methods are always accessed by objc_msgSend().
+  if (decl->isFinal() || decl->isDynamic())
+    return false;
+
+  if (auto *fd = dyn_cast<FuncDecl>(decl)) {
+    switch (fd->getAccessorKind()) {
+    case AccessorKind::NotAccessor:
+    case AccessorKind::IsGetter:
+    case AccessorKind::IsSetter:
+      break;
+    case AccessorKind::IsAddressor:
+    case AccessorKind::IsMutableAddressor:
+      return false;
+    case AccessorKind::IsMaterializeForSet:
+      // Special case -- materializeForSet on dynamic storage is not
+      // itself dynamic, but should be treated as such for the
+      // purpose of constructing a vtable.
+      // FIXME: It should probably just be 'final'.
+      if (fd->getAccessorStorageDecl()->isDynamic())
+        return false;
+      break;
+    case AccessorKind::IsWillSet:
+    case AccessorKind::IsDidSet:
+      return false;
+    }
+  }
+
+  auto base = decl->getOverriddenDecl();
+  if (!base || base->hasClangNode())
+    return true;
+
+  // If the method overrides something, we only need a new entry if the
+  // override has a more general AST type. However an abstraction
+  // change is OK; we don't want to add a whole new vtable entry just
+  // because an @in parameter because @owned, or whatever.
+  auto baseInterfaceTy = base->getInterfaceType();
+  auto derivedInterfaceTy = decl->getInterfaceType();
+
+  auto selfInterfaceTy = decl->getDeclContext()->getDeclaredInterfaceType();
+
+  auto overrideInterfaceTy = selfInterfaceTy->adjustSuperclassMemberDeclType(
+      base, decl, baseInterfaceTy);
+
+  return !derivedInterfaceTy->matches(overrideInterfaceTy,
+                                      TypeMatchFlags::AllowABICompatible,
+                                      /*resolver*/nullptr);
+}
+
+void AbstractFunctionDecl::computeNeedsNewVTableEntry() {
+  setNeedsNewVTableEntry(requiresNewVTableEntry(this));
+}
+
+
 FuncDecl *FuncDecl::createImpl(ASTContext &Context,
                                SourceLoc StaticLoc,
                                StaticSpellingKind StaticSpelling,
@@ -4762,7 +4819,7 @@ ConstructorDecl::ConstructorDecl(DeclName Name, SourceLoc ConstructorLoc,
   setParameterLists(SelfDecl, BodyParams);
 
   ConstructorDeclBits.ComputedBodyInitKind = 0;
-  ConstructorDeclBits.HasStubImplementation = 0;
+  this->HasStubImplementation = 0;
   this->InitKind = static_cast<unsigned>(CtorInitializerKind::Designated);
   this->Failability = static_cast<unsigned>(Failability);
 }