Print type of every call in a method call chain #96918

estebank · 2022-05-10T19:41:56Z

error[E0271]: type mismatch resolving `<std::collections::hash_map::Iter<'_, _, _> as Iterator>::Item == &_`
  --> $DIR/issue-33941.rs:6:36
   |
LL |     for _ in HashMap::new().iter().cloned() {}
   |              ------------   ----   ^^^^^^ expected reference, found tuple
   |              |              |
   |              |              this call is of type `std::collections::hash_map::Iter<'_, _, _>`
   |              this call is of type `&HashMap<_, _>`
   |
   = note: expected reference `&_`
                  found tuple `(&_, &_)`
note: required by a bound in `cloned`
  --> $SRC_DIR/core/src/iter/traits/iterator.rs:LL:COL
   |
LL |         Self: Sized + Iterator<Item = &'a T>,
   |                                ^^^^^^^^^^^^ required by this bound in `cloned`

Partially address #33941.

rust-highfive · 2022-05-10T19:41:59Z

r? @compiler-errors

(rust-highfive has picked a reviewer for you, use r? to override)

compiler-errors · 2022-05-10T19:52:02Z

Also I kinda find the new output to be misleading, at least when presented inline with the original error.

My eyes see:

expected reference, found tuple

Then one line later:

this call is of type std::collections::hash_map::Iter<'_, _, _>

The first error is a projection error though, and it doesn't feel obvious from the second line why that projection error would show up.

I think the issue is that we're just providing the types, but that doesn't really make the issue more specific unless someone knows what the impl block impl Iterator for std::collections::hash_map::Iter<'_, _, _> looks like off the top of their heads.

estebank · 2022-05-10T20:07:15Z

No, it was meant to be #33941

``` error[E0271]: type mismatch resolving `<std::collections::hash_map::Iter<'_, _, _> as Iterator>::Item == &_` --> $DIR/issue-33941.rs:6:36 | LL | for _ in HashMap::new().iter().cloned() {} | ------------ ---- ^^^^^^ expected reference, found tuple | | | | | this call is of type `std::collections::hash_map::Iter<'_, _, _>` | this call is of type `&HashMap<_, _>` | = note: expected reference `&_` found tuple `(&_, &_)` note: required by a bound in `cloned` --> $SRC_DIR/core/src/iter/traits/iterator.rs:LL:COL | LL | Self: Sized + Iterator<Item = &'a T>, | ^^^^^^^^^^^^ required by this bound in `cloned` ``` Partially address rust-lang#33941.

estebank · 2022-05-10T20:38:58Z

Also I kinda find the new output to be misleading, at least when presented inline with the original error.

We could present it in its own subwindow (if we wanted)

I agree that it isn't great. I wanted to add more context for the more gnarly cases of long iterator chains (which RA in VSCode kind of does already with inline hints), and this somewhat an intermediary step to probing for methods on the type on the left that can give you something that the call in the right would succeed on. This for example should mention that .keys() and .values() would make the chain succeed, but I haven't prototyped that yet. Don't know if this is worth landing on its own though, particularly if it feels like it adds confusion.

compiler-errors · 2022-06-24T03:14:42Z

Sorry for leaving this so long without a review. I've thought a bit, and personally don't think this suggestion is worth landing on its own, at least specifically for projection errors like this.

For example, given this error output:

error[E0271]: type mismatch resolving `<std::collections::hash_map::Iter<'_, _, _> as Iterator>::Item == &_`
  --> $DIR/issue-33941.rs:6:36
   |
LL |     for _ in HashMap::new().iter().cloned() {}
   |              ------------   ----   ^^^^^^ expected reference, found tuple
   |              |              |
   |              |              this call is of type `std::collections::hash_map::Iter<'_, _, _>`
   |              this call is of type `&HashMap<_, _>`

I'm not sure how the user is supposed to use the extra information to solve the error at hand. Perhaps it's useful to understand where the $TYPE in <$TYPE as Trait>::Item is coming from, but I think this is more of a shortcoming of the presentation of the error message of E0271 than something that adding more spanned labels can fix.

Thoughts?

estebank · 2022-06-24T21:16:11Z

compiler/rustc_trait_selection/src/traits/error_reporting/mod.rs

+                // FIXME(estebank): look for method calls that can be applied on the previous
+                // element of the chain, that would make the next call to typeck.


@compiler-errors I think that this is the part that would be useful from this PR, that this PR didn't handle: enabling rustc to look for method calls that could be added to make the code compile would be the biggest benefit of this change, but this PR on its own might not be as useful as I envisioned.

At least in the case of #33941, suggesting other methods that are compatible with the real Item type doesn't seem like the right direction for a suggestion to go imo.

I think the compiler should be trying to stress that that (&K, &V) is not &T instead. I'm not sure how to make the connection more clear that if they want (&K, &V) -> (K, V) they've gotta do the map themselves... now I don't think this needs special-casing per se, but conversely, I think suggesting other methods is probably going to lead them further astray?

estebank · 2022-06-24T21:17:57Z

compiler/rustc_trait_selection/src/traits/error_reporting/mod.rs

