From 1c4132f00574d6333f39e24060d49c268a5bc584 Mon Sep 17 00:00:00 2001 From: Hamish Knight Date: Sun, 28 Sep 2025 14:57:52 +0100 Subject: [PATCH 1/4] [AST] Remove the use of UnresolvedType in `replacingTypeVariablesAndPlaceholders` Now that ErrorType prints as `_`, we can use that instead of UnresolvedType here since the original type is only really used for type printing and debugging. --- lib/AST/ASTContext.cpp | 15 +++++++++------ 1 file changed, 9 insertions(+), 6 deletions(-) diff --git a/lib/AST/ASTContext.cpp b/lib/AST/ASTContext.cpp index ac76b08bc59b1..4a1d8b9be6db0 100644 --- a/lib/AST/ASTContext.cpp +++ b/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(typePtr) || isa(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); From 0c58138a4210b026a66be30d5c6d7412783821e8 Mon Sep 17 00:00:00 2001 From: Hamish Knight Date: Sun, 28 Sep 2025 14:57:52 +0100 Subject: [PATCH 2/4] [Sema] Avoid using UnresolvedType in `PatternTypeRequest` Return PlaceholderType instead of UnresolvedType, meaning we now treat the following cases the same: ``` let x1 = foo let x2: _ = foo ``` --- include/swift/AST/Types.h | 10 +++++----- lib/AST/ASTDumper.cpp | 2 ++ lib/AST/ASTPrinter.cpp | 2 ++ lib/Sema/SyntacticElementTarget.cpp | 2 +- lib/Sema/TypeCheckPattern.cpp | 4 ++-- lib/Sema/TypeCheckStorage.cpp | 4 +--- lib/Sema/TypeChecker.h | 4 +++- test/Sema/placeholder_type.swift | 2 +- 8 files changed, 17 insertions(+), 13 deletions(-) diff --git a/include/swift/AST/Types.h b/include/swift/AST/Types.h index eee1dea674fc4..6b257866dc283 100644 --- a/include/swift/AST/Types.h +++ b/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; + VarDecl *, ErrorExpr *, TypeRepr *, Pattern *>; Originator O; diff --git a/lib/AST/ASTDumper.cpp b/lib/AST/ASTDumper.cpp index 2cf34f15ce68d..ca59d0f067ce7 100644 --- a/lib/AST/ASTDumper.cpp +++ b/lib/AST/ASTDumper.cpp @@ -6078,6 +6078,8 @@ namespace { printRec(DMT, Label::always("dependent_member_type")); } else if (isa(originator)) { printFlag("type_repr"); + } else if (isa(originator)) { + printFlag("pattern"); } else { assert(false && "unknown originator"); } diff --git a/lib/AST/ASTPrinter.cpp b/lib/AST/ASTPrinter.cpp index c420f8c692087..7ac33f49a4135 100644 --- a/lib/AST/ASTPrinter.cpp +++ b/lib/AST/ASTPrinter.cpp @@ -6235,6 +6235,8 @@ class TypePrinter : public TypeVisitor(originator)) { Printer << "type_repr"; + } else if (isa(originator)) { + Printer << "pattern"; } else { assert(false && "unknown originator"); } diff --git a/lib/Sema/SyntacticElementTarget.cpp b/lib/Sema/SyntacticElementTarget.cpp index d86a4861a3d26..fc6a522fdce76 100644 --- a/lib/Sema/SyntacticElementTarget.cpp +++ b/lib/Sema/SyntacticElementTarget.cpp @@ -141,7 +141,7 @@ SyntacticElementTarget::forInitialization(Expr *initializer, DeclContext *dc, // Determine the contextual type for the initialization. TypeLoc contextualType; if (!(isa(pattern) && !pattern->isImplicit()) && - patternType && !patternType->is()) { + patternType && !patternType->is()) { contextualType = TypeLoc::withoutLoc(patternType); // Only provide a TypeLoc if it makes sense to allow diagnostics. diff --git a/lib/Sema/TypeCheckPattern.cpp b/lib/Sema/TypeCheckPattern.cpp index 52d8705d7b072..b7236fcfc6d06 100644 --- a/lib/Sema/TypeCheckPattern.cpp +++ b/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(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!"); } diff --git a/lib/Sema/TypeCheckStorage.cpp b/lib/Sema/TypeCheckStorage.cpp index ca44b4a00f594..c4230edb0bdba 100644 --- a/lib/Sema/TypeCheckStorage.cpp +++ b/lib/Sema/TypeCheckStorage.cpp @@ -516,9 +516,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(); diff --git a/lib/Sema/TypeChecker.h b/lib/Sema/TypeChecker.h index f176242a32f60..e6808789e603b 100644 --- a/lib/Sema/TypeChecker.h +++ b/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 diff --git a/test/Sema/placeholder_type.swift b/test/Sema/placeholder_type.swift index f7207c036cb4a..99fc6930f87a8 100644 --- a/test/Sema/placeholder_type.swift +++ b/test/Sema/placeholder_type.swift @@ -13,7 +13,7 @@ let arr = [_](repeating: "hi", count: 3) func foo(_ arr: [_] = [0]) {} // expected-error {{type placeholder not allowed here}} let foo = _.foo // expected-error {{type placeholder not allowed here}} -let zero: _ = .zero // expected-error {{cannot infer contextual base in reference to member 'zero'}} +let zero: _ = .zero // expected-error {{reference to member 'zero' cannot be resolved without a contextual type}} struct S { var x: T From 2c0ea134a918307dcbec98d58f7db2934187c004 Mon Sep 17 00:00:00 2001 From: Hamish Knight Date: Sun, 28 Sep 2025 14:57:52 +0100 Subject: [PATCH 3/4] [Sema] Remove some `hasError` checks for `typeCheckPattern` These shouldn't be necessary anymore, we should be able to handle ErrorType patterns. --- lib/Sema/CSGen.cpp | 10 ---------- lib/Sema/TypeCheckStmt.cpp | 4 ---- lib/Sema/TypeCheckStorage.cpp | 6 ------ test/Constraints/generics.swift | 4 +++- test/Constraints/patterns.swift | 3 +++ 5 files changed, 6 insertions(+), 21 deletions(-) diff --git a/lib/Sema/CSGen.cpp b/lib/Sema/CSGen.cpp index 56ce4b28d47e9..dab7f314720de 100644 --- a/lib/Sema/CSGen.cpp +++ b/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(forEachExpr)) { auto *expansion = cast(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) && diff --git a/lib/Sema/TypeCheckStmt.cpp b/lib/Sema/TypeCheckStmt.cpp index 2e99295f2be8e..b1fe8cf1e426c 100644 --- a/lib/Sema/TypeCheckStmt.cpp +++ b/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. diff --git a/lib/Sema/TypeCheckStorage.cpp b/lib/Sema/TypeCheckStorage.cpp index c4230edb0bdba..411582b6f09b8 100644 --- a/lib/Sema/TypeCheckStorage.cpp +++ b/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 vars; binding->getPattern(entryNumber)->collectVariables(vars); diff --git a/test/Constraints/generics.swift b/test/Constraints/generics.swift index 17e5aa24d879f..daf7bdfcd1392 100644 --- a/test/Constraints/generics.swift +++ b/test/Constraints/generics.swift @@ -472,7 +472,9 @@ do { } class testStdlibType { - let _: Array // expected-error {{reference to generic type 'Array' requires arguments in <...>}} {{15-15=}} + let _: Array + // expected-error@-1 {{reference to generic type 'Array' requires arguments in <...>}} {{15-15=}} + // expected-error@-2 {{property declaration does not bind any variables}} } // rdar://problem/32697033 diff --git a/test/Constraints/patterns.swift b/test/Constraints/patterns.swift index bb7fb24e5a0ec..0f6ccedc781a0 100644 --- a/test/Constraints/patterns.swift +++ b/test/Constraints/patterns.swift @@ -840,4 +840,7 @@ func testUndefinedInClosureVar() { _ = { var x: Undefined // expected-error {{cannot find type 'Undefined' in scope}} } + _ = { + for x: Undefined in [0] {} // expected-error {{cannot find type 'Undefined' in scope}} + } } From fb5e356f9ec3681335c0124e03c856a4037afee6 Mon Sep 17 00:00:00 2001 From: Hamish Knight Date: Sun, 28 Sep 2025 14:57:52 +0100 Subject: [PATCH 4/4] [Sema] Remove a couple other uses of `hasError` and `hasUnresolvedType` These cases should no longer be reachable. --- lib/Sema/SyntacticElementTarget.cpp | 11 --------- lib/Sema/TypeCheckPattern.cpp | 35 +++++------------------------ 2 files changed, 6 insertions(+), 40 deletions(-) diff --git a/lib/Sema/SyntacticElementTarget.cpp b/lib/Sema/SyntacticElementTarget.cpp index fc6a522fdce76..1984908b9f2d9 100644 --- a/lib/Sema/SyntacticElementTarget.cpp +++ b/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() || - (type->is() && type->hasUnresolvedType())) { - contextualInfo.typeLoc = TypeLoc(); - contextualPurpose = CTP_Unused; - } - } - kind = Kind::expression; expression.expression = expr; expression.dc = dc; diff --git a/lib/Sema/TypeCheckPattern.cpp b/lib/Sema/TypeCheckPattern.cpp index b7236fcfc6d06..11711db491d1f 100644 --- a/lib/Sema/TypeCheckPattern.cpp +++ b/lib/Sema/TypeCheckPattern.cpp @@ -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 params = FN->getParams(); for (unsigned i = 0, e = P->size(); i != e; ++i) { auto ¶m = 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()); } }