On type mismatch involving fn/method call, point at definition #119340
Conversation
|
r? @cjgillot (rustbot has picked a reviewer for you, use r? to override) |
rustbot
added
S-waiting-on-review
| @@ -21,6 +21,8 @@ LL | fn f(x: &mut dyn Iterator<Item: Iterator<Item = &'_ ()>>) -> Option<&'_ ()> | |||
| | | |||
| = note: expected enum `Option<&()>` | |||
| found enum `Option<impl Iterator<Item = &'_ ()>>` | |||
| note: the method next is defined here | |||
| --> $SRC_DIR/core/src/iter/traits/iterator.rs:LL:COL | |||
There was a problem hiding this comment.
Not sure what's up with cases like this one--FWIW the span gets printed correctly when I run locally.
This comment has been minimized.
This comment has been minimized.
sjwang05
force-pushed
the
issue-61066
branch
from
f0cfc4a to
9e1170c
Compare
This comment has been minimized.
This comment has been minimized.
sjwang05
force-pushed
the
issue-61066
branch
from
9e1170c to
d8fb04e
Compare
| }) | ||
| | hir::Node::Item(hir::Item { kind: hir::ItemKind::Fn(sig, ..), .. }) = node | ||
| && let ret_span = sig.decl.output.span() | ||
| && !ret_span.from_expansion() |
There was a problem hiding this comment.
Mainly to avoid giving weird suggestions in desugaring like async fn, though if you think we should still give suggestions in macros, we could check for desugaring_kind().is_none() instead.
| if let Some(local_did) = def_id.as_local() | ||
| && let Some(node) = self.tcx.opt_hir_node(self.tcx.local_def_id_to_hir_id(local_did)) | ||
| && let hir::Node::ImplItem(hir::ImplItem { | ||
| kind: hir::ImplItemKind::Fn(sig, ..), .. |
There was a problem hiding this comment.
How do we check we aren't suggesting a change in a trait impl?
There was a problem hiding this comment.
We don't match on hir::TraitItem--I made the check a bit clearer when addressing #119340 (comment)
cjgillot
added
S-waiting-on-author
|
Thanks for all the suggestions. @rustbot review |
rustbot
added
S-waiting-on-review
There was a problem hiding this comment.
Sorry for any unintended harshness, but I am really not in favor of this change 😅
I believe it inverts the normal "source of truth" we use (the signature) when suggesting to change a function signature that may have other call-sites, and I am skeptical that adding this information on every mismatch is worthwhile. Does it account for the other call-sites of the function that may break when this signature is changed?
As for the technical impl, I believe it needs to avoid labeling spans that don't actually map to lines in the source map, and needs to use backticks on item names.
Also, does this handle polymorphic functions correctly? How does it map a type mismatch back to the generic parameters of the function, for example?
Finally, why do we need to provide a structured suggestion -- seems to break run-rustfix tests for a very heuristic suggestion?
compiler-errors
added
S-waiting-on-author
|
☔ The latest upstream changes (presumably #120877) made this pull request unmergeable. Please resolve the merge conflicts. |
|
@sjwang05 any updates on this? |
|
Closing this as inactive. Feel free to reöpen this pr or create a new pr if you get the time to work on this. Thanks |
Dylan-DPC
added
S-inactive
Closes #61066