Merge pull request #39588 from slavapestov/circular-conformance-path-fix

slavapestov · web-flow · commit 92520d5c4712 · 2021-10-06T13:18:42.000-04:00
IRGen: Fix subtle issue with "circular" conformance access paths
diff --git a/lib/AST/RequirementMachine/GenericSignatureQueries.cpp b/lib/AST/RequirementMachine/GenericSignatureQueries.cpp
@@ -434,19 +434,6 @@ Type RequirementMachine::getCanonicalTypeInContext(
   });
 }
 
-/// Replace 'Self' in the given dependent type (\c depTy) with the given
-/// dependent type, producing a type that refers to the nested type.
-static Type replaceSelfWithType(Type selfType, Type depTy) {
-  if (auto depMemTy = depTy->getAs<DependentMemberType>()) {
-    Type baseType = replaceSelfWithType(selfType, depMemTy->getBase());
-    assert(depMemTy->getAssocType() && "Missing associated type");
-    return DependentMemberType::get(baseType, depMemTy->getAssocType());
-  }
-
-  assert(depTy->is<GenericTypeParamType>() && "missing Self?");
-  return selfType;
-}
-
 /// Retrieve the conformance access path used to extract the conformance of
 /// interface \c type to the given \c protocol.
 ///
@@ -462,12 +449,29 @@ static Type replaceSelfWithType(Type selfType, Type depTy) {
 ConformanceAccessPath
 RequirementMachine::getConformanceAccessPath(Type type,
                                              ProtocolDecl *protocol) {
-  auto canType = getCanonicalTypeInContext(type, { })->getCanonicalType();
-  assert(canType->isTypeParameter());
+  assert(type->isTypeParameter());
+
+  auto mutTerm = Context.getMutableTermForType(type->getCanonicalType(),
+                                               /*proto=*/nullptr);
+  System.simplify(mutTerm);
+  verify(mutTerm);
+
+#ifndef NDEBUG
+  auto *props = Map.lookUpProperties(mutTerm);
+  assert(props &&
+         "Subject type of conformance access path should be known");
+  assert(!props->isConcreteType() &&
+         "Concrete types do not have conformance access paths");
+  auto conformsTo = props->getConformsTo();
+  assert(std::find(conformsTo.begin(), conformsTo.end(), protocol) &&
+         "Subject type of conformance access path must conform to protocol");
+#endif
+
+  auto term = Term::get(mutTerm, Context);
 
   // Check if we've already cached the result before doing anything else.
   auto found = ConformanceAccessPaths.find(
-      std::make_pair(canType, protocol));
+      std::make_pair(term, protocol));
   if (found != ConformanceAccessPaths.end()) {
     return found->second;
   }
@@ -476,13 +480,13 @@ RequirementMachine::getConformanceAccessPath(Type type,
 
   FrontendStatsTracer tracer(Stats, "get-conformance-access-path");
 
-  auto recordPath = [&](CanType type, ProtocolDecl *proto,
+  auto recordPath = [&](Term term, ProtocolDecl *proto,
                         ConformanceAccessPath path) {
     // Add the path to the buffer.
-    CurrentConformanceAccessPaths.emplace_back(type, path);
+    CurrentConformanceAccessPaths.emplace_back(term, path);
 
     // Add the path to the map.
-    auto key = std::make_pair(type, proto);
+    auto key = std::make_pair(term, proto);
     auto inserted = ConformanceAccessPaths.insert(
         std::make_pair(key, path));
     assert(inserted.second);
@@ -508,22 +512,27 @@ RequirementMachine::getConformanceAccessPath(Type type,
       ArrayRef<ConformanceAccessPath::Entry> path(root);
       ConformanceAccessPath result(ctx.AllocateCopy(path));
 
-      recordPath(rootType, rootProto, result);
+      auto mutTerm = Context.getMutableTermForType(rootType, nullptr);
+      System.simplify(mutTerm);
+
+      auto rootTerm = Term::get(mutTerm, Context);
+      recordPath(rootTerm, rootProto, result);
     }
   }
 
   // We enumerate conformance access paths in shortlex order until we find the
   // path whose corresponding type canonicalizes to the one we are looking for.
   while (true) {
     auto found = ConformanceAccessPaths.find(
-        std::make_pair(canType, protocol));
+        std::make_pair(term, protocol));
     if (found != ConformanceAccessPaths.end()) {
       return found->second;
     }
 
     if (CurrentConformanceAccessPaths.empty()) {
       llvm::errs() << "Failed to find conformance access path for ";
-      llvm::errs() << type << " " << protocol->getName() << ":\n";
+      llvm::errs() << type << " (" << term << ")" << " : ";
+      llvm::errs() << protocol->getName() << ":\n";
       type.dump(llvm::errs());
       llvm::errs() << "\n";
       dump(llvm::errs());
@@ -533,7 +542,7 @@ RequirementMachine::getConformanceAccessPath(Type type,
     // The buffer consists of all conformance access paths of length N.
     // Swap it out with an empty buffer, and fill it with all paths of
     // length N+1.
-    std::vector<std::pair<CanType, ConformanceAccessPath>> oldPaths;
+    std::vector<std::pair<Term, ConformanceAccessPath>> oldPaths;
     std::swap(CurrentConformanceAccessPaths, oldPaths);
 
     for (const auto &pair : oldPaths) {
@@ -551,21 +560,19 @@ RequirementMachine::getConformanceAccessPath(Type type,
         auto nextSubjectType = req.getFirstType()->getCanonicalType();
         auto *nextProto = req.getProtocolDecl();
 
-        // Compute the canonical anchor for this conformance requirement.
-        auto nextType = replaceSelfWithType(pair.first, nextSubjectType);
-        auto nextCanType = getCanonicalTypeInContext(nextType, { })
-            ->getCanonicalType();
+        MutableTerm mutTerm(pair.first);
+        mutTerm.append(Context.getMutableTermForType(nextSubjectType,
+                                                     /*proto=*/lastProto));
+        System.simplify(mutTerm);
 
-        // Skip "derived via concrete" sources.
-        if (!nextCanType->isTypeParameter())
-          continue;
+        auto nextTerm = Term::get(mutTerm, Context);
 
         // If we've already seen a path for this conformance, skip it and
         // don't add it to the buffer. Note that because we iterate over
         // conformance access paths in shortlex order, the existing
         // conformance access path is shorter than the one we found just now.
         if (ConformanceAccessPaths.count(
-                std::make_pair(nextCanType, nextProto)))
+                std::make_pair(nextTerm, nextProto)))
           continue;
 
         if (entries.empty()) {
@@ -580,7 +587,7 @@ RequirementMachine::getConformanceAccessPath(Type type,
         ConformanceAccessPath result = ctx.AllocateCopy(entries);
         entries.pop_back();
 
-        recordPath(nextCanType, nextProto, result);
+        recordPath(nextTerm, nextProto, result);
       }
     }
   }
diff --git a/lib/AST/RequirementMachine/RequirementMachine.h b/lib/AST/RequirementMachine/RequirementMachine.h
@@ -61,14 +61,14 @@ class RequirementMachine final {
   UnifiedStatsReporter *Stats;
 
   /// All conformance access paths computed so far.
-  llvm::DenseMap<std::pair<CanType, ProtocolDecl *>,
+  llvm::DenseMap<std::pair<Term, ProtocolDecl *>,
                  ConformanceAccessPath> ConformanceAccessPaths;
 
   /// Conformance access paths computed during the last round. All elements
   /// have the same length. If a conformance access path of greater length
   /// is requested, we refill CurrentConformanceAccessPaths with all paths of
   /// length N+1, and add them to the ConformanceAccessPaths map.
-  std::vector<std::pair<CanType, ConformanceAccessPath>>
+  std::vector<std::pair<Term, ConformanceAccessPath>>
       CurrentConformanceAccessPaths;
 
   explicit RequirementMachine(RewriteContext &rewriteCtx);
diff --git a/lib/AST/RequirementMachine/Term.h b/lib/AST/RequirementMachine/Term.h
@@ -65,6 +65,10 @@ class Term final {
     return Ptr;
   }
 
+  static Term fromOpaquePointer(void *ptr) {
+    return Term((Storage *) ptr);
+  }
+
   static Term get(const MutableTerm &term, RewriteContext &ctx);
 
   ArrayRef<const ProtocolDecl *> getRootProtocols() const {
@@ -203,4 +207,24 @@ class MutableTerm final {
 
 } // end namespace swift
 
+namespace llvm {
+  template<> struct DenseMapInfo<swift::rewriting::Term> {
+    static swift::rewriting::Term getEmptyKey() {
+      return swift::rewriting::Term::fromOpaquePointer(
+        llvm::DenseMapInfo<void *>::getEmptyKey());
+    }
+    static swift::rewriting::Term getTombstoneKey() {
+      return swift::rewriting::Term::fromOpaquePointer(
+        llvm::DenseMapInfo<void *>::getTombstoneKey());
+    }
+    static unsigned getHashValue(swift::rewriting::Term Val) {
+      return DenseMapInfo<void *>::getHashValue(Val.getOpaquePointer());
+    }
+    static bool isEqual(swift::rewriting::Term LHS,
+                        swift::rewriting::Term RHS) {
+      return LHS == RHS;
+    }
+  };
+} // end namespace llvm
+
 #endif
diff --git a/lib/IRGen/GenProto.cpp b/lib/IRGen/GenProto.cpp
@@ -2599,14 +2599,7 @@ MetadataResponse MetadataPath::followComponent(IRGenFunction &IGF,
 
     CanType associatedType =
       sourceConformance.getAssociatedType(sourceType, association)
-      ->getCanonicalType();
-    if (sourceConformance.isConcrete() &&
-        isa<NormalProtocolConformance>(sourceConformance.getConcrete())) {
-      associatedType =
-        sourceConformance.getConcrete()->getDeclContext()
-          ->mapTypeIntoContext(associatedType)
-          ->getCanonicalType();
-    }
+        ->getCanonicalType();
     sourceKey.Type = associatedType;
 
     auto associatedConformance =
@@ -2621,10 +2614,77 @@ MetadataResponse MetadataPath::followComponent(IRGenFunction &IGF,
 
     if (!source) return MetadataResponse();
 
-    auto sourceMetadata =
-      IGF.emitAbstractTypeMetadataRef(sourceType);
-    auto associatedMetadata =
-      IGF.emitAbstractTypeMetadataRef(sourceKey.Type);
+    auto *sourceMetadata =
+        IGF.emitAbstractTypeMetadataRef(sourceType);
+
+    // Try to avoid circularity when realizing the associated metadata.
+    //
+    // Suppose we are asked to produce the witness table for some
+    // conformance access path (T : P)(Self.X : Q)(Self.Y : R).
+    //
+    // The associated conformance accessor for (Self.Y : R) takes the
+    // metadata for T.X and T.X : Q as arguments. If T.X is concrete,
+    // there are two ways of building it:
+    //
+    // a) Using the knowledge of the concrete type to build it directly,
+    // by first constructing the type metadata for its generic arguments.
+    //
+    // b) Obtaining T.X from the witness table for T : P. This will also
+    // construct the generic type, but from the generic environment
+    // of the concrete type of T, and not the abstract environment of
+    // our conformance access path.
+    //
+    // Now, say that T.X == Foo<T.X.Y>, with "Foo<A> where A : R".
+    //
+    // If approach a) is taken, then constructing Foo<T.X.Y> requires
+    // recovering the conformance T.X.Y : R, which recursively evaluates
+    // the same conformance access path, eventually causing a stack
+    // overflow.
+    //
+    // With approach b) on the other hand, the type metadata for
+    // Foo<T.X.Y> is constructed from the concrete type metadata for T,
+    // which must provide some other conformance access path for the
+    // conformance to R.
+    //
+    // This is not very principled, and a remaining issue is with conformance
+    // requirements where the subject type consists of multiple terms.
+    //
+    // A better approach would be for conformance access paths to directly
+    // record how type metadata at each intermediate step is constructed.
+    llvm::Value *associatedMetadata = nullptr;
+
+    if (auto response = IGF.tryGetLocalTypeMetadata(associatedType,
+                                                    MetadataState::Abstract)) {
+      // The associated type metadata was already cached, so we're fine.
+      associatedMetadata = response.getMetadata();
+    } else {
+      // If the associated type is concrete and the parent type is an archetype,
+      // it is better to realize the associated type metadata from the witness
+      // table of the parent's conformance, instead of realizing the concrete
+      // type directly.
+      auto depMemType = cast<DependentMemberType>(association);
+      CanType baseSubstType =
+        sourceConformance.getAssociatedType(sourceType, depMemType.getBase())
+          ->getCanonicalType();
+      if (auto archetypeType = cast<ArchetypeType>(baseSubstType)) {
+        AssociatedType baseAssocType(depMemType->getAssocType());
+
+        MetadataResponse response =
+          emitAssociatedTypeMetadataRef(IGF, archetypeType, baseAssocType,
+                                        MetadataState::Abstract);
+
+        // Cache this response in case we have to realize the associated type
+        // again later.
+        IGF.setScopedLocalTypeMetadata(associatedType, response);
+
+        associatedMetadata = response.getMetadata();
+      } else {
+        // Ok, fall back to realizing the (possibly concrete) type.
+        associatedMetadata =
+          IGF.emitAbstractTypeMetadataRef(sourceKey.Type);
+      }
+    }
+
     auto sourceWTable = source.getMetadata();
 
     AssociatedConformance associatedConformanceRef(sourceProtocol,
diff --git a/test/Generics/rdar83687967.swift b/test/Generics/rdar83687967.swift
@@ -0,0 +1,66 @@
+// RUN: %target-typecheck-verify-swift
+// RUN: %target-swift-frontend -typecheck -debug-generic-signatures %s 2>&1 | %FileCheck %s
+// RUN: %target-swift-frontend -emit-ir %s
+
+public protocol P1 {}
+
+public protocol P2 {
+  associatedtype A: P1
+}
+
+public protocol P3 {
+  associatedtype B: P2
+  associatedtype A where B.A == A
+}
+
+public struct G<A: P1>: P2 {}
+
+public func callee<T: P1>(_: T.Type) {}
+
+// CHECK: rdar83687967.(file).caller11@
+// CHECK: Generic signature: <Child where Child : P3, Child.B == G<Child.A>>
+public func caller11<Child: P3>(_: Child)
+    where Child.B == G<Child.A> {
+  callee(Child.A.self)
+}
+
+// CHECK: rdar83687967.(file).caller12@
+// CHECK: Generic signature: <Child where Child : P3, Child.B == G<Child.A>>
+public func caller12<Child: P3>(_: Child)
+    // expected-note@-1 {{conformance constraint 'Child.A' : 'P1' implied here}}
+    where Child.B == G<Child.A>, Child.A : P1 {
+    // expected-warning@-1 {{redundant conformance constraint 'Child.A' : 'P1'}}
+
+  // Make sure IRGen can evaluate the conformance access path
+  // (Child : P3)(Self.B : P2)(Self.A : P1).
+  callee(Child.A.self)
+}
+
+// CHECK: rdar83687967.(file).X1@
+// CHECK: Requirement signature: <Self where Self.Child : P3, Self.Child.B == G<Self.Child.A>>
+public protocol X1 {
+  associatedtype Child: P3
+    where Child.B == G<Child.A>
+}
+
+// CHECK: rdar83687967.(file).X2@
+// CHECK: Requirement signature: <Self where Self.Child : P3, Self.Child.B == G<Self.Child.A>>
+
+public protocol X2 {
+  associatedtype Child: P3
+    // expected-note@-1 {{conformance constraint 'Self.Child.A' : 'P1' implied here}}
+    where Child.B == G<Child.A>, Child.A : P1
+    // expected-warning@-1 {{redundant conformance constraint 'Self.Child.A' : 'P1'}}
+}
+
+public func caller21<T : X1>(_: T) {
+  // Make sure IRGen can evaluate the conformance access path
+  // (T : X1)(Child : P3)(Self.B : P2)(Self.A : P1).
+  callee(T.Child.A.self)
+}
+
+public func caller22<T : X2>(_: T) {
+  // Make sure IRGen can evaluate the conformance access path
+  // (T : X2)(Child : P3)(Self.B : P2)(Self.A : P1).
+  callee(T.Child.A.self)
+}
diff --git a/test/IRGen/associated_types.swift b/test/IRGen/associated_types.swift
@@ -75,17 +75,17 @@ func testFastRuncible<T: Runcible, U: FastRuncible>(_ t: T, u: U)
 //   1. Get the type metadata for U.RuncerType.Runcee.
 //     1a. Get the type metadata for U.RuncerType.
 //         Note that we actually look things up in T, which is going to prove unfortunate.
-// CHECK: [[T2:%.*]] = call swiftcc %swift.metadata_response @swift_getAssociatedTypeWitness([[INT]] 255, i8** %T.Runcible, %swift.type* %T, %swift.protocol_requirement* getelementptr{{.*}}i32 12), i32 -1), %swift.protocol_requirement* getelementptr inbounds (<{{.*}}>, <{{.*}}>* @"$s16associated_types8RuncibleMp", i32 0, i32 14)) [[NOUNWIND_READNONE:#.*]]
-// CHECK-NEXT: %T.RuncerType = extractvalue %swift.metadata_response [[T2]], 0
+// CHECK: [[T2:%.*]] = call swiftcc %swift.metadata_response @swift_getAssociatedTypeWitness([[INT]] 255, i8** %U.FastRuncible, %swift.type* %U, %swift.protocol_requirement* getelementptr{{.*}}i32 9), i32 -1), %swift.protocol_requirement* getelementptr inbounds (<{{.*}}>, <{{.*}}>* @"$s16associated_types12FastRuncibleMp", i32 0, i32 10)) [[NOUNWIND_READNONE:#.*]]
+// CHECK-NEXT: [[T3:%.*]] = extractvalue %swift.metadata_response [[T2]], 0
 // CHECK-NEXT: extractvalue %swift.metadata_response [[T2]], 1
 //   2. Get the witness table for U.RuncerType.Runcee : Speedy
 //     2a. Get the protocol witness table for U.RuncerType : FastRuncer.
-// CHECK-NEXT: %T.RuncerType.FastRuncer = call swiftcc i8** @swift_getAssociatedConformanceWitness(i8** %U.FastRuncible, %swift.type* %U, %swift.type* %T.RuncerType
+// CHECK-NEXT: %T.RuncerType.FastRuncer = call swiftcc i8** @swift_getAssociatedConformanceWitness(i8** %U.FastRuncible, %swift.type* %U, %swift.type* [[T3]]
 //     1c. Get the type metadata for U.RuncerType.Runcee.
-// CHECK-NEXT: [[T2:%.*]] = call swiftcc %swift.metadata_response @swift_getAssociatedTypeWitness([[INT]] 0, i8** %T.RuncerType.FastRuncer, %swift.type* %T.RuncerType, {{.*}}, %swift.protocol_requirement* getelementptr inbounds (<{{.*}}>, <{{.*}}>* @"$s16associated_types10FastRuncerMp", i32 0, i32 10))
+// CHECK-NEXT: [[T2:%.*]] = call swiftcc %swift.metadata_response @swift_getAssociatedTypeWitness([[INT]] 0, i8** %T.RuncerType.FastRuncer, %swift.type* [[T3]], {{.*}}, %swift.protocol_requirement* getelementptr inbounds (<{{.*}}>, <{{.*}}>* @"$s16associated_types10FastRuncerMp", i32 0, i32 10))
 // CHECK-NEXT: %T.RuncerType.Runcee = extractvalue %swift.metadata_response [[T2]], 0
 //     2b. Get the witness table for U.RuncerType.Runcee : Speedy.
-// CHECK-NEXT: %T.RuncerType.Runcee.Speedy = call swiftcc i8** @swift_getAssociatedConformanceWitness(i8** %T.RuncerType.FastRuncer, %swift.type* %T.RuncerType, %swift.type* %T.RuncerType.Runcee
+// CHECK-NEXT: %T.RuncerType.Runcee.Speedy = call swiftcc i8** @swift_getAssociatedConformanceWitness(i8** %T.RuncerType.FastRuncer, %swift.type* [[T3]], %swift.type* %T.RuncerType.Runcee
 //   3. Perform the actual call.
 // CHECK-NEXT: [[T0_GEP:%.*]] = getelementptr inbounds i8*, i8** %T.RuncerType.Runcee.Speedy, i32 1
 // CHECK-NEXT: [[T0:%.*]] = load i8*, i8** [[T0_GEP]]
