[Generic signature builder] Substitute requirements instead of threading a PA around.

huonw · huonw · commit f1dba0e7e8b8 · 2017-02-10T18:58:56.000-08:00
This essentially undoes the implementation in 51da51d, which implicitly did a substitution of the Self type in a protocol's requirement signature by threading around the replacement PA. This is brittle because every part of the code needs to take and pass around the argument. By preemptively substituting, the whole requirement is in the right form from the time it enters `addRequirement`. The infrastructure here also allows simplifying some code.
diff --git a/include/swift/AST/GenericSignatureBuilder.h b/include/swift/AST/GenericSignatureBuilder.h
@@ -172,7 +172,6 @@ class GenericSignatureBuilder {
   /// requirement mismatch.
   bool addSameTypeRequirement(
                         Type T1, Type T2, RequirementSource Source,
-                        PotentialArchetype *basePA,
                         llvm::function_ref<void(Type, Type)> diagnoseMismatch);
 
   /// Add the requirements placed on the given abstract type parameter
@@ -258,7 +257,6 @@ class GenericSignatureBuilder {
   bool addRequirement(const Requirement &req, RequirementSource source);
 
   bool addRequirement(const Requirement &req, RequirementSource source,
-                      PotentialArchetype *basePA,
                       llvm::SmallPtrSetImpl<ProtocolDecl *> &Visited);
 
   bool addLayoutRequirement(PotentialArchetype *PAT,
@@ -323,12 +321,7 @@ class GenericSignatureBuilder {
   /// signature are fully resolved).
   ///
   /// For any type that cannot refer to an archetype, this routine returns null.
-  ///
-  /// A non-null \c basePA is used in place of the "true" potential archetype
-  /// for a GenericTypeParamType, effectively performing a substitution like,
-  /// e.g., Self = <some PA>.
-  PotentialArchetype *resolveArchetype(Type type,
-                                       PotentialArchetype *basePA = nullptr);
+  PotentialArchetype *resolveArchetype(Type type);
 
   /// \brief Dump all of the requirements, both specified and inferred.
   LLVM_ATTRIBUTE_DEPRECATED(
diff --git a/include/swift/AST/Requirement.h b/include/swift/AST/Requirement.h
@@ -86,6 +86,31 @@ class Requirement {
     return SecondType;
   }
 
+  /// \brief Subst the types involved in this requirement.
+  ///
+  /// The \c args arguments are passed through to Type::subst. This doesn't
+  /// touch the superclasses, protocols or layout constraints.
+  template <typename... Args>
+  Optional<Requirement> subst(Args &&... args) const {
+    auto newFirst = getFirstType().subst(std::forward<Args>(args)...);
+    if (!newFirst)
+      return None;
+
+    switch (getKind()) {
+    case RequirementKind::SameType: {
+      auto newSecond = getSecondType().subst(std::forward<Args>(args)...);
+      if (!newSecond)
+        return None;
+      return Requirement(getKind(), newFirst, newSecond);
+    }
+    case RequirementKind::Conformance:
+    case RequirementKind::Superclass:
+      return Requirement(getKind(), newFirst, getSecondType());
+    case RequirementKind::Layout:
+      return Requirement(getKind(), newFirst, getLayoutConstraint());
+    }
+  }
+
   /// \brief Retrieve the layout constraint.
   LayoutConstraint getLayoutConstraint() const {
     assert(getKind() == RequirementKind::Layout);
diff --git a/lib/AST/GenericSignatureBuilder.cpp b/lib/AST/GenericSignatureBuilder.cpp
@@ -1019,12 +1019,8 @@ LazyResolver *GenericSignatureBuilder::getLazyResolver() const {
   return Context.getLazyResolver();
 }
 
-auto GenericSignatureBuilder::resolveArchetype(Type type, PotentialArchetype *basePA)
-    -> PotentialArchetype * {
+auto GenericSignatureBuilder::resolveArchetype(Type type) -> PotentialArchetype * {
   if (auto genericParam = type->getAs<GenericTypeParamType>()) {
-    if (basePA)
-      return basePA;
-
     unsigned index = GenericParamKey(genericParam).findIndexIn(
                                                            Impl->GenericParams);
     if (index < Impl->GenericParams.size())
@@ -1034,7 +1030,7 @@ auto GenericSignatureBuilder::resolveArchetype(Type type, PotentialArchetype *ba
   }
 
   if (auto dependentMember = type->getAs<DependentMemberType>()) {
-    auto base = resolveArchetype(dependentMember->getBase(), basePA);
+    auto base = resolveArchetype(dependentMember->getBase());
     if (!base)
       return nullptr;
 
@@ -1112,9 +1108,15 @@ bool GenericSignatureBuilder::addConformanceRequirement(PotentialArchetype *PAT,
   if (Proto->isRequirementSignatureComputed()) {
     auto reqSig = Proto->getRequirementSignature();
 
-    for (auto req : reqSig->getRequirements()) {
+    auto concreteSelf = T->getDependentType({}, /*allowUnresolved=*/true);
+    auto subMap = SubstitutionMap::getProtocolSubstitutions(
+        Proto, concreteSelf, ProtocolConformanceRef(Proto));
+
+    for (auto rawReq : reqSig->getRequirements()) {
+      auto req = rawReq.subst(subMap);
+      assert(req && "substituting Self in requirement shouldn't fail");
       RequirementSource InnerSource(Kind, Source.getLoc());
-      addRequirement(req, InnerSource, T, Visited);
+      addRequirement(*req, InnerSource, Visited);
     }
   } else {
     // Conformances to inherit protocols are explicit in a protocol requirement
@@ -1333,9 +1335,8 @@ bool GenericSignatureBuilder::addSameTypeRequirementBetweenArchetypes(
   Type concrete2 = T2->getConcreteType();
   
   if (concrete1 && concrete2) {
-    bool mismatch =
-      addSameTypeRequirement(concrete1, concrete2, Source, nullptr,
-        [&](Type type1, Type type2) {
+    bool mismatch = addSameTypeRequirement(
+        concrete1, concrete2, Source, [&](Type type1, Type type2) {
           Diags.diagnose(Source.getLoc(),
                          diag::requires_same_type_conflict,
                          T1->getDependentType(/*FIXME: */{ }, true), type1,
@@ -1411,9 +1412,8 @@ bool GenericSignatureBuilder::addSameTypeRequirementToConcrete(
   // If we've already been bound to a type, we're either done, or we have a
   // problem.
   if (auto oldConcrete = T->getConcreteType()) {
-    bool mismatch =
-      addSameTypeRequirement(oldConcrete, Concrete, Source, nullptr,
-        [&](Type type1, Type type2) {
+    bool mismatch = addSameTypeRequirement(
+        oldConcrete, Concrete, Source, [&](Type type1, Type type2) {
           Diags.diagnose(Source.getLoc(),
                          diag::requires_same_type_conflict,
                          T->getDependentType(/*FIXME: */{ }, true), type1,
@@ -1486,30 +1486,25 @@ bool GenericSignatureBuilder::addSameTypeRequirementToConcrete(
 }
 
 bool GenericSignatureBuilder::addSameTypeRequirement(
-                        Type type1, Type type2,
-                        RequirementSource source,
-                        PotentialArchetype *basePA,
-                        llvm::function_ref<void(Type, Type)> diagnoseMismatch) {
+    Type type1, Type type2, RequirementSource source,
+    llvm::function_ref<void(Type, Type)> diagnoseMismatch) {
   // Local class to handle matching the two sides of the same-type constraint.
   class ReqTypeMatcher : public TypeMatcher<ReqTypeMatcher> {
     GenericSignatureBuilder &builder;
     RequirementSource source;
-    PotentialArchetype *basePA;
     llvm::function_ref<void(Type, Type)> diagnoseMismatch;
 
   public:
-    ReqTypeMatcher(GenericSignatureBuilder &builder,
-                   RequirementSource source,
-                   PotentialArchetype *basePA,
+    ReqTypeMatcher(GenericSignatureBuilder &builder, RequirementSource source,
                    llvm::function_ref<void(Type, Type)> diagnoseMismatch)
-      : builder(builder), source(source), basePA(basePA),
-        diagnoseMismatch(diagnoseMismatch) { }
+        : builder(builder), source(source), diagnoseMismatch(diagnoseMismatch) {
+    }
 
     bool mismatch(TypeBase *firstType, TypeBase *secondType,
                   Type sugaredFirstType) {
       // Find the potential archetypes.
-      PotentialArchetype *pa1 = builder.resolveArchetype(firstType, basePA);
-      PotentialArchetype *pa2 = builder.resolveArchetype(secondType, basePA);
+      PotentialArchetype *pa1 = builder.resolveArchetype(firstType);
+      PotentialArchetype *pa2 = builder.resolveArchetype(secondType);
 
       // If both sides of the requirement are type parameters, equate them.
       if (pa1 && pa2)
@@ -1527,7 +1522,7 @@ bool GenericSignatureBuilder::addSameTypeRequirement(
       diagnoseMismatch(sugaredFirstType, secondType);
       return false;
     }
-  } matcher(*this, source, basePA, diagnoseMismatch);
+  } matcher(*this, source, diagnoseMismatch);
 
   return !matcher.match(type1, type2);
 }
@@ -1697,17 +1692,16 @@ bool GenericSignatureBuilder::addRequirement(const RequirementRepr &Req) {
 bool GenericSignatureBuilder::addRequirement(const Requirement &req,
                                       RequirementSource source) {
   llvm::SmallPtrSet<ProtocolDecl *, 8> Visited;
-  return addRequirement(req, source, nullptr, Visited);
+  return addRequirement(req, source, Visited);
 }
 
 bool GenericSignatureBuilder::addRequirement(
     const Requirement &req, RequirementSource source,
-    PotentialArchetype *basePA,
     llvm::SmallPtrSetImpl<ProtocolDecl *> &Visited) {
   switch (req.getKind()) {
   case RequirementKind::Superclass: {
     // FIXME: Diagnose this.
-    PotentialArchetype *pa = resolveArchetype(req.getFirstType(), basePA);
+    PotentialArchetype *pa = resolveArchetype(req.getFirstType());
     if (!pa) return false;
 
     assert(req.getSecondType()->getClassOrBoundGenericClass());
@@ -1716,15 +1710,15 @@ bool GenericSignatureBuilder::addRequirement(
 
   case RequirementKind::Layout: {
     // FIXME: Diagnose this.
-    PotentialArchetype *pa = resolveArchetype(req.getFirstType(), basePA);
+    PotentialArchetype *pa = resolveArchetype(req.getFirstType());
     if (!pa) return false;
 
     return addLayoutRequirement(pa, req.getLayoutConstraint(), source);
   }
 
   case RequirementKind::Conformance: {
     // FIXME: Diagnose this.
-    PotentialArchetype *pa = resolveArchetype(req.getFirstType(), basePA);
+    PotentialArchetype *pa = resolveArchetype(req.getFirstType());
     if (!pa) return false;
 
     SmallVector<ProtocolDecl *, 4> conformsTo;
@@ -1744,7 +1738,7 @@ bool GenericSignatureBuilder::addRequirement(
 
   case RequirementKind::SameType:
     return addSameTypeRequirement(
-             req.getFirstType(), req.getSecondType(), source, basePA,
+             req.getFirstType(), req.getSecondType(), source,
              [&](Type type1, Type type2) {
                if (source.getLoc().isValid())
                  Diags.diagnose(source.getLoc(),
@@ -1804,25 +1798,21 @@ class GenericSignatureBuilder::InferRequirementsWalker : public TypeWalker {
 
     // Handle the requirements.
     RequirementSource source(RequirementSource::Inferred, Loc);
-    for (const auto &req : genericSig->getRequirements()) {
-      switch (req.getKind()) {
-      case RequirementKind::SameType: {
-        auto firstType = req.getFirstType().subst(
-                           getTypeSubstitution,
-                           Builder.getLookupConformanceFn());
-        if (!firstType)
-          break;
+    for (const auto &rawReq : genericSig->getRequirements()) {
+      auto req =
+          rawReq.subst(getTypeSubstitution, Builder.getLookupConformanceFn());
+      if (!req)
+        continue;
 
+      switch (req->getKind()) {
+      case RequirementKind::SameType: {
+        auto firstType = req->getFirstType();
         auto firstPA = Builder.resolveArchetype(firstType);
 
         if (firstPA && isOuterArchetype(firstPA))
           return Action::Continue;
 
-        auto secondType = req.getSecondType().subst(
-                           getTypeSubstitution,
-                           Builder.getLookupConformanceFn());
-        if (!secondType)
-          break;
+        auto secondType = req->getSecondType();
         auto secondPA = Builder.resolveArchetype(secondType);
 
         if (firstPA && secondPA) {
@@ -1843,12 +1833,7 @@ class GenericSignatureBuilder::InferRequirementsWalker : public TypeWalker {
       case RequirementKind::Superclass:
       case RequirementKind::Layout:
       case RequirementKind::Conformance: {
-        auto subjectType = req.getFirstType().subst(
-                             getTypeSubstitution,
-                             Builder.getLookupConformanceFn());
-        if (!subjectType)
-          break;
-
+        auto subjectType = req->getFirstType();
         auto subjectPA = Builder.resolveArchetype(subjectType);
         if (!subjectPA) {
           break;
@@ -1857,19 +1842,19 @@ class GenericSignatureBuilder::InferRequirementsWalker : public TypeWalker {
         if (isOuterArchetype(subjectPA))
           return Action::Continue;
 
-        if (req.getKind() == RequirementKind::Conformance) {
-          auto proto = req.getSecondType()->castTo<ProtocolType>();
+        if (req->getKind() == RequirementKind::Conformance) {
+          auto proto = req->getSecondType()->castTo<ProtocolType>();
           if (Builder.addConformanceRequirement(subjectPA, proto->getDecl(),
                                                 source)) {
             return Action::Stop;
           }
-        } else if (req.getKind() == RequirementKind::Layout) {
-          if (Builder.addLayoutRequirement(subjectPA, req.getLayoutConstraint(),
-                                           source)) {
+        } else if (req->getKind() == RequirementKind::Layout) {
+          if (Builder.addLayoutRequirement(
+                  subjectPA, req->getLayoutConstraint(), source)) {
             return Action::Stop;
           }
         } else {
-          if (Builder.addSuperclassRequirement(subjectPA, req.getSecondType(),
+          if (Builder.addSuperclassRequirement(subjectPA, req->getSecondType(),
                                                source)) {
             return Action::Stop;
           }
