Suggest a borrow when using dbg!()

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

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).

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.

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.

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.

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));
   |                              +

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.)

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.

Rollup merge of rust-lang#120990 - chenyukang:yukang-fix-120327-dbg, …

Unrolled build for rust-lang#120990

Navigation Menu

Repository navigation

Suggest a borrow when using dbg!() #120327

Code

Current output

Desired output

Rationale and extra context

Other cases

Rust Version

Anything else?

Metadata

Assignees

Labels

Type

Projects

Milestone

Relationships

Development

Suggest a borrow when using dbg!() #120327

Description

Code

Current output

Desired output

Rationale and extra context

Other cases

Rust Version

Anything else?

Activity

detly commented on Feb 27, 2024

gak commented on Feb 28, 2024

detly commented on Feb 28, 2024

chenyukang commented on Mar 2, 2024

chenyukang commented on Mar 2, 2024

detly commented on Mar 5, 2024

chenyukang commented on Mar 5, 2024

Metadata

Metadata

Assignees

Labels

Type

Projects

Milestone

Relationships

Development

Issue actions