Merge pull request #84556 from hamishknight/unresolve

[Sema] Remove a couple uses of UnresolvedType

Author: hamishknight

include/swift/AST/Types.h

@@ -7759,16 +7759,16 @@ class ErrorUnionType final
 };
 DEFINE_EMPTY_CAN_TYPE_WRAPPER(ErrorUnionType, Type)
 
-/// PlaceholderType - This represents a placeholder type for a type variable
-/// or dependent member type that cannot be resolved to a concrete type
-/// because the expression is ambiguous. This type is only used by the
-/// constraint solver and transformed into UnresolvedType to be used in AST.
+/// PlaceholderType - In the AST, this represents the type of a placeholder `_`.
+/// In the constraint system, this is opened into a type variable, and uses of
+/// PlaceholderType are instead used to represent holes where types cannot be
+/// inferred.
 class PlaceholderType : public TypeBase {
   // NOTE: If you add a new Type-based originator, you'll need to update the
   // recursive property logic in PlaceholderType::get.
   using Originator =
       llvm::PointerUnion<TypeVariableType *, DependentMemberType *, ErrorType *,
-                         VarDecl *, ErrorExpr *, TypeRepr *>;
+                         VarDecl *, ErrorExpr *, TypeRepr *, Pattern *>;
 
   Originator O;
 

lib/AST/ASTContext.cpp

@@ -3605,13 +3605,9 @@ static Type replacingTypeVariablesAndPlaceholders(Type ty) {
     if (!ty->hasTypeVariableOrPlaceholder())
       return ty;
 
-    // Match the logic in `Solution::simplifyType` and use UnresolvedType.
-    // FIXME: Ideally we'd get rid of UnresolvedType and just use a fresh
-    // PlaceholderType, but we don't currently support placeholders with no
-    // originators.
     auto *typePtr = ty.getPointer();
     if (isa<TypeVariableType>(typePtr) || isa<PlaceholderType>(typePtr))
-      return ctx.TheUnresolvedType;
+      return ErrorType::get(ctx);
 
     return std::nullopt;
   });
