Point at LHS on binop type err if relevant #105192
Conversation
|
r? @fee1-dead (rustbot has picked a reviewer for you, use r? to override) |
rustbot
added
S-waiting-on-review
| LL | assert_eq!(3i32, &3i32); | ||
| | ^^^^^^^^^^^^^^^^^^^^^^^ expected `i32`, found `&i32` | ||
| | ^^^^^^^^^^^^^^^^^^^^^^^ | ||
| | | | ||
| | expected `i32`, found `&i32` | ||
| | expected because of this binary operation left hand side expression |
There was a problem hiding this comment.
I find this to be confusing. Can we not add this note if the binop is within a macro expansion, or maybe find a way to actually point at 3i32 here?
There was a problem hiding this comment.
We can't point at the macro argument because the macro machinery doesn't have enough intelligence yet to do that once its gone though any expression. In this case the macro expands to something that loses the span. That is a project that I've tried to tackle in the past and will not be fixed anytime soon, sadly.
| { | ||
| err.span_label( | ||
| lhs.span, | ||
| "expected because of this binary operation left hand side expression", |
There was a problem hiding this comment.
| "expected because of this binary operation left hand side expression", | |
| "expected due to this binary operation left hand side expression", |
Since the label is a bit lengthy will it make sense to cut it shorter?
| | ^^^^^^^^ expected `usize`, found `isize` | ||
| | ------- ^^^^^^^^ expected `usize`, found `isize` | ||
| | | | ||
| | expected because of this binary operation left hand side expression |
There was a problem hiding this comment.
Maybe we could say something like "expected because the left-hand side is $TY"?
Unrelated, I feel like "binary operation left hand side expression" is a lot of words but not necessarily clearer than just "left-hand side".
There was a problem hiding this comment.
Changed the wording to be "expected because this is Ty", that should be understandable while also short and jargon-free.
|
Implementation looks fine, should be good to go after addressing the comments @rustbot author |
rustbot
added
S-waiting-on-author
estebank
force-pushed
the
point-at-lhs-on-binop
branch
from
38e67c3
to
132a140
Compare
estebank
added
S-waiting-on-review
|
@bors r+ rollup |
bors
added
S-waiting-on-bors
…iaskrgr Rollup of 6 pull requests Successful merges: - rust-lang#104460 (Migrate parts of `rustc_expand` to session diagnostics) - rust-lang#105192 (Point at LHS on binop type err if relevant) - rust-lang#105234 (Remove unneeded field from `SwitchTargets`) - rust-lang#105239 (Avoid heap allocation when truncating thread names) - rust-lang#105410 (Consider `parent_count` for const param defaults) - rust-lang#105482 (Fix invalid codegen during debuginfo lowering) Failed merges: - rust-lang#105411 (Introduce `with_forced_trimmed_paths`) r? `@ghost` `@rustbot` modify labels: rollup
No description provided.