Skip to content

Conversation

xizheyin
Copy link
Member

@xizheyin xizheyin commented Aug 21, 2025

Fixes #145634

In this example, &Some((1, 2)) is an &_, and we cannot know the exact type, so I think suppressing this suggestion is the most appropriate choice here. This is similar to / inspired by #145361.

I expanded TypeFlags to indicate whether a type contains a type with is_ty_var true.

The test with regression(#3680, #5358, #12552) are all ICE, and some are even from many years ago. Therefore, this change in PR should not have a significant impact on them?

Two commits shows the diff of the newly added test.

r? compiler

@rustbot rustbot added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. labels Aug 21, 2025
@rustbot
Copy link
Collaborator

rustbot commented Aug 21, 2025

This PR modifies tests/ui/issues/. If this PR is adding new tests to tests/ui/issues/,
please refrain from doing so, and instead add it to more descriptive subdirectories.

Comment on lines +94 to +95
// when found is unresolved var, we can't suggest anything
if exp_found.found.has_type_flags(TypeFlags::HAS_INFER_TY_VAR) {
Copy link
Contributor

@dianne dianne Aug 22, 2025

Choose a reason for hiding this comment

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

If you're checking for inference variables on the error path, would it make sense to do so in a way that doesn't require tracking them for all types on the happy path too?

I think the "found" type may also be very likely to have inference variables when it doesn't match the expected type, on account of how pattern type-checking works. The type of the constructor pattern is checked against the type of the scrutinee, without knowledge of what the constructor's type's generic arguments are; I think those are all left for type inference to handle. As such, I think something more targeted may be needed if the suggestion is to support types with generic arguments.

@fee1-dead
Copy link
Member

it seems like if the scrutinee isn't a reference while the pattern clearly is, we could try suggesting removing the & from the pattern

@xizheyin
Copy link
Member Author

@rustbot author

@rustbot rustbot added S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Aug 24, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

rustc recommends wrapping in Some instead of removing extra reference
4 participants