More TBD work

include/swift/AST/Decl.h

@@ -4099,6 +4099,10 @@ class AbstractStorageDecl : public ValueDecl {
 
   FuncDecl *getAccessorFunction(AccessorKind accessor) const;
 
+  /// \brief Push all of the accessor functions associated with this VarDecl
+  /// onto `decls`.
+  void getAllAccessorFunctions(SmallVectorImpl<Decl *> &decls) const;
+
   /// \brief Turn this into a computed variable, providing a getter and setter.
   void makeComputed(SourceLoc LBraceLoc, FuncDecl *Get, FuncDecl *Set,
                     FuncDecl *MaterializeForSet, SourceLoc RBraceLoc);

include/swift/AST/ProtocolConformance.h

@@ -397,7 +397,14 @@ class NormalProtocolConformance : public ProtocolConformance,
   AbstractStorageDecl *getBehaviorDecl() const {
     return ContextAndInvalid.getPointer().dyn_cast<AbstractStorageDecl *>();
   }
-
+
+  bool isSerialized() const {
+    auto *nominal = getType()->getAnyNominal();
+    return nominal->hasFixedLayout() &&
+           getProtocol()->getEffectiveAccess() >= Accessibility::Public &&
+           nominal->getEffectiveAccess() >= Accessibility::Public;
+  }
+
   /// Retrieve the type witness and type decl (if one exists)
   /// for the given associated type.
   std::pair<Type, TypeDecl *>

include/swift/SIL/SILLinkage.h

@@ -252,6 +252,20 @@ inline SILLinkage effectiveLinkageForClassMember(SILLinkage linkage,
   return linkage;
 }
 
+// FIXME: This should not be necessary, but it looks like visibility rules for
+// extension members are slightly bogus, and so some protocol witness thunks
+// need to be public.
+//
+// We allow a 'public' member of an extension to witness a public
+// protocol requirement, even if the extended type is not public;
+// then SILGen gives the member private linkage, ignoring the more
+// visible accessibility it was given in the AST.
+inline bool
+fixmeWitnessHasLinkageThatNeedsToBePublic(SILLinkage witnessLinkage) {
+  return !hasPublicVisibility(witnessLinkage) &&
+         !hasSharedVisibility(witnessLinkage);
+}
+
 } // end swift namespace
 
 #endif

lib/AST/Decl.cpp

@@ -3430,6 +3430,28 @@ FuncDecl *AbstractStorageDecl::getAccessorFunction(AccessorKind kind) const {
   llvm_unreachable("bad accessor kind!");
 }
 
