Merge pull request #8387 from slavapestov/protocol-witness-thunk-linkage

Protocol witness thunks don't need public linkage

Author: slavapestov

lib/SIL/SILVerifier.cpp

@@ -1131,8 +1131,25 @@ class SILVerifier : public SILVerifierBase<SILVerifier> {
                                     "result of function_ref");
     require(!fnType->getExtInfo().hasContext(),
             "function_ref should have a context-free function result");
+
+    // Note: in SingleFunction mode, we relax some of these checks because
+    // we may not have linked everything yet.
+
+    SILFunction *RefF = FRI->getReferencedFunction();
+
+    // A direct reference to a shared_external declaration is an error; we
+    // should have deserialized a body.
+    if (RefF->isExternalDeclaration()) {
+      require(SingleFunction ||
+              !hasSharedVisibility(RefF->getLinkage()) ||
+              RefF->hasForeignBody(),
+              "external declarations of SILFunctions with shared visibility is "
+              "not allowed");
+    }
+
+    // A direct reference to a non-public or shared but not fragile function
+    // from a fragile function is an error.
     if (F.isSerialized()) {
-      SILFunction *RefF = FRI->getReferencedFunction();
       require((SingleFunction && RefF->isExternalDeclaration()) ||
               RefF->hasValidLinkageForFragileRef(),
               "function_ref inside fragile function cannot "
@@ -3912,9 +3929,6 @@ class SILVerifier : public SILVerifierBase<SILVerifier> {
 
       assert(F->isAvailableExternally() &&
              "external declaration of internal SILFunction not allowed");
-      assert(!hasSharedVisibility(F->getLinkage()) &&
-             "external declarations of SILFunctions with shared visibility is not "
-             "allowed");
       // If F is an external declaration, there is nothing further to do,
       // return.
       return;
@@ -4037,15 +4051,20 @@ void SILWitnessTable::verify(const SILModule &M) const {
 
   auto *protocol = getConformance()->getProtocol();
 
-  // Currently all witness tables have public conformances, thus witness tables
-  // should not reference SILFunctions without public/public_external linkage.
-  // FIXME: Once we support private conformances, update this.
   for (const Entry &E : getEntries())
     if (E.getKind() == SILWitnessTable::WitnessKind::Method) {
       SILFunction *F = E.getMethodWitness().Witness;
       if (F) {
-        assert(!isLessVisibleThan(F->getLinkage(), getLinkage()) &&
-               "Witness tables should not reference less visible functions.");
+        // If a SILWitnessTable is going to be serialized, it must only
+        // reference public or serializable functions.
+        if (isSerialized()) {
+          assert((!isLessVisibleThan(F->getLinkage(), getLinkage()) ||
+                  (F->isSerialized() &&
+                   hasSharedVisibility(F->getLinkage()))) &&
+                 "Fragile witness tables should not reference "
+                 "less visible functions.");
+        }
+
         assert(F->getLoweredFunctionType()->getRepresentation() ==
                SILFunctionTypeRepresentation::WitnessMethod &&
                "Witnesses must have witness_method representation.");
@@ -4069,9 +4088,12 @@ void SILDefaultWitnessTable::verify(const SILModule &M) const {
       continue;
 
     SILFunction *F = E.getWitness();
+    // FIXME
+    #if 0
     assert(!isLessVisibleThan(F->getLinkage(), getLinkage()) &&
            "Default witness tables should not reference "
            "less visible functions.");
+    #endif
     assert(F->getLoweredFunctionType()->getRepresentation() ==
            SILFunctionTypeRepresentation::WitnessMethod &&
            "Default witnesses must have witness_method representation.");

lib/SILGen/SILGen.h

@@ -317,6 +317,7 @@ class LLVM_LIBRARY_VISIBILITY SILGenModule : public ASTVisitor<SILGenModule> {
   /// Emit a protocol witness entry point.
   SILFunction *emitProtocolWitness(ProtocolConformance *conformance,
                                    SILLinkage linkage,
+                                   IsSerialized_t isSerialized,
                                    SILDeclRef requirement,
                                    SILDeclRef witnessRef,
                                    IsFreeFunctionWitness_t isFree,

lib/SILGen/SILGenType.cpp

@@ -312,16 +312,34 @@ class SILGenConformance : public SILGenWitnessTable<SILGenConformance> {
   NormalProtocolConformance *Conformance;
   std::vector<SILWitnessTable::Entry> Entries;
   SILLinkage Linkage;
+  IsSerialized_t Serialized;
 
   SILGenConformance(SILGenModule &SGM, NormalProtocolConformance *C)
     // We only need to emit witness tables for base NormalProtocolConformances.
     : SGM(SGM), Conformance(C->getRootNormalConformance()),
       Linkage(getLinkageForProtocolConformance(Conformance,
                                                ForDefinition))
   {
-    // Not all protocols use witness tables.
-    if (!Lowering::TypeConverter::protocolRequiresWitnessTable(
-        Conformance->getProtocol()))
+    auto *proto = Conformance->getProtocol();
+
+    Serialized = IsNotSerialized;
+
+    // Serialize the witness table if we're serializing everything with
+    // -sil-serialize-all.
+    if (SGM.makeModuleFragile)
+      Serialized = IsSerialized;
+
+    // Serialize the witness table if type has a fixed layout in all
+    // resilience domains, and the conformance is externally visible.
+    auto nominal = Conformance->getInterfaceType()->getAnyNominal();
+    if (nominal->hasFixedLayout() &&
+        proto->getEffectiveAccess() >= Accessibility::Public &&
+        nominal->getEffectiveAccess() >= Accessibility::Public)
+      Serialized = IsSerialized;
+
+    // Not all protocols use witness tables; in this case we just skip
+    // all of emit() below completely.
+    if (!Lowering::TypeConverter::protocolRequiresWitnessTable(proto))
       Conformance = nullptr;
   }
 
@@ -333,19 +351,6 @@ class SILGenConformance : public SILGenWitnessTable<SILGenConformance> {
     auto *proto = Conformance->getProtocol();
     visitProtocolDecl(proto);
 
-    // Serialize the witness table in two cases:
-    // 1) We're serializing everything
-    // 2) The type has a fixed layout in all resilience domains, and the
-    //    conformance is externally visible
-    IsSerialized_t isSerialized = IsNotSerialized;
-    if (SGM.makeModuleFragile)
-      isSerialized = IsSerialized;
-    if (auto nominal = Conformance->getInterfaceType()->getAnyNominal())
-      if (nominal->hasFixedLayout() &&
-          proto->getEffectiveAccess() >= Accessibility::Public &&
-          nominal->getEffectiveAccess() >= Accessibility::Public)
-        isSerialized = IsSerialized;
-
     // Check if we already have a declaration or definition for this witness
     // table.
     if (auto *wt = SGM.M.lookUpWitnessTable(Conformance, false)) {
@@ -358,7 +363,7 @@ class SILGenConformance : public SILGenWitnessTable<SILGenConformance> {
 
       // If we have a declaration, convert the witness table to a definition.
       if (wt->isDeclaration()) {
-        wt->convertToDefinition(Entries, isSerialized);
+        wt->convertToDefinition(Entries, Serialized);
 
         // Since we had a declaration before, its linkage should be external,
         // ensure that we have a compatible linkage for sanity. *NOTE* we are ok
@@ -375,7 +380,7 @@ class SILGenConformance : public SILGenWitnessTable<SILGenConformance> {
     }
 
     // Otherwise if we have no witness table yet, create it.
-    return SILWitnessTable::create(SGM.M, Linkage, isSerialized,
+    return SILWitnessTable::create(SGM.M, Linkage, Serialized,
                                    Conformance, Entries);
   }
 
@@ -427,9 +432,33 @@ class SILGenConformance : public SILGenWitnessTable<SILGenConformance> {
       return;
     }
 
+    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.
+      witnessLinkage = SILLinkage::Public;
+      witnessSerialized = (SGM.makeModuleFragile
+                           ? IsSerialized
+                           : IsNotSerialized);
+    } else {
+      // This is the "real" rule; the above case should go away once we
+      // figure out what's going on.
+      witnessLinkage = (witnessSerialized
+                        ? SILLinkage::Shared
+                        : SILLinkage::Private);
+    }
+
     SILFunction *witnessFn =
-      SGM.emitProtocolWitness(Conformance, Linkage, requirementRef, witnessRef,
-                              isFree, witness);
+      SGM.emitProtocolWitness(Conformance, witnessLinkage, witnessSerialized,
+                              requirementRef, witnessRef, isFree, witness);
     Entries.push_back(
                     SILWitnessTable::MethodWitness{requirementRef, witnessFn});
   }
@@ -540,6 +569,7 @@ static bool maybeOpenCodeProtocolWitness(SILGenFunction &gen,
 SILFunction *
 SILGenModule::emitProtocolWitness(ProtocolConformance *conformance,
                                   SILLinkage linkage,
+                                  IsSerialized_t isSerialized,
                                   SILDeclRef requirement,
                                   SILDeclRef witnessRef,
                                   IsFreeFunctionWitness_t isFree,
@@ -637,14 +667,6 @@ SILGenModule::emitProtocolWitness(ProtocolConformance *conformance,
   if (witnessRef.isAlwaysInline())
     InlineStrategy = AlwaysInline;
 
-  IsSerialized_t isSerialized = IsNotSerialized;
-  if (makeModuleFragile)
-    isSerialized = IsSerialized;
-  if (witnessRef.isSerialized() &&
-      (hasSharedVisibility(linkage) ||
-       hasPublicVisibility(linkage)))
-    isSerialized = IsSerialized;
-
   auto *f = M.createFunction(
       linkage, nameBuffer, witnessSILFnType,
       genericEnv, SILLocation(witnessRef.getDecl()),
@@ -742,7 +764,9 @@ class SILGenDefaultWitnessTable
                  SILDeclRef witnessRef,
                  IsFreeFunctionWitness_t isFree,
                  Witness witness) {
-    SILFunction *witnessFn = SGM.emitProtocolWitness(nullptr, Linkage,
+    SILFunction *witnessFn = SGM.emitProtocolWitness(nullptr,
+                                                     SILLinkage::Private,
+                                                     IsNotSerialized,
                                                      requirementRef, witnessRef,
                                                      isFree, witness);
     auto entry = SILDefaultWitnessTable::Entry(requirementRef, witnessFn);

test/IRGen/dependent_reabstraction.swift

@@ -8,7 +8,7 @@ protocol A {
 }
 
 struct X<Y> : A {
-  // CHECK-LABEL: define hidden swiftcc void @_T023dependent_reabstraction1XVyxGAA1AAAlAaEP1by1BQzFTW(%swift.type** noalias nocapture dereferenceable({{.*}}), %T23dependent_reabstraction1XV* noalias nocapture swiftself, %swift.type* %Self, i8** %SelfWitnessTable)
+  // CHECK-LABEL: define internal swiftcc void @_T023dependent_reabstraction1XVyxGAA1AAAlAaEP1by1BQzFTW(%swift.type** noalias nocapture dereferenceable({{.*}}), %T23dependent_reabstraction1XV* noalias nocapture swiftself, %swift.type* %Self, i8** %SelfWitnessTable)
   func b(_ b: X.Type) {
     let x: Any = b
     markUsed(b as X.Type)

test/IRGen/metadata_dominance.swift

@@ -79,6 +79,6 @@ func testMakeFoo(_ p: P) -> Foo.Type {
 // The protocol witness for metadata_dominance.P.makeFoo () -> metadata_dominance.Foo in
 // conformance metadata_dominance.Foo : metadata_dominance.P should not use the Self type
 // as the type of the object to be created. It should dynamically obtain the type.
-// CHECK-OPT-LABEL: define hidden swiftcc %T18metadata_dominance3FooC* @_T018metadata_dominance3FooCAA1PA2aDP04makeC0ACyFTW
+// CHECK-OPT-LABEL: define internal swiftcc %T18metadata_dominance3FooC* @_T018metadata_dominance3FooCAA1PA2aDP04makeC0ACyFTW
 // CHECK-OPT-NOT: tail call noalias %swift.refcounted* @swift_rt_swift_allocObject(%swift.type* %Self
 

test/IRGen/objc_generic_protocol_conformance.swift

@@ -9,5 +9,5 @@ protocol P {
 
 extension Foo: P {}
 
-// SIL-LABEL: sil hidden [transparent] [thunk] @_T0So3FooCyxG33objc_generic_protocol_conformance1PADs9AnyObjectRzlAdEP3fooyyFTW {{.*}} @pseudogeneric
-// IR-LABEL: define hidden swiftcc void @_T0So3FooCyxG33objc_generic_protocol_conformance1PADs9AnyObjectRzlAdEP3fooyyFTW(%TSo3FooC** noalias nocapture swiftself dereferenceable({{4|8}}), %swift.type*{{( %Self)?}}, i8**{{( %SelfWitnessTable)?}})
+// SIL-LABEL: sil private [transparent] [thunk] @_T0So3FooCyxG33objc_generic_protocol_conformance1PADs9AnyObjectRzlAdEP3fooyyFTW {{.*}} @pseudogeneric
+// IR-LABEL: define internal swiftcc void @_T0So3FooCyxG33objc_generic_protocol_conformance1PADs9AnyObjectRzlAdEP3fooyyFTW(%TSo3FooC** noalias nocapture swiftself dereferenceable({{4|8}}), %swift.type*{{( %Self)?}}, i8**{{( %SelfWitnessTable)?}})

test/SILGen/cf.swift

@@ -65,9 +65,9 @@ protocol Impedance {
 
 extension CCImpedance: Impedance {}
 
-// CHECK-LABEL: sil hidden [transparent] [thunk] @_T0SC11CCImpedanceV2cf9ImpedanceA2cDP4real9ComponentQzfgTW
+// CHECK-LABEL: sil private [transparent] [thunk] @_T0SC11CCImpedanceV2cf9ImpedanceA2cDP4real9ComponentQzfgTW
 // CHECK-LABEL: sil shared [transparent] [serializable] @_T0SC11CCImpedanceV4realSdfg
-// CHECK-LABEL: sil hidden [transparent] [thunk] @_T0SC11CCImpedanceV2cf9ImpedanceA2cDP4imag9ComponentQzfgTW
+// CHECK-LABEL: sil private [transparent] [thunk] @_T0SC11CCImpedanceV2cf9ImpedanceA2cDP4imag9ComponentQzfgTW
 // CHECK-LABEL: sil shared [transparent] [serializable] @_T0SC11CCImpedanceV4imagSdfg
 
 class MyMagnetism : CCMagnetismModel {

test/SILGen/dependent_member_lowering.swift

@@ -9,13 +9,13 @@ struct Foo<T>: P {
   typealias A = T.Type
 
   func f(_ t: T.Type) {}
-  // CHECK-LABEL: sil hidden [transparent] [thunk] @_T025dependent_member_lowering3FooVyxGAA1PAAlAaEP1fy1AQzFTW : $@convention(witness_method) <τ_0_0> (@in @thick τ_0_0.Type, @in_guaranteed Foo<τ_0_0>) -> ()
+  // CHECK-LABEL: sil private [transparent] [thunk] @_T025dependent_member_lowering3FooVyxGAA1PAAlAaEP1fy1AQzFTW : $@convention(witness_method) <τ_0_0> (@in @thick τ_0_0.Type, @in_guaranteed Foo<τ_0_0>) -> ()
   // CHECK:       bb0(%0 : $*@thick τ_0_0.Type, %1 : $*Foo<τ_0_0>):
 }
 struct Bar<T>: P {
   typealias A = (Int) -> T
 
   func f(_ t: @escaping (Int) -> T) {}
-  // CHECK-LABEL: sil hidden [transparent] [thunk] @_T025dependent_member_lowering3BarVyxGAA1PAAlAaEP1fy1AQzFTW : $@convention(witness_method) <τ_0_0> (@in @callee_owned (@in Int) -> @out τ_0_0, @in_guaranteed Bar<τ_0_0>) -> ()
+  // CHECK-LABEL: sil private [transparent] [thunk] @_T025dependent_member_lowering3BarVyxGAA1PAAlAaEP1fy1AQzFTW : $@convention(witness_method) <τ_0_0> (@in @callee_owned (@in Int) -> @out τ_0_0, @in_guaranteed Bar<τ_0_0>) -> ()
   // CHECK:       bb0(%0 : $*@callee_owned (@in Int) -> @out τ_0_0, %1 : $*Bar<τ_0_0>):
 }