Suggest a borrow when using dbg #120990
Suggest a borrow when using dbg #120990
Conversation
e216c00
to
1224847
Compare
|
☔ The latest upstream changes (presumably #123108) made this pull request unmergeable. Please resolve the merge conflicts. |
1224847
to
66d7f4d
Compare
|
@rustbot ready |
|
@estebank, any comments on this PR? |
|
r? rust-lang/compiler |
| // it's useless to suggest inserting `ref` when the span comes from std library | ||
| // anyway, user can not modify std library in most cases, so let's keep it quite? | ||
| if !from_std { |
There was a problem hiding this comment.
I think you'll want to forbid the suggestion whenever it points anywhere but to local code, the standard library is just the most common external crate in our test suite, but the issue probably exists for any dependency?
There was a problem hiding this comment.
changed it to:
&& !move_span.is_dummy()
&& !self.infcx.tcx.sess.source_map().is_imported(move_span)| // only suggest for macro | ||
| if move_span.source_callsite() == move_span { | ||
| return; | ||
| } |
There was a problem hiding this comment.
why? How do the diagnostics look without macros and could they improved by this, too?
There was a problem hiding this comment.
If we don't limit only for macro, it will also add suggestion for this code:
fn s() -> String {
let a = String::new();
let _b = match a {
tmp => {
eprintln!("dbg: {}", tmp);
tmp
}
};
return a;
}with diagnostic:
--> ./t/ha.rs:15:12
|
7 | let a = String::new();
| - move occurs because `a` has type `String`, which does not implement the `Copy` trait
...
10 | tmp => {
| --- value moved here
...
15 | return a; //~ ERROR use of moved value:
| ^ value used here after move
|
help: consider borrowing instead of transferring ownership
|
9 | let _b = match &a {
| +
help: borrow this binding in the pattern to avoid moving the value
|
10 | ref tmp => {
| +++Seems it's also a good one.
I will add it into testcase.
| let arg_code = if let Some(span) = span | ||
| && let Ok(code) = sm.span_to_snippet(span) |
There was a problem hiding this comment.
I would prefer not to be using snippets for making decisions. Do we not have the expression of the value somewhere?
There was a problem hiding this comment.
Seems no, I just have a try to use ExpressionFinder to get the hir, and it only can not handle this scenario:
the argument comes from the function parameters, since an ExpressionFinder will search in the body.
There was a problem hiding this comment.
You can check self.body.var_debug_info for any entry for place, that gives you a name. Not perfect, ideally we'd have an Ident with expansion information, but this is better than span_to_snippet imo
b31f9b9
to
6ff8a84
Compare
|
@rustbot ready |
| let arg_code = if let Some(span) = span | ||
| && let Ok(code) = sm.span_to_snippet(span) |
There was a problem hiding this comment.
You can check self.body.var_debug_info for any entry for place, that gives you a name. Not perfect, ideally we'd have an Ident with expansion information, but this is better than span_to_snippet imo
This comment has been minimized.
This comment has been minimized.
b01c541
to
1767432
Compare
|
@bors r+ rollup |
…r=oli-obk Suggest a borrow when using dbg Fixes rust-lang#120327 r? `@estebank`
|
@bors r- needs rebase + build fix |
85e312f
to
3179526
Compare
|
rebased. |
|
thx! |
…r=oli-obk Suggest a borrow when using dbg Fixes rust-lang#120327 r? `@estebank`
…r=oli-obk Suggest a borrow when using dbg Fixes rust-lang#120327 r? ``@estebank``
…iaskrgr Rollup of 5 pull requests Successful merges: - rust-lang#120990 (Suggest a borrow when using dbg) - rust-lang#127047 (fix least significant digits of f128 associated constants) - rust-lang#127629 (Suggest using `map_or` when `Option<&T>::unwrap_or where T: Deref` fails) - rust-lang#127770 (Update books) - rust-lang#127780 (Make sure trait def ids match before zipping args in `note_function_argument_obligation`) r? `@ghost` `@rustbot` modify labels: rollup
…r=oli-obk Suggest a borrow when using dbg Fixes rust-lang#120327 r? ```@estebank```
Rollup of 5 pull requests Successful merges: - rust-lang#120990 (Suggest a borrow when using dbg) - rust-lang#127047 (fix least significant digits of f128 associated constants) - rust-lang#127680 (Bootstrap command refactoring: port remaining commands with access to `Build` (step 6)) - rust-lang#127770 (Update books) - rust-lang#127780 (Make sure trait def ids match before zipping args in `note_function_argument_obligation`) r? `@ghost` `@rustbot` modify labels: rollup
Rollup of 6 pull requests Successful merges: - rust-lang#120990 (Suggest a borrow when using dbg) - rust-lang#127047 (fix least significant digits of f128 associated constants) - rust-lang#127709 (match lowering: Move `MatchPair` tree creation to its own module) - rust-lang#127770 (Update books) - rust-lang#127780 (Make sure trait def ids match before zipping args in `note_function_argument_obligation`) - rust-lang#127795 (Fix typos in RELEASES.md) r? `@ghost` `@rustbot` modify labels: rollup
Rollup merge of rust-lang#120990 - chenyukang:yukang-fix-120327-dbg, r=oli-obk Suggest a borrow when using dbg Fixes rust-lang#120327 r? ````@estebank````
Fixes #120327
r? @estebank