Merge pull request #60587 from slavapestov/requirement-subst-cleanup-5.7

slavapestov · web-flow · commit 4076fd3bd6b1 · 2022-08-17T09:49:39.000-04:00
AST: Change return type of Requirement::subst() to Requirement [5.7]
diff --git a/include/swift/AST/Requirement.h b/include/swift/AST/Requirement.h
@@ -53,21 +53,15 @@ class Requirement
 
   /// 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.
+  /// The \c args arguments are passed through to Type::subst.
   template <typename ...Args>
-  llvm::Optional<Requirement> subst(Args &&...args) const {
+  Requirement subst(Args &&...args) const {
     auto newFirst = getFirstType().subst(std::forward<Args>(args)...);
-    if (newFirst->hasError())
-      return None;
-
     switch (getKind()) {
     case RequirementKind::Conformance:
     case RequirementKind::Superclass:
     case RequirementKind::SameType: {
       auto newSecond = getSecondType().subst(std::forward<Args>(args)...);
-      if (newSecond->hasError())
-        return None;
       return Requirement(getKind(), newFirst, newSecond);
     }
     case RequirementKind::Layout:
diff --git a/lib/AST/ASTContext.cpp b/lib/AST/ASTContext.cpp
@@ -5340,9 +5340,8 @@ ASTContext::getOverrideGenericSignature(const ValueDecl *base,
     };
 
     for (auto reqt : baseGenericSig.getRequirements()) {
-      if (auto substReqt = reqt.subst(substFn, lookupConformanceFn)) {
-        addedRequirements.push_back(*substReqt);
-      }
+      auto substReqt = reqt.subst(substFn, lookupConformanceFn);
+      addedRequirements.push_back(substReqt);
     }
   }
 
diff --git a/lib/AST/ExistentialGeneralization.cpp b/lib/AST/ExistentialGeneralization.cpp
@@ -212,10 +212,7 @@ class Generalizer : public CanTypeVisitor<Generalizer, Type> {
         if (origReq.getKind() != RequirementKind::Conformance) continue;
         auto origConformance = origConformances[i++];
 
-        auto optNewReq = origReq.subst(newSubs);
-        assert(optNewReq && "generalization substitution failed");
-        auto &newReq = *optNewReq;
-
+        auto newReq = origReq.subst(newSubs);
         addedRequirements.push_back(newReq);
 
         substConformances.insert({{newReq.getFirstType()->getCanonicalType(),
diff --git a/lib/AST/GenericSignature.cpp b/lib/AST/GenericSignature.cpp
@@ -373,19 +373,14 @@ bool GenericSignatureImpl::isRequirementSatisfied(
   if (requirement.getFirstType()->hasTypeParameter()) {
     auto *genericEnv = getGenericEnvironment();
 
-    auto substituted = requirement.subst(
+    requirement = requirement.subst(
         [&](SubstitutableType *type) -> Type {
           if (auto *paramType = type->getAs<GenericTypeParamType>())
             return genericEnv->mapTypeIntoContext(paramType);
 
           return type;
         },
         LookUpConformanceInSignature(this));
-
-    if (!substituted)
-      return false;
-
-    requirement = *substituted;
   }
 
   // FIXME: Need to check conditional requirements here.
diff --git a/lib/AST/GenericSignatureBuilder.cpp b/lib/AST/GenericSignatureBuilder.cpp
@@ -5169,8 +5169,8 @@ class GenericSignatureBuilder::InferRequirementsWalker : public TypeWalker {
       auto decl = TypeAlias->getDecl();
       auto subMap = TypeAlias->getSubstitutionMap();
       for (const auto &rawReq : decl->getGenericSignature().getRequirements()) {
-        if (auto req = rawReq.subst(subMap))
-          Builder.addRequirement(*req, source, nullptr);
+        auto req = rawReq.subst(subMap);
+        Builder.addRequirement(req, source, nullptr);
       }
 
       return Action::Continue;
@@ -5237,8 +5237,8 @@ class GenericSignatureBuilder::InferRequirementsWalker : public TypeWalker {
     // Handle the requirements.
     // FIXME: Inaccurate TypeReprs.
     for (const auto &rawReq : genericSig.getRequirements()) {
-      if (auto req = rawReq.subst(subMap))
-        Builder.addRequirement(*req, source, nullptr);
+      auto req = rawReq.subst(subMap);
+      Builder.addRequirement(req, source, nullptr);
     }
 
     return Action::Continue;
@@ -8402,7 +8402,7 @@ AbstractGenericSignatureRequestGSB::evaluate(
           },
           MakeAbstractConformanceForGenericType(),
           SubstFlags::AllowLoweredTypes);
-      resugaredRequirements.push_back(*resugaredReq);
+      resugaredRequirements.push_back(resugaredReq);
     }
 
     return GenericSignatureWithError(
diff --git a/lib/AST/ProtocolConformance.cpp b/lib/AST/ProtocolConformance.cpp
@@ -992,9 +992,9 @@ void SpecializedProtocolConformance::computeConditionalRequirements() const {
 
     SmallVector<Requirement, 4> newReqs;
     for (auto oldReq : *parentCondReqs) {
-      if (auto newReq = oldReq.subst(QuerySubstitutionMap{subMap},
-                                     LookUpConformanceInModule(module)))
-        newReqs.push_back(*newReq);
+      auto newReq = oldReq.subst(QuerySubstitutionMap{subMap},
+                                 LookUpConformanceInModule(module));
+      newReqs.push_back(newReq);
     }
     auto &ctxt = getProtocol()->getASTContext();
     ConditionalRequirements = ctxt.AllocateCopy(newReqs);
@@ -1201,7 +1201,7 @@ ProtocolConformance::subst(TypeSubstitutionFn subs,
 
     SmallVector<Requirement, 2> requirements;
     for (auto req : getConditionalRequirements()) {
-      requirements.push_back(*req.subst(subs, conformances, options));
+      requirements.push_back(req.subst(subs, conformances, options));
     }
 
     auto kind = cast<BuiltinProtocolConformance>(this)
diff --git a/lib/AST/RequirementEnvironment.cpp b/lib/AST/RequirementEnvironment.cpp
@@ -181,9 +181,9 @@ RequirementEnvironment::RequirementEnvironment(
 
   if (conformanceSig) {
     for (auto &rawReq : conformanceSig.getRequirements()) {
-      if (auto req = rawReq.subst(conformanceToSyntheticTypeFn,
-                                  conformanceToSyntheticConformanceFn))
-        requirements.push_back(*req);
+      auto req = rawReq.subst(conformanceToSyntheticTypeFn,
+                              conformanceToSyntheticConformanceFn);
+      requirements.push_back(req);
     }
   }
 
@@ -207,8 +207,8 @@ RequirementEnvironment::RequirementEnvironment(
   // Next, add each of the requirements (mapped from the requirement's
   // interface types into the abstract type parameters).
   for (auto &rawReq : reqSig.getRequirements()) {
-    if (auto req = rawReq.subst(reqToSyntheticEnvMap))
-      requirements.push_back(*req);
+    auto req = rawReq.subst(reqToSyntheticEnvMap);
+    requirements.push_back(req);
   }
 
   // Produce the generic signature and environment.
diff --git a/lib/AST/RequirementMachine/RequirementLowering.cpp b/lib/AST/RequirementMachine/RequirementLowering.cpp
@@ -465,8 +465,7 @@ struct InferRequirementsWalker : public TypeWalker {
       auto decl = typeAlias->getDecl();
       auto subMap = typeAlias->getSubstitutionMap();
       for (const auto &rawReq : decl->getGenericSignature().getRequirements()) {
-        if (auto req = rawReq.subst(subMap))
-          desugarRequirement(*req, SourceLoc(), reqs, errors);
+        desugarRequirement(rawReq.subst(subMap), SourceLoc(), reqs, errors);
       }
 
       return Action::Continue;
@@ -532,8 +531,8 @@ struct InferRequirementsWalker : public TypeWalker {
     // Handle the requirements.
     // FIXME: Inaccurate TypeReprs.
     for (const auto &rawReq : genericSig.getRequirements()) {
-      if (auto req = rawReq.subst(subMap))
-        desugarRequirement(*req, SourceLoc(), reqs, errors);
+      auto req = rawReq.subst(subMap);
+      desugarRequirement(req, SourceLoc(), reqs, errors);
     }
 
     return Action::Continue;
diff --git a/lib/AST/RequirementMachine/RequirementMachineRequests.cpp b/lib/AST/RequirementMachine/RequirementMachineRequests.cpp
@@ -599,7 +599,7 @@ AbstractGenericSignatureRequestRQM::evaluate(
           },
           MakeAbstractConformanceForGenericType(),
           SubstFlags::AllowLoweredTypes);
-      resugaredRequirements.push_back(*resugaredReq);
+      resugaredRequirements.push_back(resugaredReq);
     }
 
     return GenericSignatureWithError(
diff --git a/lib/AST/Type.cpp b/lib/AST/Type.cpp
@@ -4527,26 +4527,16 @@ static Type substGenericFunctionType(GenericFunctionType *genericFnType,
   for (const auto &req : genericFnType->getRequirements()) {
     // Substitute into the requirement.
     auto substReqt = req.subst(substitutions, lookupConformances, options);
-    if (!substReqt) {
-      anySemanticChanges = true;
-      continue;
-    }
 
     // Did anything change?
     if (!anySemanticChanges &&
-        (!req.getFirstType()->isEqual(substReqt->getFirstType()) ||
+        (!req.getFirstType()->isEqual(substReqt.getFirstType()) ||
          (req.getKind() != RequirementKind::Layout &&
-          !req.getSecondType()->isEqual(substReqt->getSecondType())))) {
+          !req.getSecondType()->isEqual(substReqt.getSecondType())))) {
       anySemanticChanges = true;
     }
 
-    // Skip any erroneous requirements.
-    if (substReqt->getFirstType()->hasError() ||
-        (substReqt->getKind() != RequirementKind::Layout &&
-         substReqt->getSecondType()->hasError()))
-      continue;
-
-    requirements.push_back(*substReqt);
+    requirements.push_back(substReqt);
   }
 
   GenericSignature genericSig;
diff --git a/lib/IDE/IDETypeChecking.cpp b/lib/IDE/IDETypeChecking.cpp
@@ -323,15 +323,13 @@ struct SynthesizedExtensionAnalyzer::Implementation {
         if (!BaseType->isExistentialType()) {
           // Apply any substitutions we need to map the requirements from a
           // a protocol extension to an extension on the conforming type.
-          auto SubstReq = Req.subst(subMap);
-          if (!SubstReq) {
+          Req = Req.subst(subMap);
+          if (Req.hasError()) {
             // Substitution with interface type bases can only fail
             // if a concrete type fails to conform to a protocol.
             // In this case, just give up on the extension altogether.
             return true;
           }
-
-          Req = *SubstReq;
         }
 
         assert(!Req.getFirstType()->hasArchetype());
@@ -346,13 +344,13 @@ struct SynthesizedExtensionAnalyzer::Implementation {
             return type;
           },
           LookUpConformanceInModule(M));
-        if (!SubstReq)
+        if (SubstReq.hasError())
           return true;
 
         // FIXME: Need to handle conditional requirements here!
         ArrayRef<Requirement> conditionalRequirements;
-        if (!SubstReq->isSatisfied(conditionalRequirements)) {
-          if (!SubstReq->canBeSatisfied())
+        if (!SubstReq.isSatisfied(conditionalRequirements)) {
+          if (!SubstReq.canBeSatisfied())
             return true;
 
           MergeInfo.addRequirement(Req);
diff --git a/lib/SILOptimizer/Utils/Generics.cpp b/lib/SILOptimizer/Utils/Generics.cpp
@@ -1540,7 +1540,7 @@ void FunctionSignaturePartialSpecializer::addRequirements(
   for (auto &reqReq : Reqs) {
     LLVM_DEBUG(llvm::dbgs() << "\n\nRe-mapping the requirement:\n";
                reqReq.dump(llvm::dbgs()));
-    AllRequirements.push_back(*reqReq.subst(SubsMap));
+    AllRequirements.push_back(reqReq.subst(SubsMap));
   }
 }
 
diff --git a/lib/Sema/CodeSynthesis.cpp b/lib/Sema/CodeSynthesis.cpp
@@ -529,9 +529,10 @@ computeDesignatedInitOverrideSignature(ASTContext &ctx,
       };
 
       SmallVector<Requirement, 2> requirements;
-      for (auto reqt : superclassCtorSig.getRequirements())
-        if (auto substReqt = reqt.subst(substFn, lookupConformanceFn))
-          requirements.push_back(*substReqt);
+      for (auto reqt : superclassCtorSig.getRequirements()) {
+        auto substReqt = reqt.subst(substFn, lookupConformanceFn);
+        requirements.push_back(substReqt);
+      }
 
       // Now form the substitution map that will be used to remap parameter
       // types.
diff --git a/lib/Sema/TypeCheckGeneric.cpp b/lib/Sema/TypeCheckGeneric.cpp
@@ -925,9 +925,10 @@ CheckGenericArgumentsResult TypeChecker::checkGenericArgumentsForDiagnostics(
       Requirement substReq = req;
       if (isPrimaryReq) {
         // Primary requirements do not have substitutions applied.
-        if (auto resolved =
-                req.subst(substitutions, LookUpConformanceInModule(module))) {
-          substReq = *resolved;
+        auto resolved =
+            req.subst(substitutions, LookUpConformanceInModule(module));
+        if (!resolved.hasError()) {
+          substReq = resolved;
         } else {
           // Another requirement might fail later; just continue.
           hadSubstFailure = true;
@@ -969,9 +970,10 @@ CheckGenericArgumentsResult::Kind TypeChecker::checkGenericArguments(
   bool valid = true;
 
   for (auto req : requirements) {
-    if (auto resolved = req.subst(substitutions,
-                                  LookUpConformanceInModule(module), options)) {
-      worklist.push_back(*resolved);
+    auto resolved = req.subst(substitutions,
+                              LookUpConformanceInModule(module), options);
+    if (!resolved.hasError()) {
+      worklist.push_back(resolved);
     } else {
       valid = false;
     }
diff --git a/lib/Sema/TypeCheckProtocol.cpp b/lib/Sema/TypeCheckProtocol.cpp
@@ -391,8 +391,8 @@ matchWitnessDifferentiableAttr(DeclContext *dc, ValueDecl *req,
              witnessConfig.derivativeGenericSignature.getRequirements()) {
           auto substReq = req.subst(result.WitnessSubstitutions);
           bool reqDiffGenSigSatisfies =
-              reqDiffGenSig && substReq &&
-              reqDiffGenSig->isRequirementSatisfied(*substReq);
+              reqDiffGenSig && !substReq.hasError() &&
+              reqDiffGenSig->isRequirementSatisfied(substReq);
           bool conformanceGenSigSatisfies =
               conformanceGenSig &&
               conformanceGenSig->isRequirementSatisfied(req);
diff --git a/validation-test/IDE/crashers_fixed/rdar98565072.swift b/validation-test/IDE/crashers_fixed/rdar98565072.swift
@@ -0,0 +1,8 @@
+// RUN: %target-swift-ide-test -code-completion -code-completion-token COMPLETE -source-filename %s
+
+protocol PreferenceKey {
+    associatedtype Value
+}
+protocol View {}
+
+func propagateHeight<K: PreferenceKey>() -> some View where K.Value == #^COMPLETE^#

Original file line number	Diff line number	Diff line change
`@@ -5340,9 +5340,8 @@ ASTContext::getOverrideGenericSignature(const ValueDecl *base,`
`5340`	`5340`	`};`
`5341`	`5341`
`5342`	`5342`	`for (auto reqt : baseGenericSig.getRequirements()) {`
`5343`		`- if (auto substReqt = reqt.subst(substFn, lookupConformanceFn)) {`
`5344`		`- addedRequirements.push_back(*substReqt);`
`5345`		`- }`
	`5343`	`+ auto substReqt = reqt.subst(substFn, lookupConformanceFn);`
	`5344`	`+ addedRequirements.push_back(substReqt);`
`5346`	`5345`	`}`
`5347`	`5346`	`}`
`5348`	`5347`
Original file line number	Diff line number	Diff line change
`@@ -599,7 +599,7 @@ AbstractGenericSignatureRequestRQM::evaluate(`
`599`	`599`	`},`
`600`	`600`	`MakeAbstractConformanceForGenericType(),`
`601`	`601`	`SubstFlags::AllowLoweredTypes);`
`602`		`- resugaredRequirements.push_back(*resugaredReq);`
	`602`	`+ resugaredRequirements.push_back(resugaredReq);`
`603`	`603`	`}`
`604`	`604`
`605`	`605`	`return GenericSignatureWithError(`