+void AbstractStorageDecl::getAllAccessorFunctions(
+    SmallVectorImpl<Decl *> &decls) const {
+  auto tryPush = [&](Decl *decl) {
+    if (decl)
+      decls.push_back(decl);
+  };
+
+  tryPush(getGetter());
+  tryPush(getSetter());
+  tryPush(getMaterializeForSetFunc());
+
+  if (hasObservers()) {
+    tryPush(getDidSetFunc());
+    tryPush(getWillSetFunc());
+  }
+
+  if (hasAddressors()) {
+    tryPush(getAddressor());
+    tryPush(getMutableAddressor());
+  }
+}
+
 void AbstractStorageDecl::configureGetSetRecord(GetSetRecord *getSetInfo,
                                                 FuncDecl *getter,
                                                 FuncDecl *setter,

lib/SILGen/SILGenType.cpp

@@ -315,13 +315,7 @@ class SILGenConformance : public SILGenWitnessTable<SILGenConformance> {
 
     // Serialize the witness table if we're serializing everything with
     // -sil-serialize-all, or if the conformance itself thinks it should be.
-    if (SGM.isMakeModuleFragile())
-      Serialized = IsSerialized;
-
-    auto *nominal = Conformance->getType()->getAnyNominal();
-    if (nominal->hasFixedLayout() &&
-        proto->getEffectiveAccess() >= Accessibility::Public &&
-        nominal->getEffectiveAccess() >= Accessibility::Public)
+    if (SGM.isMakeModuleFragile() || Conformance->isSerialized())
       Serialized = IsSerialized;
 
     // Not all protocols use witness tables; in this case we just skip
@@ -421,15 +415,7 @@ class SILGenConformance : public SILGenWitnessTable<SILGenConformance> {
     auto witnessLinkage = witnessRef.getLinkage(ForDefinition);
     auto witnessSerialized = Serialized;
     if (witnessSerialized &&
-        !hasPublicVisibility(witnessLinkage) &&
-        !hasSharedVisibility(witnessLinkage)) {
-      // FIXME: This should not happen, but it looks like visibility rules
-      // for extension members are slightly bogus.
-      //
-      // We allow a 'public' member of an extension to witness a public
-      // protocol requirement, even if the extended type is not public;
-      // then SILGen gives the member private linkage, ignoring the more
-      // visible accessibility it was given in the AST.
+        fixmeWitnessHasLinkageThatNeedsToBePublic(witnessLinkage)) {
       witnessLinkage = SILLinkage::Public;
       witnessSerialized = (SGM.isMakeModuleFragile()
                            ? IsSerialized

lib/TBDGen/TBDGen.cpp

@@ -62,22 +62,7 @@ class TBDGenVisitor : public ASTVisitor<TBDGenVisitor> {
       addSymbol(linkage.getName());
   }
 
-  void addConformances(DeclContext *DC) {
-    for (auto conformance : DC->getLocalConformances()) {
-      auto needsWTable = Lowering::TypeConverter::protocolRequiresWitnessTable(
-          conformance->getProtocol());
-      if (!needsWTable)
-        continue;
-
-      // Only normal conformances get symbols; the others get any public symbols
-      // from their parent normal conformance.
-      if (conformance->getKind() != ProtocolConformanceKind::Normal)
-        continue;
-
-      addSymbol(LinkEntity::forDirectProtocolWitnessTable(conformance));
-      addSymbol(LinkEntity::forProtocolWitnessTableAccessFunction(conformance));
-    }
-  }
+  void addConformances(DeclContext *DC);
 
 public:
   TBDGenVisitor(StringSet &symbols,
@@ -96,6 +81,8 @@ class TBDGenVisitor : public ASTVisitor<TBDGenVisitor> {
       addMembers(ED->getMembers());
     else if (auto NTD = dyn_cast<NominalTypeDecl>(D))
       addMembers(NTD->getMembers());
+    else if (auto VD = dyn_cast<VarDecl>(D))
+      VD->getAllAccessorFunctions(members);
 
     for (auto member : members) {
       ASTVisitor::visit(member);
@@ -160,10 +147,6 @@ void TBDGenVisitor::addSymbol(SILDeclRef declRef, bool checkSILOnly) {
   // currently need to refer to them by symbol for their own vtable.
   switch (declRef.getSubclassScope()) {
   case SubclassScope::External:
-    // Allocating constructors retain their normal linkage behavior.
-    if (declRef.kind == SILDeclRef::Kind::Allocator)
-      break;
-
     // Unlike the "truly" public things, private things have public symbols
     // unconditionally, even if they're theoretically SIL only.
     if (isPrivate) {
@@ -186,6 +169,81 @@ void TBDGenVisitor::addSymbol(SILDeclRef declRef, bool checkSILOnly) {
   addSymbol(declRef.mangle());
 }
 
+void TBDGenVisitor::addConformances(DeclContext *DC) {
+  for (auto conformance : DC->getLocalConformances()) {
+    auto protocol = conformance->getProtocol();
+    auto needsWTable =
+        Lowering::TypeConverter::protocolRequiresWitnessTable(protocol);
+    if (!needsWTable)
+      continue;
+
+    // Only normal conformances get symbols; the others get any public symbols
+    // from their parent normal conformance.
+    auto normalConformance = dyn_cast<NormalProtocolConformance>(conformance);
+    if (!normalConformance)
+      continue;
+
+    addSymbol(LinkEntity::forDirectProtocolWitnessTable(normalConformance));
+    addSymbol(
+        LinkEntity::forProtocolWitnessTableAccessFunction(normalConformance));
+
+    // FIXME: the logic around visibility in extensions is confusing, and
+    // sometimes witness thunks need to be manually made public.
+
+    auto conformanceIsSerialized = normalConformance->isSerialized();
+    auto addSymbolIfNecessary = [&](ValueDecl *valueReq,
+                                    SILLinkage witnessLinkage) {
+      if (conformanceIsSerialized &&
+          fixmeWitnessHasLinkageThatNeedsToBePublic(witnessLinkage)) {
+        Mangle::ASTMangler Mangler;
+        addSymbol(Mangler.mangleWitnessThunk(normalConformance, valueReq));
+      }
+    };
+    normalConformance->forEachValueWitness(nullptr, [&](ValueDecl *valueReq,
+                                                        Witness witness) {
+      if (isa<AbstractFunctionDecl>(valueReq)) {
+        auto witnessLinkage =
+            SILDeclRef(witness.getDecl()).getLinkage(ForDefinition);
+        addSymbolIfNecessary(valueReq, witnessLinkage);
+      } else if (auto VD = dyn_cast<AbstractStorageDecl>(valueReq)) {
+        // A var or subscript decl needs special handling in the special
+        // handling: the things that end up in the witness table are the
+        // accessors, but the compiler only talks about the actual storage decl
+        // in the conformance, so we have to manually walk over the members,
+        // having pulled out something that will have the right linkage.
+        auto witnessVD = cast<AbstractStorageDecl>(witness.getDecl());
+
+        SmallVector<Decl *, 4> members;
+        VD->getAllAccessorFunctions(members);
+
+        // Grab one of the accessors, and then use that to pull out which of the
+        // getter or setter will have the appropriate linkage.
+        FuncDecl *witnessWithRelevantLinkage;
+        switch (cast<FuncDecl>(members[0])->getAccessorKind()) {
+        case AccessorKind::NotAccessor:
+          llvm_unreachable("must be an accessor");
+        case AccessorKind::IsGetter:
+        case AccessorKind::IsAddressor:
+          witnessWithRelevantLinkage = witnessVD->getGetter();
+          break;
+        case AccessorKind::IsSetter:
+        case AccessorKind::IsWillSet:
+        case AccessorKind::IsDidSet:
+        case AccessorKind::IsMaterializeForSet:
+        case AccessorKind::IsMutableAddressor:
+          witnessWithRelevantLinkage = witnessVD->getSetter();
+          break;
+        }
+        auto witnessLinkage =
+            SILDeclRef(witnessWithRelevantLinkage).getLinkage(ForDefinition);
+        for (auto member : members) {
+          addSymbolIfNecessary(cast<ValueDecl>(member), witnessLinkage);
+        }
+      }
+    });
+  }
+}
+
 void TBDGenVisitor::visitValueDecl(ValueDecl *VD) {
   addSymbol(SILDeclRef(VD));
   visitMembers(VD);
@@ -225,9 +283,11 @@ void TBDGenVisitor::visitVarDecl(VarDecl *VD) {
     // like globals.
     if (!FileHasEntryPoint)
       addSymbol(SILDeclRef(VD, SILDeclRef::Kind::GlobalAccessor));
-  }
 
-  visitMembers(VD);
+    // In this case, the members of the VarDecl don't also appear as top-level
+    // decls, so we need to explicitly walk them.
+    visitMembers(VD);
+  }
 }
 
 void TBDGenVisitor::visitNominalTypeDecl(NominalTypeDecl *NTD) {
@@ -281,9 +341,11 @@ void TBDGenVisitor::visitClassDecl(ClassDecl *CD) {
     auto hasFieldOffset =
         !isGeneric && var && var->hasStorage() && !var->isStatic();
     if (hasFieldOffset) {
-      // Field are only direct if the class's internals are completely known.
-      auto isIndirect = !CD->hasFixedLayout();
-      addSymbol(LinkEntity::forFieldOffset(var, isIndirect));
+      // FIXME: a field only has one sort of offset, but it is moderately
+      // non-trivial to compute which one. Including both is less painful than
+      // missing the correct one (for now), so we do that.
+      addSymbol(LinkEntity::forFieldOffset(var, /*isIndirect=*/false));
+      addSymbol(LinkEntity::forFieldOffset(var, /*isIndirect=*/true));
     }
 
     // The non-allocating forms of the constructors and destructors.

test/TBD/class.swift

@@ -1,4 +1,4 @@
-// RUN: %target-swift-frontend -emit-ir -o- -parse-as-library -module-name test -validate-tbd-against-ir=all %s
+// RUN: %target-swift-frontend -emit-ir -o- -parse-as-library -module-name test -validate-tbd-against-ir=missing %s
 
 open class OpenNothing {}