[IRGen] Handle ProtocolInfo for protocols whose members aren't used (swiftlang#18692)

jrose-apple · web-flow · commit 84f471b03123 · 2018-08-14T11:10:02.000-07:00
Certain uses of protocols only formally need the requirement
signature, not any of the method requirements. This results in IRGen
seeing a protocol where none of the members have been validated except
the associated types. Account for this by allowing ProtocolInfo to
only contain the layout for the base protocols and associated types,
if requested.

Note that this relies on the layout of a witness table always putting
the "requirement signature part" at the front, or at least at offsets
that aren't affected by function requirements.

rdar://problem/43260117
diff --git a/include/swift/SIL/SILWitnessVisitor.h b/include/swift/SIL/SILWitnessVisitor.h
@@ -122,11 +122,20 @@ template <class T> class SILWitnessVisitor : public ASTVisitor<T> {
     // Add the associated types if we haven't yet.
     addAssociatedTypes();
 
+    if (asDerived().shouldVisitRequirementSignatureOnly())
+      return;
+
     // Visit the witnesses for the direct members of a protocol.
     for (Decl *member : protocol->getMembers())
       ASTVisitor<T>::visit(member);
   }
 
+  /// If true, only the base protocols and associated types will be visited.
+  /// The base implementation returns false.
+  bool shouldVisitRequirementSignatureOnly() const {
+    return false;
+  }
+
   /// Fallback for unexpected protocol requirements.
   void visitDecl(Decl *d) {
     llvm_unreachable("unhandled protocol requirement");
diff --git a/lib/IRGen/Fulfillment.cpp b/lib/IRGen/Fulfillment.cpp
@@ -209,7 +209,8 @@ bool FulfillmentMap::searchWitnessTable(
 
   bool hadFulfillment = false;
 
-  auto &pi = IGM.getProtocolInfo(protocol);
+  auto &pi = IGM.getProtocolInfo(protocol,
+                                 ProtocolInfoKind::RequirementSignature);
 
   for (auto &entry : pi.getWitnessEntries()) {
     if (!entry.isBase()) continue;
diff --git a/lib/IRGen/GenArchetype.cpp b/lib/IRGen/GenArchetype.cpp
@@ -182,7 +182,8 @@ llvm::Value *irgen::emitArchetypeWitnessTableRef(IRGenFunction &IGF,
            "non-opened archetype lacking generic environment?");
     SmallVector<ProtocolEntry, 4> entries;
     for (auto p : archetype->getConformsTo()) {
-      const ProtocolInfo &impl = IGF.IGM.getProtocolInfo(p);
+      const ProtocolInfo &impl =
+          IGF.IGM.getProtocolInfo(p, ProtocolInfoKind::RequirementSignature);
       entries.push_back(ProtocolEntry(p, impl));
     }
 
@@ -227,7 +228,9 @@ llvm::Value *irgen::emitArchetypeWitnessTableRef(IRGenFunction &IGF,
     CanType depType = CanType(entry.first);
     ProtocolDecl *requirement = entry.second;
 
-    auto &lastPI = IGF.IGM.getProtocolInfo(lastProtocol);
+    const ProtocolInfo &lastPI =
+        IGF.IGM.getProtocolInfo(lastProtocol,
+                                ProtocolInfoKind::RequirementSignature);
 
     // If it's a type parameter, it's self, and this is a base protocol
     // requirement.
diff --git a/lib/IRGen/GenDecl.cpp b/lib/IRGen/GenDecl.cpp
@@ -920,7 +920,8 @@ llvm::Constant *IRGenModule::getAddrOfAssociatedTypeGenericParamRef(
     
     // Find the offset of the associated type entry in witness tables of this
     // protocol.
-    auto &protoInfo = getProtocolInfo(assocType->getProtocol());
+    auto &protoInfo = getProtocolInfo(assocType->getProtocol(),
+                                      ProtocolInfoKind::RequirementSignature);
     auto index = protoInfo.getAssociatedTypeIndex(AssociatedType(assocType))
       .getValue();
     
diff --git a/lib/IRGen/GenKeyPath.cpp b/lib/IRGen/GenKeyPath.cpp
@@ -974,7 +974,8 @@ emitKeyPathComponent(IRGenModule &IGM,
           idValue = llvm::ConstantInt::get(IGM.SizeTy, offset.getValue());
           idResolved = true;
         } else if (auto methodProto = dyn_cast<ProtocolDecl>(dc)) {
-          auto &protoInfo = IGM.getProtocolInfo(methodProto);
+          auto &protoInfo = IGM.getProtocolInfo(methodProto,
+                                                ProtocolInfoKind::Full);
           auto index = protoInfo.getFunctionIndex(
                                cast<AbstractFunctionDecl>(declRef.getDecl()));
           idValue = llvm::ConstantInt::get(IGM.SizeTy, -index.getValue());
diff --git a/lib/IRGen/GenMeta.cpp b/lib/IRGen/GenMeta.cpp
@@ -654,7 +654,7 @@ namespace {
     }
 
     void addRequirements() {
-      auto &pi = IGM.getProtocolInfo(Proto);
+      auto &pi = IGM.getProtocolInfo(Proto, ProtocolInfoKind::Full);
 
       B.fillPlaceholderWithInt(*NumRequirements, IGM.Int32Ty,
                                pi.getNumWitnesses());
@@ -692,7 +692,8 @@ namespace {
     void addAssociatedTypeNames() {
       std::string AssociatedTypeNames;
 
-      auto &pi = IGM.getProtocolInfo(Proto);
+      auto &pi = IGM.getProtocolInfo(Proto,
+                                     ProtocolInfoKind::RequirementSignature);
       for (auto &entry : pi.getWitnessEntries()) {
         // Add the associated type name to the list.
         if (entry.isAssociatedType()) {
diff --git a/lib/IRGen/GenProto.cpp b/lib/IRGen/GenProto.cpp
@@ -749,8 +749,24 @@ namespace {
   /// A class which lays out a witness table in the abstract.
   class WitnessTableLayout : public SILWitnessVisitor<WitnessTableLayout> {
     SmallVector<WitnessTableEntry, 16> Entries;
+    bool requirementSignatureOnly;
 
   public:
+    explicit WitnessTableLayout(ProtocolInfoKind resultKind) {
+      switch (resultKind) {
+      case ProtocolInfoKind::RequirementSignature:
+        requirementSignatureOnly = true;
+        break;
+      case ProtocolInfoKind::Full:
+        requirementSignatureOnly = false;
+        break;
+      }
+    }
+
+    bool shouldVisitRequirementSignatureOnly() {
+      return requirementSignatureOnly;
+    }
+
     void addProtocolConformanceDescriptor() { }
 
     /// The next witness is an out-of-line base protocol.
@@ -896,9 +912,11 @@ namespace {
 
         // Otherwise, if there isn't a better path through this base,
         // don't accumulate anything in the path.
-        } else if (!findBetterPath(base, IGM.getProtocolInfo(base),
-                                   lengthToBase)) {
-          continue;
+        } else {
+          const ProtocolInfo &baseInfo =
+              IGM.getProtocolInfo(base, ProtocolInfoKind::RequirementSignature);
+          if (!findBetterPath(base, baseInfo, lengthToBase))
+            continue;
         }
 
         // Okay, we've found a better path, and ReversePath contains a
@@ -1210,7 +1228,8 @@ llvm::Value *uniqueForeignWitnessTableRef(IRGenFunction &IGF,
                                                          Conformance.getDeclContext())),
           SILEntries(SILWT->getEntries()),
           SILConditionalConformances(SILWT->getConditionalConformances()),
-          PI(IGM.getProtocolInfo(SILWT->getConformance()->getProtocol())) {
+          PI(IGM.getProtocolInfo(SILWT->getConformance()->getProtocol(),
+                                 ProtocolInfoKind::Full)) {
       // If the conformance is resilient, we require runtime instantiation.
       if (isResilientConformance(&Conformance)) {
         RequiresSpecialization = true;
@@ -2079,38 +2098,53 @@ void IRGenModule::ensureRelativeSymbolCollocation(SILWitnessTable &wt) {
 }
 
 /// Do a memoized witness-table layout for a protocol.
-const ProtocolInfo &IRGenModule::getProtocolInfo(ProtocolDecl *protocol) {
-  return Types.getProtocolInfo(protocol);
+const ProtocolInfo &IRGenModule::getProtocolInfo(ProtocolDecl *protocol,
+                                                 ProtocolInfoKind kind) {
+  return Types.getProtocolInfo(protocol, kind);
 }
 
 /// Do a memoized witness-table layout for a protocol.
-const ProtocolInfo &TypeConverter::getProtocolInfo(ProtocolDecl *protocol) {
+const ProtocolInfo &TypeConverter::getProtocolInfo(ProtocolDecl *protocol,
+                                                   ProtocolInfoKind kind) {
   // Check whether we've already translated this protocol.
   auto it = Protocols.find(protocol);
-  if (it != Protocols.end()) return *it->second;
+  if (it != Protocols.end() && it->second->getKind() >= kind)
+    return *it->second;
 
   // If not, lay out the protocol's witness table, if it needs one.
-  WitnessTableLayout layout;
+  WitnessTableLayout layout(kind);
   if (Lowering::TypeConverter::protocolRequiresWitnessTable(protocol))
     layout.visitProtocolDecl(protocol);
 
   // Create a ProtocolInfo object from the layout.
-  ProtocolInfo *info = ProtocolInfo::create(layout.getEntries());
-  info->NextConverted = FirstProtocol;
+  ProtocolInfo *info = ProtocolInfo::create(layout.getEntries(), kind);
+  info->NextConverted.setPointer(FirstProtocol);
   FirstProtocol = info;
 
+  // Verify that we haven't generated an incompatible layout.
+  if (it != Protocols.end()) {
+    ArrayRef<WitnessTableEntry> originalEntries =
+        it->second->getWitnessEntries();
+    ArrayRef<WitnessTableEntry> newEntries = info->getWitnessEntries();
+    assert(newEntries.size() >= originalEntries.size());
+    assert(newEntries.take_front(originalEntries.size()) == originalEntries);
+    (void)originalEntries;
+    (void)newEntries;
+  }
+
   // Memoize.
-  Protocols.insert(std::make_pair(protocol, info));
+  Protocols[protocol] = info;
 
   // Done.
   return *info;
 }
 
 /// Allocate a new ProtocolInfo.
-ProtocolInfo *ProtocolInfo::create(ArrayRef<WitnessTableEntry> table) {
+ProtocolInfo *ProtocolInfo::create(ArrayRef<WitnessTableEntry> table,
+                                   ProtocolInfoKind kind) {
   size_t bufferSize = totalSizeToAlloc<WitnessTableEntry>(table.size());
   void *buffer = ::operator new(bufferSize);
-  return new(buffer) ProtocolInfo(table);
+  return new(buffer) ProtocolInfo(table, kind);
 }
 
 // Provide a unique home for the ConformanceInfo vtable.
@@ -2486,7 +2520,8 @@ MetadataResponse MetadataPath::followComponent(IRGenFunction &IGF,
   case Component::Kind::OutOfLineBaseProtocol: {
     auto conformance = sourceKey.Kind.getProtocolConformance();
     auto protocol = conformance.getRequirement();
-    auto &pi = IGF.IGM.getProtocolInfo(protocol);
+    auto &pi = IGF.IGM.getProtocolInfo(protocol,
+                                       ProtocolInfoKind::RequirementSignature);
 
     auto &entry = pi.getWitnessEntries()[component.getPrimaryIndex()];
     assert(entry.isOutOfLineBase());
@@ -2522,7 +2557,8 @@ MetadataResponse MetadataPath::followComponent(IRGenFunction &IGF,
     auto sourceType = sourceKey.Type;
     auto sourceConformance = sourceKey.Kind.getProtocolConformance();
     auto sourceProtocol = sourceConformance.getRequirement();
-    auto &pi = IGF.IGM.getProtocolInfo(sourceProtocol);
+    auto &pi = IGF.IGM.getProtocolInfo(sourceProtocol,
+                                       ProtocolInfoKind::RequirementSignature);
 
     auto &entry = pi.getWitnessEntries()[component.getPrimaryIndex()];
     assert(entry.isAssociatedConformance());
@@ -3243,7 +3279,7 @@ irgen::emitWitnessMethodValue(IRGenFunction &IGF,
   assert(!IGF.IGM.isResilient(proto, ResilienceExpansion::Maximal));
 
   // Find the witness we're interested in.
-  auto &fnProtoInfo = IGF.IGM.getProtocolInfo(proto);
+  auto &fnProtoInfo = IGF.IGM.getProtocolInfo(proto, ProtocolInfoKind::Full);
   auto index = fnProtoInfo.getFunctionIndex(fn);
   llvm::Value *witnessFnPtr =
     emitInvariantLoadOfOpaqueWitness(IGF, wtable,
@@ -3292,7 +3328,8 @@ irgen::emitAssociatedTypeMetadataRef(IRGenFunction &IGF,
                                      llvm::Value *wtable,
                                      AssociatedType associatedType,
                                      DynamicMetadataRequest request) {
-  auto &pi = IGF.IGM.getProtocolInfo(associatedType.getSourceProtocol());
+  auto &pi = IGF.IGM.getProtocolInfo(associatedType.getSourceProtocol(),
+                                     ProtocolInfoKind::RequirementSignature);
   auto index = pi.getAssociatedTypeIndex(associatedType);
   llvm::Value *witness = emitInvariantLoadOfOpaqueWitness(IGF, wtable,
                                             index.forProtocolWitnessTable());
diff --git a/lib/IRGen/GenType.cpp b/lib/IRGen/GenType.cpp
@@ -1058,12 +1058,10 @@ TypeConverter::createImmovable(llvm::Type *type, Size size, Alignment align) {
 }
 
 static TypeInfo *invalidTypeInfo() { return (TypeInfo*) 1; }
-static ProtocolInfo *invalidProtocolInfo() { return (ProtocolInfo*) 1; }
 
 TypeConverter::TypeConverter(IRGenModule &IGM)
   : IGM(IGM),
-    FirstType(invalidTypeInfo()),
-    FirstProtocol(invalidProtocolInfo()) {
+    FirstType(invalidTypeInfo()) {
   // FIXME: In LLDB, everything is completely fragile, so that IRGen can query
   // the size of resilient types. Of course this is not the right long term
   // solution, because it won't work once the swiftmodule file is not in
@@ -1081,9 +1079,9 @@ TypeConverter::~TypeConverter() {
     delete Cur;
   }
   
-  for (const ProtocolInfo *I = FirstProtocol; I != invalidProtocolInfo(); ) {
+  for (const ProtocolInfo *I = FirstProtocol; I != nullptr; ) {
     const ProtocolInfo *Cur = I;
-    I = Cur->NextConverted;
+    I = Cur->NextConverted.getPointer();
     delete Cur;
   }
 }
diff --git a/lib/IRGen/GenType.h b/lib/IRGen/GenType.h
@@ -70,7 +70,7 @@ class TypeConverter {
   llvm::DenseMap<ProtocolDecl*, const ProtocolInfo*> Protocols;
   const TypeInfo *FirstType;
   
-  const ProtocolInfo *FirstProtocol;
+  const ProtocolInfo *FirstProtocol = nullptr;
   const LoadableTypeInfo *NativeObjectTI = nullptr;
   const LoadableTypeInfo *UnknownObjectTI = nullptr;
   const LoadableTypeInfo *BridgeObjectTI = nullptr;
@@ -146,7 +146,7 @@ class TypeConverter {
   const LoadableTypeInfo &getWitnessTablePtrTypeInfo();
   const LoadableTypeInfo &getEmptyTypeInfo();
   const TypeInfo &getResilientStructTypeInfo(IsABIAccessible_t abiAccessible);
-  const ProtocolInfo &getProtocolInfo(ProtocolDecl *P);
+  const ProtocolInfo &getProtocolInfo(ProtocolDecl *P, ProtocolInfoKind kind);
   const LoadableTypeInfo &getOpaqueStorageTypeInfo(Size storageSize,
                                                    Alignment storageAlign);
   const TypeInfo &getMetatypeTypeInfo(MetatypeRepresentation representation);
diff --git a/lib/IRGen/IRGenModule.h b/lib/IRGen/IRGenModule.h
@@ -129,6 +129,7 @@ namespace irgen {
   class NominalMetadataLayout;
   class OutliningMetadataCollector;
   class ProtocolInfo;
+  enum class ProtocolInfoKind : uint8_t;
   class Signature;
   class StructMetadataLayout;
   struct SymbolicMangling;
@@ -703,7 +704,7 @@ class IRGenModule {
   
 //--- Types -----------------------------------------------------------------
 public:
-  const ProtocolInfo &getProtocolInfo(ProtocolDecl *D);
+  const ProtocolInfo &getProtocolInfo(ProtocolDecl *D, ProtocolInfoKind kind);
 
   // Not strictly a type operation, but similar.
   const ConformanceInfo &
diff --git a/lib/IRGen/ProtocolInfo.h b/lib/IRGen/ProtocolInfo.h
@@ -146,6 +146,19 @@ class WitnessTableEntry {
     assert(isAssociatedConformance());
     return Protocol;
   }
+
+  friend bool operator==(WitnessTableEntry left, WitnessTableEntry right) {
+    return left.MemberOrAssociatedType == right.MemberOrAssociatedType &&
+           left.Protocol == right.Protocol;
+  }
+};
+
+/// Describes the information available in a ProtocolInfo.
+///
+/// Each kind includes the information of the kinds before it.
+enum class ProtocolInfoKind : uint8_t {
+  RequirementSignature,
+  Full
 };
 
 /// An abstract description of a protocol.
@@ -154,24 +167,30 @@ class ProtocolInfo final :
   friend TrailingObjects;
 
   /// A singly-linked-list of all the protocols that have been laid out.
-  const ProtocolInfo *NextConverted;
+  llvm::PointerIntPair<const ProtocolInfo *, 1, ProtocolInfoKind> NextConverted;
   friend class TypeConverter;
 
+  ProtocolInfoKind getKind() const {
+    return NextConverted.getInt();
+  }
+
   /// The number of table entries in this protocol layout.
   unsigned NumTableEntries;
 
-  ProtocolInfo(ArrayRef<WitnessTableEntry> table)
-      : NumTableEntries(table.size()) {
+  ProtocolInfo(ArrayRef<WitnessTableEntry> table, ProtocolInfoKind kind)
+      : NextConverted(nullptr, kind), NumTableEntries(table.size()) {
     std::uninitialized_copy(table.begin(), table.end(),
                             getTrailingObjects<WitnessTableEntry>());
   }
 
-  static ProtocolInfo *create(ArrayRef<WitnessTableEntry> table);
+  static ProtocolInfo *create(ArrayRef<WitnessTableEntry> table,
+                              ProtocolInfoKind kind);
 
 public:
   /// The number of witness slots in a conformance to this protocol;
   /// in other words, the size of the table in words.
   unsigned getNumWitnesses() const {
+    assert(getKind() == ProtocolInfoKind::Full);
     return NumTableEntries;
   }
 
@@ -219,6 +238,7 @@ class ProtocolInfo final :
   /// Return the witness index for the witness function for the given
   /// function requirement.
   WitnessIndex getFunctionIndex(AbstractFunctionDecl *function) const {
+    assert(getKind() >= ProtocolInfoKind::Full);
     for (auto &witness : getWitnessEntries()) {
       if (witness.matchesFunction(function))
         return getNonBaseWitnessIndex(&witness);
diff --git a/test/IRGen/Inputs/protocol_accessor_multifile_other.swift b/test/IRGen/Inputs/protocol_accessor_multifile_other.swift
@@ -8,3 +8,13 @@ extension Proto {
 var globalExistential: Proto {
   fatalError()
 }
+
+protocol ClassProtoBase: AnyObject {
+  var baseProp: Int { get set }
+}
+protocol ClassProto: ClassProtoBase {
+  var prop: Int { get set }
+}
+func getClassExistential() -> ClassProto? {
+  fatalError()
+}
diff --git a/test/IRGen/protocol_accessor_multifile.swift b/test/IRGen/protocol_accessor_multifile.swift

Original file line number	Diff line number	Diff line change
`@@ -1058,12 +1058,10 @@ TypeConverter::createImmovable(llvm::Type *type, Size size, Alignment align) {`
`1058`	`1058`	`}`
`1059`	`1059`
`1060`	`1060`	`static TypeInfo invalidTypeInfo() { return (TypeInfo) 1; }`
`1061`		`-static ProtocolInfo invalidProtocolInfo() { return (ProtocolInfo) 1; }`
`1062`	`1061`
`1063`	`1062`	`TypeConverter::TypeConverter(IRGenModule &IGM)`
`1064`	`1063`	`: IGM(IGM),`
`1065`		`- FirstType(invalidTypeInfo()),`
`1066`		`- FirstProtocol(invalidProtocolInfo()) {`
	`1064`	`+ FirstType(invalidTypeInfo()) {`
`1067`	`1065`	`// FIXME: In LLDB, everything is completely fragile, so that IRGen can query`
`1068`	`1066`	`// the size of resilient types. Of course this is not the right long term`
`1069`	`1067`	`// solution, because it won't work once the swiftmodule file is not in`
`@@ -1081,9 +1079,9 @@ TypeConverter::~TypeConverter() {`
`1081`	`1079`	`delete Cur;`
`1082`	`1080`	`}`
`1083`	`1081`
`1084`		`- for (const ProtocolInfo *I = FirstProtocol; I != invalidProtocolInfo(); ) {`
	`1082`	`+ for (const ProtocolInfo *I = FirstProtocol; I != nullptr; ) {`
`1085`	`1083`	`const ProtocolInfo *Cur = I;`
`1086`		`- I = Cur->NextConverted;`
	`1084`	`+ I = Cur->NextConverted.getPointer();`
`1087`	`1085`	`delete Cur;`
`1088`	`1086`	`}`
`1089`	`1087`	`}`