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

This lets us serialize that decision, which means we can conceivably
/change/ the decision in later versions of the compiler without
breaking existing code. More immediately, it's groundwork that will
eventually allow us to drop decls from the AST without affecting
vtable layout.

This isn't actually a great answer; what we really want is for SIL
vtables to be serialized consistently and treated as the point of
truth. But that would be more change than we're comfortable taking in
the Swift 4 timeframe.

First part of rdar://problem/31878396.

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/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);
 }

lib/ClangImporter/ImporterImpl.h

@@ -1128,6 +1128,8 @@ class LLVM_LIBRARY_VISIBILITY ClangImporter::Implementation
       ASD->setSetterAccessibility(access);
     // All imported decls are constructed fully validated.
     D->setValidationStarted();
+    if (auto AFD = dyn_cast<AbstractFunctionDecl>(static_cast<Decl *>(D)))
+      AFD->setNeedsNewVTableEntry(false);
     return D;
   }