@@ -3620,7 +3616,14 @@ static Type replacingTypeVariablesAndPlaceholders(Type ty) {
 Type ErrorType::get(Type originalType) {
   // The original type is only used for printing/debugging, and we don't support
   // solver-allocated ErrorTypes. As such, fold any type variables and
-  // placeholders into UnresolvedTypes, which print as placeholders.
+  // placeholders into bare ErrorTypes, which print as placeholders.
+  //
+  // FIXME: If the originalType is itself an ErrorType we ought to be flattening
+  // it, but that's currently load-bearing as it avoids crashing for recursive
+  // generic signatures such as in `0120-rdar34184392.swift`. To fix this we
+  // ought to change the evaluator to ensure the outer step of a request cycle
+  // returns the same default value as the inner step such that we don't end up
+  // with conflicting generic signatures on encountering a cycle.
   originalType = replacingTypeVariablesAndPlaceholders(originalType);
 
   ASSERT(originalType);

lib/AST/ASTDumper.cpp

@@ -6078,6 +6078,8 @@ namespace {
         printRec(DMT, Label::always("dependent_member_type"));
       } else if (isa<TypeRepr *>(originator)) {
         printFlag("type_repr");
+      } else if (isa<Pattern *>(originator)) {
+        printFlag("pattern");
       } else {
         assert(false && "unknown originator");
       }

lib/AST/ASTPrinter.cpp

@@ -6235,6 +6235,8 @@ class TypePrinter : public TypeVisitor<TypePrinter, void, NonRecursivePrintOptio
         visit(DMT);
       } else if (isa<TypeRepr *>(originator)) {
         Printer << "type_repr";
+      } else if (isa<Pattern *>(originator)) {
+        Printer << "pattern";
       } else {
         assert(false && "unknown originator");
       }

lib/Sema/CSGen.cpp

@@ -4823,12 +4823,6 @@ generateForEachPreambleConstraints(ConstraintSystem &cs,
     return std::nullopt;
   target.setPattern(pattern);
 
-  auto contextualPattern = ContextualPattern::forRawPattern(pattern, dc);
-
-  if (TypeChecker::typeCheckPattern(contextualPattern)->hasError()) {
-    return std::nullopt;
-  }
-
   if (isa<PackExpansionExpr>(forEachExpr)) {
     auto *expansion = cast<PackExpansionExpr>(forEachExpr);
 
@@ -4996,10 +4990,6 @@ bool ConstraintSystem::generateConstraints(
           ContextualPattern::forPatternBindingDecl(patternBinding, index);
       Type patternType = TypeChecker::typeCheckPattern(contextualPattern);
 
-      // Fail early if pattern couldn't be type-checked.
-      if (!patternType || patternType->hasError())
-        return true;
-
       auto *init = patternBinding->getInit(index);
 
       if (!init && patternBinding->isDefaultInitializable(index) &&

lib/Sema/SyntacticElementTarget.cpp

@@ -36,17 +36,6 @@ SyntacticElementTarget::SyntacticElementTarget(
   assert((!contextualInfo.getType() || contextualPurpose != CTP_Unused) &&
          "Purpose for conversion type was not specified");
 
-  // Take a look at the conversion type to check to make sure it is sensible.
-  if (auto type = contextualInfo.getType()) {
-    // If we're asked to convert to an UnresolvedType, then ignore the request.
-    // This happens when CSDiags nukes a type.
-    if (type->is<UnresolvedType>() ||
-        (type->is<MetatypeType>() && type->hasUnresolvedType())) {
-      contextualInfo.typeLoc = TypeLoc();
-      contextualPurpose = CTP_Unused;
-    }
-  }
-
   kind = Kind::expression;
   expression.expression = expr;
   expression.dc = dc;
@@ -141,7 +130,7 @@ SyntacticElementTarget::forInitialization(Expr *initializer, DeclContext *dc,
   // Determine the contextual type for the initialization.
   TypeLoc contextualType;
   if (!(isa<OptionalSomePattern>(pattern) && !pattern->isImplicit()) &&
-      patternType && !patternType->is<UnresolvedType>()) {
+      patternType && !patternType->is<PlaceholderType>()) {
     contextualType = TypeLoc::withoutLoc(patternType);
 
     // Only provide a TypeLoc if it makes sense to allow diagnostics.

lib/Sema/TypeCheckPattern.cpp

@@ -866,7 +866,7 @@ Type PatternTypeRequest::evaluate(Evaluator &evaluator,
     // If we're type checking this pattern in a context that can provide type
     // information, then the lack of type information is not an error.
     if (options & TypeResolutionFlags::AllowUnspecifiedTypes)
-      return Context.TheUnresolvedType;
+      return PlaceholderType::get(Context, P);
 
     Context.Diags.diagnose(P->getLoc(), diag::cannot_infer_type_for_pattern);
     if (auto named = dyn_cast<NamedPattern>(P)) {
@@ -946,7 +946,7 @@ Type PatternTypeRequest::evaluate(Evaluator &evaluator,
       return ErrorType::get(Context);
     }
 
-    return Context.TheUnresolvedType;
+    return PlaceholderType::get(Context, P);
   }
   llvm_unreachable("bad pattern kind!");
 }
@@ -1736,41 +1736,18 @@ Pattern *TypeChecker::coercePatternToType(
 /// contextual type.
 void TypeChecker::coerceParameterListToType(ParameterList *P,
                                             AnyFunctionType *FN) {
-
-  // Local function to check if the given type is valid e.g. doesn't have
-  // errors, type variables or unresolved types related to it.
-  auto isValidType = [](Type type) -> bool {
-    return !(type->hasError() || type->hasUnresolvedType());
-  };
-
-  // Local function to check whether type of given parameter
-  // should be coerced to a given contextual type or not.
-  auto shouldOverwriteParam = [&](ParamDecl *param) -> bool {
-    return !isValidType(param->getTypeInContext());
-  };
-
-  auto handleParameter = [&](ParamDecl *param, Type ty, bool forceMutable) {
-    if (forceMutable)
-      param->setSpecifier(ParamDecl::Specifier::InOut);
-
-    // If contextual type is invalid and we have a valid argument type
-    // trying to coerce argument to contextual type would mean erasing
-    // valuable diagnostic information.
-    if (isValidType(ty) || shouldOverwriteParam(param)) {
-      param->setInterfaceType(ty->mapTypeOutOfContext());
-    }
-  };
-
   // Coerce each parameter to the respective type.
   ArrayRef<AnyFunctionType::Param> params = FN->getParams();
   for (unsigned i = 0, e = P->size(); i != e; ++i) {
     auto &param = P->get(i);
     assert(param->getArgumentName().empty() &&
            "Closures cannot have API names");
-    
-    handleParameter(param,
-                    params[i].getParameterType(),
-                    params[i].isInOut());
     assert(!param->isDefaultArgument() && "Closures cannot have default args");
+
+    if (params[i].isInOut())
+      param->setSpecifier(ParamDecl::Specifier::InOut);
+
+    param->setInterfaceType(
+        params[i].getParameterType()->mapTypeOutOfContext());
   }
 }

lib/Sema/TypeCheckStmt.cpp

@@ -905,10 +905,6 @@ typeCheckPatternBindingStmtConditionElement(StmtConditionElement &elt,
   // provide type information.
   auto contextualPattern = ContextualPattern::forRawPattern(pattern, dc);
   Type patternType = TypeChecker::typeCheckPattern(contextualPattern);
-  if (patternType->hasError()) {
-    typeCheckPatternFailed();
-    return true;
-  }
 
   // If the pattern didn't get a type, it's because we ran into some
   // unknown types along the way. We'll need to check the initializer.

lib/Sema/TypeCheckStorage.cpp

@@ -451,12 +451,6 @@ const PatternBindingEntry *PatternBindingEntryRequest::evaluate(
   auto contextualPattern =
       ContextualPattern::forPatternBindingDecl(binding, entryNumber);
   Type patternType = TypeChecker::typeCheckPattern(contextualPattern);
-  if (patternType->hasError()) {
-    swift::setBoundVarsTypeError(pattern, Context);
-    binding->setInvalid();
-    pattern->setType(ErrorType::get(Context));
-    return &pbe;
-  }
 
   llvm::SmallVector<VarDecl *, 2> vars;
   binding->getPattern(entryNumber)->collectVariables(vars);
@@ -516,9 +510,7 @@ const PatternBindingEntry *PatternBindingEntryRequest::evaluate(
 
   // If the pattern contains some form of unresolved type, we'll need to
   // check the initializer.
-  if (patternType->hasUnresolvedType() ||
-      patternType->hasPlaceholder() ||
-      patternType->hasUnboundGenericType()) {
+  if (patternType->hasPlaceholder() || patternType->hasUnboundGenericType()) {
     if (TypeChecker::typeCheckPatternBinding(binding, entryNumber,
                                              patternType)) {
       binding->setInvalid();

lib/Sema/TypeChecker.h

@@ -753,8 +753,10 @@ Pattern *resolvePattern(Pattern *P, DeclContext *dc, bool isStmtCondition);
 ///
 /// \returns the type of the pattern, which may be an error type if an
 /// unrecoverable error occurred. If the options permit it, the type may
-/// involve \c UnresolvedType (for patterns with no type information) and
+/// involve \c PlaceholderType (for patterns with no type information) and
 /// unbound generic types.
+/// TODO: We ought to expose hooks that let callers open the
+/// PlaceholderTypes directly, similar to type resolution.
 Type typeCheckPattern(ContextualPattern pattern);
 
 /// Attempt to simplify an ExprPattern into a BoolPattern or