Skip to content

Improve error message for ambiguous numeric types in closure parameters #147577

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.

Sign up for GitHub

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 3 commits into from
Oct 20, 2025
+97 −0
Merged

Improve error message for ambiguous numeric types in closure parameters #147577

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
0eaa336
Improve error message for ambiguous numeric types in closure parameters
JohnTitor Oct 11, 2025
dbc7327
Fix suggestions for nested refs and muts
JohnTitor Oct 11, 2025
632e759
Add FIXME about FileName check's fragility
JohnTitor Oct 11, 2025
File filter

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Next Next commit
Improve error message for ambiguous numeric types in closure parameters
  • Loading branch information
@JohnTitor
JohnTitor committed Oct 11, 2025
commit 0eaa3366f904e9fde3bd3ada9d2642f23140577d
16 changes: 16 additions & 0 deletions compiler/rustc_hir_typeck/src/method/suggest.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2568,6 +2568,22 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
Applicability::MaybeIncorrect,
);
}
// For closure parameters with reference patterns (e.g., |&v|), suggest the type annotation
// on the pattern itself, e.g., |&v: &i32|
(FileName::Real(_), Node::Pat(pat))
Copy link
Member

@fmease fmease Oct 11, 2025 •
edited
Loading

Choose a reason for hiding this comment

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

(preexisting) Matching on the file name presumably to suppress this suggestion if this comes from a macro expansion (FileName::MacroExpansion) is wild and simply incorrect, lol. E.g., it means it never suggests anything if the source is STDIN (FileName::Anon) like for printf '/* ... */' | rustc -.
Copy link
Member Author

@JohnTitor JohnTitor Oct 11, 2025

Choose a reason for hiding this comment

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

Makes sense, but addressing it in this PR feels like out-of-scope, so added FIXME: 632e759 (#147577)

Or do you want me to resolve it here?
Copy link
Member

@fmease fmease Oct 11, 2025

Choose a reason for hiding this comment

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

Or do you want me to resolve it here?

No, a FIXME is fine 👍
Copy link
Member

@davidtwco davidtwco Oct 13, 2025

Choose a reason for hiding this comment

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

Could we just check span.from_expansion() and fix the use of filename here in this PR? Doesn't seem like it would be a big change?
Copy link
Member

@fmease fmease Oct 13, 2025 •
edited
Loading

Choose a reason for hiding this comment

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

Note that suppressing if .from_expansion() holds is always a sledgehammer solution, it suppresses the suggestion in perfectly fine cases like the one below:

fn main() { macro_rules! wrap { () => { let x = 0; x.pow(0); } } wrap!(); }

What you often actually want is comparing syntax contexts (e.g., eq_ctxt) or adjusting the span (e.g., find_*ancestor*) to avoid suggesting garbage when macro meta-vars $x and/or macro calls m!() are contained anywhere (but don't span the entire thing) which is the real issue, not the expansion itself in most cases.

Tho that often requires a bit of finesse to get right, so from_expansion is an okay stopgap.
Copy link
Member Author

@JohnTitor JohnTitor Oct 20, 2025

Choose a reason for hiding this comment

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

Thinking about this but I'm going to keep it as-is for now because either solution (current vs. .from_expansion()) has a downside and the former wouldn't break the current at least. As León said, implementing more complete way would be needed but I'd like to resolve it in another PR to keep this PR's purpose simple.
if let Node::Pat(binding_pat) = self.tcx.hir_node(hir_id)
&& let hir::PatKind::Binding(..) = binding_pat.kind
&& let Node::Pat(parent_pat) = parent_node
&& matches!(parent_pat.kind, hir::PatKind::Ref(..)) =>
Comment on lines +2576 to +2579
Copy link
Member

@fmease fmease Oct 11, 2025 •
edited
Loading

Choose a reason for hiding this comment

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

Food for thought (I don't have time to investigate this): If the unsolved inference variable is not an integer/float one ({integer} / {float}) but a normal one, then need_type_info / InferSourceKind is able to figure out the type annotation for various kinds of patterns.

E.g.,

[].into_iter().filter(|&x| x.pow(0)); // suggests `: &T` on master
[].into_iter().filter(|(x,)| x.pow(0)); // suggests `: &(T,)` on master

So I guess ideally we would be modifying needs_type_info to properly deal with integer/float ty vars since that seems to have all the infrastructure in place already (unless they really follow a different code path in which case my suggestion would be nonsensical).
{
err.span_label(span, "you must specify a type for this binding");
err.span_suggestion_verbose(
pat.span.shrink_to_hi(),
"specify the type in the closure argument list",
format!(": &{concrete_type}"),
Copy link
Member

@fmease fmease Oct 11, 2025

Choose a reason for hiding this comment

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

This also suggests & for PatKind::Ref(_, Mut) when it should probably suggest &mut

Copy link
Member

@fmease fmease Oct 11, 2025 •
edited
Loading

Choose a reason for hiding this comment

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

This doesn't account for nested refs like &&x (should suggest : &&T not : &T)
Copy link
Member Author

@JohnTitor JohnTitor Oct 11, 2025

Choose a reason for hiding this comment

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

Good point! Added a check in dbc7327 (#147577)
(I cannot come up with a mutable test case which is caught by E0689 🤔)
Copy link
Member

@fmease fmease Oct 11, 2025

Choose a reason for hiding this comment

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

I cannot come up with a mutable test case which is caught by E0689

[&mut 0].into_iter().map(|&mut x| x.pow(0))
Copy link
Member Author

@JohnTitor JohnTitor Oct 11, 2025 •
edited
Loading

Choose a reason for hiding this comment

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

I think it isn't caught by E0689 but other codes like E0282 (I guess other inference errors are stronger here), so the suggestion here wouldn't be showed.
Applicability::MaybeIncorrect,
);
}
_ => {
err.span_label(span, msg);
}
Expand Down
9 changes: 9 additions & 0 deletions tests/ui/inference/ambiguous-numeric-in-closure-ref.fixed
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,9 @@
// Test for better error message when numeric type is ambiguous in closure parameter with reference

//@ run-rustfix

fn main() {
let _ = (0..10).filter(|&v: &i32| v.pow(2) > 0);
//~^ ERROR can't call method `pow` on ambiguous numeric type `{integer}`
//~| SUGGESTION &i32
}
9 changes: 9 additions & 0 deletions tests/ui/inference/ambiguous-numeric-in-closure-ref.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,9 @@
// Test for better error message when numeric type is ambiguous in closure parameter with reference

//@ run-rustfix

fn main() {
let _ = (0..10).filter(|&v| v.pow(2) > 0);
//~^ ERROR can't call method `pow` on ambiguous numeric type `{integer}`
//~| SUGGESTION &i32
}
16 changes: 16 additions & 0 deletions tests/ui/inference/ambiguous-numeric-in-closure-ref.stderr
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,16 @@
error[E0689]: can't call method `pow` on ambiguous numeric type `{integer}`
--> $DIR/ambiguous-numeric-in-closure-ref.rs:6:35
|
LL | let _ = (0..10).filter(|&v| v.pow(2) > 0);
| - ^^^
| |
| you must specify a type for this binding
|
help: specify the type in the closure argument list
|
LL | let _ = (0..10).filter(|&v: &i32| v.pow(2) > 0);
| ++++++

error: aborting due to 1 previous error

For more information about this error, try `rustc --explain E0689`.