+                {
+                    let typeck_results = typeck_results.borrow();
+                    while let hir::ExprKind::MethodCall(method_segment, args, _) = receiver.kind {
+                        if let Some(ty) = typeck_results.expr_ty_adjusted_opt(receiver) {


I wonder if resolving ty here would give us the Item type instead of _, which would be useful then.

estebank · 2022-06-24T21:19:08Z

I wonder if it would be worth it to special case this for HashMap in particular (which I wanted to avoid).

compiler-errors · 2022-06-24T21:32:26Z

Maybe we should rework the error to put more stress on this part:

  expected reference `&_`
  found tuple `(&_, &_)`

Specifically in conjunction with the projection <std::collections::hash_map::Iter<'_, _, _> as Iterator>::Item which was the un-normalized version of the "found" type.

Ignoring the _ parts, which ideally in production code will have real types there instead of inference variables, I would like to maybe something that that stresses the fact that (&_, &_) is the result of normalizing <std::collections::hash_map::Iter<'_, _, _> as Iterator>::Item.

Like in my head, the info is all there to realize why .clone() wouldn't work, it's just hard to see from the error message -- maybe biased by the minimized example since it has no real types and _ everywhere is hard to read.

compiler-errors · 2022-06-24T21:40:37Z

I'm just thinking about projection errors in terms of one I see a lot, where we see <impl Future<Output = T> as Future>::Output == U in the error message.

The problem is in that case it's not always associated with a method call going wrong, but instead it's really just a really terse way of saying
"expected a future that yields U, but we got one that yields T".

In the same way, we have in this example "expected an iterator over a reference, but we got an iterator over a tuple" instead. I'd just want to find a better way of stating that with the info we have at hand.

But maybe I'm just rambling 🤷

JohnCSimon · 2022-07-24T06:04:32Z

ping from triage:
@compiler-errors
Looks like this PR is still in progress

FYI: when a PR is ready for review, send a message containing
@rustbot ready to switch to S-waiting-on-review so the PR is in the reviewer's backlog.

@rustbot author

compiler-errors · 2022-07-24T06:10:43Z

@JohnCSimon, I think that ping is probably better meant for @estebank. I am the reviewer on this PR, not the author.

JohnCSimon · 2022-07-24T06:40:09Z

@JohnCSimon, I think that ping is probably better meant for @estebank. I am the reviewer on this PR, not the author.

sorry. I keep switching the two around. 😨

bors · 2022-08-08T13:25:01Z

☔ The latest upstream changes (presumably #98863) made this pull request unmergeable. Please resolve the merge conflicts.

rust-highfive assigned compiler-errors May 10, 2022

rustbot added the T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. label May 10, 2022

rust-highfive added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label May 10, 2022

This comment was marked as outdated.

Sign in to view

This comment was marked as resolved.

Sign in to view

estebank force-pushed the issue-33941 branch from b71c625 to 458fdb3 Compare May 10, 2022 20:35

estebank commented Jun 24, 2022

View reviewed changes

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 Jul 24, 2022

estebank closed this Aug 19, 2022

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Print type of every call in a method call chain #96918

Print type of every call in a method call chain #96918

estebank commented May 10, 2022 •

edited

Loading

rust-highfive commented May 10, 2022

This comment was marked as outdated.

compiler-errors commented May 10, 2022 •

edited

Loading

This comment was marked as resolved.

estebank commented May 10, 2022

estebank commented May 10, 2022

compiler-errors commented Jun 24, 2022 •

edited

Loading

estebank Jun 24, 2022

compiler-errors Jun 24, 2022

estebank Jun 24, 2022

estebank commented Jun 24, 2022

compiler-errors commented Jun 24, 2022

compiler-errors commented Jun 24, 2022

JohnCSimon commented Jul 24, 2022

compiler-errors commented Jul 24, 2022 •

edited

Loading

JohnCSimon commented Jul 24, 2022

bors commented Aug 8, 2022

		// FIXME(estebank): look for method calls that can be applied on the previous
		// element of the chain, that would make the next call to typeck.

Print type of every call in a method call chain #96918

Print type of every call in a method call chain #96918

Conversation

estebank commented May 10, 2022 • edited Loading

rust-highfive commented May 10, 2022

This comment was marked as outdated.

compiler-errors commented May 10, 2022 • edited Loading

This comment was marked as resolved.

estebank commented May 10, 2022

estebank commented May 10, 2022

compiler-errors commented Jun 24, 2022 • edited Loading

estebank Jun 24, 2022

Choose a reason for hiding this comment

compiler-errors Jun 24, 2022

Choose a reason for hiding this comment

estebank Jun 24, 2022

Choose a reason for hiding this comment

estebank commented Jun 24, 2022

compiler-errors commented Jun 24, 2022

compiler-errors commented Jun 24, 2022

JohnCSimon commented Jul 24, 2022

compiler-errors commented Jul 24, 2022 • edited Loading

JohnCSimon commented Jul 24, 2022

bors commented Aug 8, 2022

estebank commented May 10, 2022 •

edited

Loading

compiler-errors commented May 10, 2022 •

edited

Loading

compiler-errors commented Jun 24, 2022 •

edited

Loading

compiler-errors commented Jul 24, 2022 •

edited

Loading