AST: Change return type of Requirement::subst() to Requirement

slavapestov · slavapestov · commit 7d8f3e6b632b · 2022-08-12T14:03:57.000-04:00
Instead of returning None, let callers check hasError() if they need to.

Fixes rdar://problem/98565072.
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
@@ -5307,10 +5307,9 @@ ASTContext::getOverrideGenericSignature(const NominalTypeDecl *baseNominal,
                         baseGenericSig, derivedParams);
 
   for (auto reqt : baseGenericSig.getRequirements()) {
-    if (auto substReqt = reqt.subst(QueryOverrideSubs(info),
-                                    LookUpConformanceInOverrideSubs(info))) {
-      addedRequirements.push_back(*substReqt);
-    }
+    auto substReqt = reqt.subst(QueryOverrideSubs(info),
+                                LookUpConformanceInOverrideSubs(info));
+    addedRequirements.push_back(substReqt);
   }
 
   auto genericSig = buildGenericSignature(*this, derivedNominalSig,
diff --git a/lib/AST/ExistentialGeneralization.cpp b/lib/AST/ExistentialGeneralization.cpp
@@ -213,10 +213,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/ProtocolConformance.cpp b/lib/AST/ProtocolConformance.cpp
@@ -962,9 +962,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);
@@ -1163,7 +1163,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
@@ -179,9 +179,9 @@ RequirementEnvironment::RequirementEnvironment(
 
   if (conformanceSig) {
     for (auto &rawReq : conformanceSig.getRequirements()) {
-      if (auto req = rawReq.subst(conformanceToWitnessThunkTypeFn,
-                                  conformanceToWitnessThunkConformanceFn))
-        requirements.push_back(*req);
+      auto req = rawReq.subst(conformanceToWitnessThunkTypeFn,
+                              conformanceToWitnessThunkConformanceFn);
+      requirements.push_back(req);
     }
   }
 
@@ -205,8 +205,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(reqToWitnessThunkSigMap))
-      requirements.push_back(*req);
+    auto req = rawReq.subst(reqToWitnessThunkSigMap);
+    requirements.push_back(req);
   }
 
   witnessThunkSig = buildGenericSignature(ctx, GenericSignature(),
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
@@ -596,7 +596,7 @@ AbstractGenericSignatureRequest::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
@@ -4523,26 +4523,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
@@ -1553,7 +1553,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/TypeCheckGeneric.cpp b/lib/Sema/TypeCheckGeneric.cpp
@@ -868,9 +868,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;
@@ -912,9 +913,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
`@@ -596,7 +596,7 @@ AbstractGenericSignatureRequest::evaluate(`
`596`	`596`	`},`
`597`	`597`	`MakeAbstractConformanceForGenericType(),`
`598`	`598`	`SubstFlags::AllowLoweredTypes);`
`599`		`- resugaredRequirements.push_back(*resugaredReq);`
	`599`	`+ resugaredRequirements.push_back(resugaredReq);`
`600`	`600`	`}`
`601`	`601`
`602`	`602`	`return GenericSignatureWithError(`