Suggest a borrow when using dbg!()

[gak]

Code

fn s() -> String {
    let a = String::new();
    dbg!(a);
    return a;
}

Current output

Compiling playground v0.0.1 (/playground)
error[E0382]: use of moved value: `a`
 --> src/lib.rs:4:12
  |
2 |     let a = String::new();
  |         - move occurs because `a` has type `String`, which does not implement the `Copy` trait
3 |     dbg!(a);
  |     ------- value moved here
4 |     return a;
  |            ^ value used here after move
  |
help: borrow this binding in the pattern to avoid moving the value
 --> /playground/.rustup/toolchains/nightly-x86_64-unknown-linux-gnu/lib/rustlib/src/rust/library/std/src/macros.rs:364:13
  |
36|             ref tmp => {
  |             +++

For more information about this error, try `rustc --explain E0382`.
error: could not compile `playground` (lib) due to 1 previous error

Desired output

help: consider borrowing instead of transferring ownership.

dbg!(&a);

Rationale and extra context

No response

Other cases

No response

Rust Version

Example was in the playground: "Build using the Nightly version: 1.77.0-nightly (2024-01-23 5d3d3479d774754856db)"

My machine has the same message:

rustc 1.76.0-nightly (a96d57bdb 2023-12-15)
binary: rustc
commit-hash: a96d57bdb6d2bb6d233d7d5aaefc2995ab99be01
commit-date: 2023-12-15
host: aarch64-apple-darwin
release: 1.76.0-nightly
LLVM version: 17.0.6

Anything else?

No response

[detly]

I thought the intent with dbg!() was to use it in a passthrough style, so you can insert it into existing expressions ie. return a; becomes return dbg!(a);. In which case, maybe the lint should be for not using the value resulting from evaluating dbg!(expr).

[gak]

Not sure about the majority of users but for me dbg!(v); is a convenience over println!(“{:#?}”, v);. I use it without using the return value almost all of the time.

[detly]

Probably a common thing to do, sure, but if we're talking about adding a lint, why not nudge the user in the direction of the intended and documented usage? You could at least do both: "insert dbg!() into an existing expression or pass it a borrow" etc.

[chenyukang]

The fix in #120990 will only report a hint when there is already a borrow of moved value happened.

For example this code:

fn in_expr() -> String {
    let b: String = "".to_string();
    let res = dbg!(b);
    let _l = b.len();
    return res; 
}

before this fix the error is:

error[E0382]: borrow of moved value: `b`
  --> ./t/de.rs:10:14
   |
8  |     let b: String = "".to_string();
   |         - move occurs because `b` has type `String`, which does not implement the `Copy` trait
9  |     let res = dbg!(b);
   |               ------- value moved here
10 |     let _l = b.len();
   |              ^ value borrowed here after move
   |
help: borrow this binding in the pattern to avoid moving the value
  --> /Users/yukang/.rustup/toolchains/nightly-aarch64-apple-darwin/lib/rustlib/src/rust/library/std/src/macros.rs:364:13
   |
364|             ref tmp => {
   |             +++

error: aborting due to 1 previous error

with the fix, since there is already a value moved here error, the lint will try to find out the original macro, and suggest a borrow:

error[E0382]: borrow of moved value: `b`
  --> ./t/de.rs:10:14
   |
8  |     let b: String = "".to_string();
   |         - move occurs because `b` has type `String`, which does not implement the `Copy` trait
9  |     let res = dbg!(b);
   |               ------- value moved here
10 |     let _l = b.len();
   |              ^ value borrowed here after move
   |
help: consider borrowing instead of transferring ownership
   |
9  |     let res = dbg!(&b);
   |                    +

The suggestion is not 100% right, since if we change it to let res = dbg!(&b), we need one more extra fix now(the return type is not expecting a reference), but there is a different issue, at least we have already moved one step on.

If the original code is:

fn in_expr() -> String {
    let b: String = "".to_string();
    let res = dbg!(b);
    let _l = b.len();
    return res.to_string();
}

Then suggest a borrow is a great and correct hint.

[chenyukang]

Similar in the scenario of function argument:

fn get_expr(_s: String) {}

fn test() {
    let a: String = "".to_string();
    let _res = get_expr(dbg!(a));
    let _l = a.len();
}

original error is:

error[E0382]: borrow of moved value: `a`
  --> ./t/de.rs:12:14
   |
10 |     let a: String = "".to_string();
   |         - move occurs because `a` has type `String`, which does not implement the `Copy` trait
11 |     let _res = get_expr(dbg!(a));
   |                         ------- value moved here
12 |     let _l = a.len();
   |              ^ value borrowed here after move
   |
help: borrow this binding in the pattern to avoid moving the value
  --> /Users/yukang/.rustup/toolchains/nightly-aarch64-apple-darwin/lib/rustlib/src/rust/library/std/src/macros.rs:364:13
   |
364|             ref tmp => {
   |             +++

new errors from the fix:

error[E0382]: borrow of moved value: `a`
  --> ./t/de.rs:12:14
   |
10 |     let a: String = "".to_string();
   |         - move occurs because `a` has type `String`, which does not implement the `Copy` trait
11 |     let _res = get_expr(dbg!(a));
   |                         ------- value moved here
12 |     let _l = a.len();
   |              ^ value borrowed here after move
   |
help: consider borrowing instead of transferring ownership
   |
11 |     let _res = get_expr(dbg!(&a));
   |                              +

[detly]

If the original code is:

fn in_expr() -> String {
    let b: String = "".to_string();
    let res = dbg!(b);
    let _l = b.len();
    return res.to_string();
}

Then suggest a borrow is a great and correct hint.

Would let _l = dbg!(b).len(); not also be a good hint?

Similar in the scenario of function argument:

The suggestion wouldn't compile anyway right? Because get_expr() would need to change to take &str? Without dbg!() involved, the compiler does indeed suggest that:

note: consider changing this parameter type in function `get_expr` to borrow instead if owning the value isn't necessary
 --> src/lib.rs:1:17
  |
1 | fn get_expr(_s: String) {}
  |    --------     ^^^^^^ this parameter takes ownership of the value
  |    |
  |    in this function
help: consider cloning the value if the performance cost is acceptable
  |
5 |     let _res = get_expr(a.clone());
  |                          ++++++++

Once that's addressed (whichever way it's done), dbg!() once again becomes transparent, and eg. dbg!(a).clone() or dbg!(&a) works. &dbg!(a) does not, however, and I see why a "consider borrowing" hint would be good there.

However, if you make the dbg!(&a) change first and leave get_expr(_s: String) as-is, the resulting error does not include the suggestion to change get_expr(), which is unhelpful.

(I agree that the ref tmp => { message is even less helpful though, I'm not disputing that.)

[chenyukang]

Would let _l = dbg!(b).len(); not also be a good hint?

Yes, it's a good hint, but from the implementation view of a hint, it's much harder to analyze the following code from the error point and then construct a new expression. Even it seems an easy thing for humans.

You are right, the suggestion is not 100% right, we may need an extra follow-up fix after applying the hint, which is also a common thing in rustc diagnostics.