Skip to content

[AST] Replace type variables and placeholders in original ErrorTypes #82292

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 1 commit into from
Jun 20, 2025

Conversation

hamishknight
Copy link
Contributor

Turns out we can also get solver-allocated original ErrorTypes through type resolution. Given the original type is only used for printing/debugging, let's just fold away any type variables and placeholders into UnresolvedType (which print as placeholders). This matches what Solution::simplifyType does.

Turns out we can also get solver-allocated original ErrorTypes through
type resolution. Given the original type is only used for
printing/debugging, let's just fold away any type variables and
placeholders into UnresolvedType (which print as placeholders). This
matches what `Solution::simplifyType` does.
@hamishknight
Copy link
Contributor Author

@swift-ci please test

// 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.
Copy link
Contributor

Choose a reason for hiding this comment

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

Just to note that this is indeed the long-term plan, we need to remove UnresolvedType in favor of PlaceholderType

Copy link
Contributor

Choose a reason for hiding this comment

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

Maybe we should limit this transformation to solver allocated placeholder types instead of all of them. We could also expand ErrorExpr originator and replace type variables with placeholders that point to ASTNode that their locator simplifies down to...

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Maybe we should limit this transformation to solver allocated placeholder types instead of all of them

I would rather apply the transform consistently to match what we do in Solution::simplifyType, conditionalizing the logic on the allocation arena or originator seems a little odd to me. We should revisit this when we get rid of UnresolvedType though.

We could also expand ErrorExpr originator and replace type variables with placeholders that point to ASTNode that their locator simplifies down to...

Yeah maybe, I don't think there are any ErrorType consumers that currently really care about originators, but maybe that could be useful for debugging?

Copy link
Contributor

Choose a reason for hiding this comment

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

Yeah, I was mostly talking long-term as part of UnresolvedType removal :)

@hamishknight hamishknight merged commit 3c8a8a8 into swiftlang:main Jun 20, 2025
5 checks passed
@hamishknight hamishknight deleted the bigger-hammer branch June 20, 2025 09:59
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