Skip to content

[CSSolver] Fix a crash in diagnostics related to pattern matching #65366

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 4 commits into from
Apr 24, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
7 changes: 7 additions & 0 deletions include/swift/Sema/ConstraintSystem.h
Original file line number Diff line number Diff line change
Expand Up @@ -3337,6 +3337,13 @@ class ConstraintSystem {
void recordPotentialHole(TypeVariableType *typeVar);
void recordAnyTypeVarAsPotentialHole(Type type);

/// Record all unbound type variables that occur in the given type
/// as being bound to "hole" type represented by \c PlaceholderType
/// in this constraint system.
///
/// \param type The type on which to holeify.
void recordTypeVariablesAsHoles(Type type);

void recordMatchCallArgumentResult(ConstraintLocator *locator,
MatchCallArgumentResult result);

Expand Down
8 changes: 1 addition & 7 deletions lib/Sema/BuilderTransform.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -1174,13 +1174,7 @@ ConstraintSystem::matchResultBuilder(AnyFunctionRef fn, Type builderType,

if (auto *closure =
getAsExpr<ClosureExpr>(fn.getAbstractClosureExpr())) {
auto closureTy = getClosureType(closure);
simplifyType(closureTy).visit([&](Type componentTy) {
if (auto *typeVar = componentTy->getAs<TypeVariableType>()) {
assignFixedType(typeVar,
PlaceholderType::get(getASTContext(), typeVar));
}
});
recordTypeVariablesAsHoles(getClosureType(closure));
}

return getTypeMatchSuccess();
Expand Down
9 changes: 5 additions & 4 deletions lib/Sema/CSGen.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -2768,8 +2768,9 @@ namespace {
// correct handling of patterns like `_ as Foo` where `_` would
// get a type of `Foo` but `is` pattern enclosing it could still be
// inferred from enclosing context.
auto isType = CS.createTypeVariable(CS.getConstraintLocator(pattern),
TVO_CanBindToNoEscape);
auto isType =
CS.createTypeVariable(CS.getConstraintLocator(pattern),
TVO_CanBindToNoEscape | TVO_CanBindToHole);
CS.addConstraint(
ConstraintKind::Conversion, subPatternType, isType,
locator.withPathElement(LocatorPathElt::PatternMatch(pattern)));
Expand Down Expand Up @@ -2919,8 +2920,8 @@ namespace {
// TODO: we could try harder here, e.g. for enum elements to provide the
// enum type.
return setType(
CS.createTypeVariable(
CS.getConstraintLocator(locator), TVO_CanBindToNoEscape));
CS.createTypeVariable(CS.getConstraintLocator(locator),
TVO_CanBindToNoEscape | TVO_CanBindToHole));
}

llvm_unreachable("Unhandled pattern kind");
Expand Down
28 changes: 19 additions & 9 deletions lib/Sema/CSSimplify.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -6159,12 +6159,7 @@ bool ConstraintSystem::repairFailures(
// unrelated fixes, let's proactively bind all of the pattern
// elemnts to holes here.
if (lhs->isAny()) {
rhs.visit([&](Type type) {
if (auto *typeVar = type->getAs<TypeVariableType>()) {
assignFixedType(typeVar,
PlaceholderType::get(getASTContext(), typeVar));
}
});
recordTypeVariablesAsHoles(rhs);
}

conversionsOrFixes.push_back(CollectionElementContextualMismatch::create(
Expand Down Expand Up @@ -10402,8 +10397,7 @@ ConstraintSystem::SolutionKind ConstraintSystem::simplifyMemberConstraint(
// equality instead of argument application constraint, so allowing
// them to bind member could mean missing valid hole positions in
// the pattern.
assignFixedType(memberTypeVar, PlaceholderType::get(getASTContext(),
memberTypeVar));
recordTypeVariablesAsHoles(memberTypeVar);
} else {
recordPotentialHole(memberTypeVar);
}
Expand Down Expand Up @@ -14008,8 +14002,24 @@ void ConstraintSystem::recordAnyTypeVarAsPotentialHole(Type type) {
return;

type.visit([&](Type type) {
if (auto *typeVar = type->getAs<TypeVariableType>())
if (auto *typeVar = type->getAs<TypeVariableType>()) {
typeVar->getImpl().enableCanBindToHole(getSavedBindings());
}
});
}

void ConstraintSystem::recordTypeVariablesAsHoles(Type type) {
type.visit([&](Type componentTy) {
if (auto *typeVar = componentTy->getAs<TypeVariableType>()) {
// Ignore bound type variables. This can happen if a type variable
// occurs in multiple positions and/or if type hasn't been fully
// simplified before this call.
if (typeVar->getImpl().hasRepresentativeOrFixed())
return;

assignFixedType(typeVar,
PlaceholderType::get(getASTContext(), typeVar));
}
});
}

Expand Down
12 changes: 2 additions & 10 deletions lib/Sema/CSStep.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -1103,13 +1103,6 @@ void ConjunctionStep::SolverSnapshot::applySolution(const Solution &solution) {
if (score.Data[SK_Fix] == 0)
return;

auto holeify = [&](Type componentTy) {
if (auto *typeVar = componentTy->getAs<TypeVariableType>()) {
CS.assignFixedType(
typeVar, PlaceholderType::get(CS.getASTContext(), typeVar));
}
};

// If this conjunction represents a closure and inference
// has failed, let's bind all of unresolved type variables
// in its interface type to holes to avoid extraneous
Expand All @@ -1118,14 +1111,13 @@ void ConjunctionStep::SolverSnapshot::applySolution(const Solution &solution) {
if (locator->directlyAt<ClosureExpr>()) {
auto closureTy =
CS.getClosureType(castToExpr<ClosureExpr>(locator->getAnchor()));

CS.simplifyType(closureTy).visit(holeify);
CS.recordTypeVariablesAsHoles(closureTy);
}

// Same for a SingleValueStmtExpr, turn any unresolved type variables present
// in its type into holes.
if (locator->isForSingleValueStmtConjunction()) {
auto *SVE = castToExpr<SingleValueStmtExpr>(locator->getAnchor());
CS.simplifyType(CS.getType(SVE)).visit(holeify);
CS.recordTypeVariablesAsHoles(CS.getType(SVE));
}
}
3 changes: 2 additions & 1 deletion test/expr/closure/multi_statement.swift
Original file line number Diff line number Diff line change
Expand Up @@ -336,9 +336,10 @@ func test_unknown_refs_in_tilde_operator() {
enum E {
}

let _: (E) -> Void = { // expected-error {{unable to infer closure type in the current context}}
let _: (E) -> Void = {
if case .test(unknown) = $0 {
// expected-error@-1 2 {{cannot find 'unknown' in scope}}
// expected-error@-2 {{type 'E' has no member 'test'}}
}
}
}
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,22 @@
// RUN: %target-typecheck-verify-swift

var a: Any?

let _: () -> Void = {
for case (is Int)? in [a] {}
if case (is Int, is Int) = a {} // expected-error {{cannot convert value of type 'Any?' to specified type '(_, _)'}}
}

let _: () -> Void = {
for case (0)? in [a] {}
if case (0, 0) = a {}
// expected-error@-1 {{cannot convert value of type 'Any?' to specified type '(_, _)}}
}

let _: () -> Void = {
for case (0)? in [a] {}
for case (0, 0) in [a] {}
// expected-error@-1 {{conflicting arguments to generic parameter 'Elements' ('[Any]' vs. '[Any?]')}}
// expected-error@-2 {{conflicting arguments to generic parameter 'Element' ('Any' vs. 'Any?')}}
// expected-error@-3 {{conflicting arguments to generic parameter 'Self' ('[Any]' vs. '[Any?]')}}
}
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@ func test(result: MyEnum, optResult: MyEnum?) {
}

if let .co(42) = result { // expected-error {{pattern matching in a condition requires the 'case' keyword}}
// expected-error@-1 {{type of expression is ambiguous without more context}}
// expected-error@-1 {{type 'MyEnum' has no member 'co'}}
}

if let .co = optResult { // expected-error {{pattern matching in a condition requires the 'case' keyword}}
Expand Down