SIL: Fix witness visibility hack to handle non-serialized declarations.

We have a hack to handle "public" declarations in extensions to internal protcols that are
intended as default implementations for a public protocol that the internal protocol refines.
This hack failed to trigger for synthesized declarations with shared linkage, such as
automatically generated `read` coroutines, causing a visibility assertion failure where we would
try to refer to the non-serializable synthesized declaration from the witness thunk we would
normally consider serialized. Fixes rdar://problem/55846638.

Author: jckarter

include/swift/SIL/SILDeclRef.h

@@ -21,6 +21,7 @@
 
 #include "swift/AST/ClangNode.h"
 #include "swift/AST/TypeAlignments.h"
+#include "swift/SIL/SILLinkage.h"
 #include "llvm/ADT/Hashing.h"
 #include "llvm/ADT/DenseMap.h"
 #include "llvm/ADT/PointerUnion.h"
@@ -42,7 +43,6 @@ namespace swift {
   class ASTContext;
   class ClassDecl;
   class SILFunctionType;
-  enum class SILLinkage : unsigned char;
   enum IsSerialized_t : unsigned char;
   enum class SubclassScope : unsigned char;
   class SILModule;
@@ -413,6 +413,22 @@ inline llvm::raw_ostream &operator<<(llvm::raw_ostream &OS, SILDeclRef C) {
   return OS;
 }
 
+// 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 access level it was given in the AST.
+inline bool
+fixmeWitnessHasLinkageThatNeedsToBePublic(SILDeclRef witness) {
+  auto witnessLinkage = witness.getLinkage(ForDefinition);
+  return !hasPublicVisibility(witnessLinkage)
+         && (!hasSharedVisibility(witnessLinkage)
+             || !witness.isSerialized());
+}
+
 } // end swift namespace
 
 namespace llvm {

include/swift/SIL/SILLinkage.h

@@ -265,20 +265,6 @@ 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 access level it was given in the AST.
-inline bool
-fixmeWitnessHasLinkageThatNeedsToBePublic(SILLinkage witnessLinkage) {
-  return !hasPublicVisibility(witnessLinkage) &&
-         !hasSharedVisibility(witnessLinkage);
-}
-
 } // end swift namespace
 
 #endif

lib/SIL/Verifier/SILVerifier.cpp

@@ -687,6 +687,8 @@ class SILVerifier : public SILVerifierBase<SILVerifier> {
     }
     llvm::dbgs() << "In function:\n";
     F.print(llvm::dbgs());
+    llvm::dbgs() << "In module:\n";
+    F.getModule().print(llvm::dbgs());
 
     // We abort by default because we want to always crash in
     // the debugger.

lib/SILGen/SILGenType.cpp

@@ -567,7 +567,7 @@ class SILGenConformance : public SILGenWitnessTable<SILGenConformance> {
     auto witnessLinkage = witnessRef.getLinkage(ForDefinition);
     auto witnessSerialized = Serialized;
     if (witnessSerialized &&
-        fixmeWitnessHasLinkageThatNeedsToBePublic(witnessLinkage)) {
+        fixmeWitnessHasLinkageThatNeedsToBePublic(witnessRef)) {
       witnessLinkage = SILLinkage::Public;
       witnessSerialized = IsNotSerialized;
     } else {

lib/TBDGen/TBDGen.cpp

@@ -473,10 +473,11 @@ void TBDGenVisitor::addConformances(DeclContext *DC) {
         rootConformance);
     auto addSymbolIfNecessary = [&](ValueDecl *requirementDecl,
                                     ValueDecl *witnessDecl) {
-      auto witnessLinkage = SILDeclRef(witnessDecl).getLinkage(ForDefinition);
+      auto witnessRef = SILDeclRef(witnessDecl);
+      auto witnessLinkage = witnessRef.getLinkage(ForDefinition);
       if (conformanceIsFixed &&
           (isa<SelfProtocolConformance>(rootConformance) ||
-           fixmeWitnessHasLinkageThatNeedsToBePublic(witnessLinkage))) {
+           fixmeWitnessHasLinkageThatNeedsToBePublic(witnessRef))) {
         Mangle::ASTMangler Mangler;
         addSymbol(
             Mangler.mangleWitnessThunk(rootConformance, requirementDecl));

test/SILGen/internal_protocol_refines_public_protocol_with_public_default_implementation.swift

@@ -0,0 +1,15 @@
+// RUN: %target-swift-emit-silgen -verify %s
+
+public protocol A {
+    @_borrowed
+    subscript() -> Int { get }
+}
+
+protocol B: A { }
+
+extension B {
+    public subscript() -> Int { return 0 }
+}
+
+public struct S: B {
+}