[TBDGen] Protocol witnesses thunks for members of public superprotocols are public.

Specifically, public superprotocols of non-public protocols have some
weird handling in SILGen, so we reproduce this.

Fixes rdar://problem/32254485 .

Author: huonw

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/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,
@@ -184,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);