Skip to content

Conversation

@hamishknight
Copy link
Contributor

Previously we would skip type-checking the result expression of a return or the initialization expression of a binding if the contextual type had an error, but that misses out on useful diagnostics and prevents code completion and cursor info from working. Change the logic such that we open ErrorTypes as holes and continue to type-check.

If we have a hole binding for the result of a closure, avoid
introducing Void as an additional binding since that'll just cause
local solution ambiguities. This doesn't affect regular type-checking
since we end up treating such ambiguities as "identical", so we just
pick one. But for completion, we don't do such filtering so need to
make sure we end up with the hole solution to avoid treating Void as
the contextual type for cases such as:

```
_ = {
    x.#^CC^#
}
```

Currently we avoid this by not handling holes in `repairFailures`
for closure body locators, but I'm changing that in the next commit.
Eagerly bind invalid type references to holes and propagate contextual
type holes in `repairFailures`. This avoids some unnecessary diagnostics
in cases where we have an invalid contextual type.
Previously we would skip type-checking the result expression of a
`return` or the initialization expression of a binding if the contextual
type had an error, but that misses out on useful diagnostics and
prevents code completion and cursor info from working. Change the logic
such that we open ErrorTypes as holes and continue to type-check.
@hamishknight
Copy link
Contributor Author

@swift-ci please test

@hamishknight
Copy link
Contributor Author

@swift-ci please SourceKit stress test

Copy link
Contributor

@xedin xedin left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good to me! I left a few comments inline about use of the temporary type variables.

// become holes.
auto *tv = CS.createTypeVariable(CS.getConstraintLocator(repr),
TVO_CanBindToHole);
CS.recordTypeVariablesAsHoles(tv);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

TypeRepr is an originator for a placeholder type, could we avoid allocating a type variable here?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hmm yeah, let me look into that in a follow-up

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

// For ErrorTypes we want to eagerly bind to a hole since we
// know this is where the issue is.
if (isa<ErrorType>(type.getPointer())) {
recordTypeVariablesAsHoles(tv);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It's nice that placeholder type could capture a type variable with its location but I'm wondering whether we use that information or maybe we should just let ErrorType be an originator for a placeholder and avoid having to allocate additional type variables here to immediately bind them?...

@hamishknight
Copy link
Contributor Author

@swift-ci please smoke test macOS

@hamishknight hamishknight enabled auto-merge August 30, 2025 08:56
@hamishknight hamishknight merged commit eeb0675 into swiftlang:main Aug 30, 2025
5 of 6 checks passed
@hamishknight hamishknight deleted the minesweeper branch August 30, 2025 13:08
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants