Suggest a borrow when using dbg

[chenyukang]

Fixes #120327
r? @estebank

[bors]

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

[chenyukang]

@rustbot ready

[chenyukang]

@estebank, any comments on this PR?

[wesleywiser]

r? rust-lang/compiler

[oli-obk]

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?

[chenyukang]

changed it to:

    && !move_span.is_dummy()
    && !self.infcx.tcx.sess.source_map().is_imported(move_span)

[oli-obk]

why? How do the diagnostics look without macros and could they improved by this, too?

[chenyukang]

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.

[oli-obk]

I would prefer not to be using snippets for making decisions. Do we not have the expression of the value somewhere?

[chenyukang]

Seems no, I just have a try to use ExpressionFinder to get the hir, and it only can not handle this scenario:

https://github.com/rust-lang/rust/pull/120990/files#diff-bd7f956e58a376780df31f75794e7af9b5d4043582aed6f129ae386a2b83ebdeR13-R17

the argument comes from the function parameters, since an ExpressionFinder will search in the body.

[oli-obk]

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

[chenyukang]

yeah, fixed the code.

[chenyukang]

@rustbot ready
I could not find a better way without using snippets.

[oli-obk]

whoops, forgot to send the review I wrote recently

[oli-obk]

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

[oli-obk]

@bors r+ rollup

[bors]

📌 Commit 6ff8a84 has been approved by oli-obk

It is now in the queue for this repository.

[matthiaskrgr]

@bors r- needs rebase + build fix
#127775 (comment)

[chenyukang]

rebased.

[matthiaskrgr]

thx!
@bors r=oli-obk

[bors]

📌 Commit 3179526 has been approved by oli-obk

It is now in the queue for this repository.