update diagnostic for variables moved by dbg!

dianne · dianne · commit 7d1b41cbb386 · 2026-03-29T09:48:41.000-07:00
diff --git a/compiler/rustc_borrowck/src/diagnostics/conflict_errors.rs b/compiler/rustc_borrowck/src/diagnostics/conflict_errors.rs
@@ -545,8 +545,6 @@ impl<'infcx, 'tcx> MirBorrowckCtxt<'_, 'infcx, 'tcx> {
     }
 
     // for dbg!(x) which may take ownership, suggest dbg!(&x) instead
-    // but here we actually do not check whether the macro name is `dbg!`
-    // so that we may extend the scope a bit larger to cover more cases
     fn suggest_ref_for_dbg_args(
         &self,
         body: &hir::Expr<'_>,
@@ -560,29 +558,41 @@ impl<'infcx, 'tcx> MirBorrowckCtxt<'_, 'infcx, 'tcx> {
         });
         let Some(var_info) = var_info else { return };
         let arg_name = var_info.name;
-        struct MatchArgFinder {
-            expr_span: Span,
-            match_arg_span: Option<Span>,
+        struct MatchArgFinder<'tcx> {
+            tcx: TyCtxt<'tcx>,
+            move_span: Span,
             arg_name: Symbol,
+            match_arg_span: Option<Span> = None,
         }
-        impl Visitor<'_> for MatchArgFinder {
+        impl Visitor<'_> for MatchArgFinder<'_> {
             fn visit_expr(&mut self, e: &hir::Expr<'_>) {
                 // dbg! is expanded into a match pattern, we need to find the right argument span
-                if let hir::ExprKind::Match(expr, ..) = &e.kind
-                    && let hir::ExprKind::Path(hir::QPath::Resolved(
-                        _,
-                        path @ Path { segments: [seg], .. },
-                    )) = &expr.kind
-                    && seg.ident.name == self.arg_name
-                    && self.expr_span.source_callsite().contains(expr.span)
+                if let hir::ExprKind::Match(scrutinee, ..) = &e.kind
+                    && let hir::ExprKind::Tup(args) = scrutinee.kind
+                    && e.span.macro_backtrace().any(|expn| {
+                        expn.macro_def_id.is_some_and(|macro_def_id| {
+                            self.tcx.is_diagnostic_item(sym::dbg_macro, macro_def_id)
+                        })
+                    })
                 {
-                    self.match_arg_span = Some(path.span);
+                    for arg in args {
+                        if let hir::ExprKind::Path(hir::QPath::Resolved(
+                            _,
+                            path @ Path { segments: [seg], .. },
+                        )) = &arg.kind
+                            && seg.ident.name == self.arg_name
+                            && self.move_span.source_equal(arg.span)
+                        {
+                            self.match_arg_span = Some(path.span);
+                            return;
+                        }
+                    }
                 }
                 hir::intravisit::walk_expr(self, e);
             }
         }
 
-        let mut finder = MatchArgFinder { expr_span: move_span, match_arg_span: None, arg_name };
+        let mut finder = MatchArgFinder { tcx: self.infcx.tcx, move_span, arg_name, .. };
         finder.visit_expr(body);
         if let Some(macro_arg_span) = finder.match_arg_span {
             err.span_suggestion_verbose(
diff --git a/compiler/rustc_span/src/symbol.rs b/compiler/rustc_span/src/symbol.rs
@@ -746,6 +746,7 @@ symbols! {
         custom_mir,
         custom_test_frameworks,
         d32,
+        dbg_macro,
         dead_code,
         dealloc,
         debug,
diff --git a/src/tools/clippy/clippy_utils/src/sym.rs b/src/tools/clippy/clippy_utils/src/sym.rs
@@ -199,7 +199,6 @@ generate! {
     cx,
     cycle,
     cyclomatic_complexity,
-    dbg_macro,
     de,
     debug_struct,
     deprecated_in_future,
diff --git a/tests/ui/borrowck/dbg-issue-120327.rs b/tests/ui/borrowck/dbg-issue-120327.rs
@@ -1,67 +1,44 @@
+//! Diagnostic test for <https://github.com/rust-lang/rust/issues/120327>: suggest borrowing
+//! variables passed to `dbg!` that are later used.
+//@ dont-require-annotations: HELP
+
 fn s() -> String {
     let a = String::new();
-    dbg!(a);
+    dbg!(a); //~ HELP consider borrowing instead of transferring ownership
     return a; //~ ERROR use of moved value:
 }
 
 fn m() -> String {
     let a = String::new();
-    dbg!(1, 2, a, 1, 2);
+    dbg!(1, 2, a, 1, 2); //~ HELP consider borrowing instead of transferring ownership
     return a; //~ ERROR use of moved value:
 }
 
 fn t(a: String) -> String {
     let b: String = "".to_string();
-    dbg!(a, b);
+    dbg!(a, b); //~ HELP consider borrowing instead of transferring ownership
     return b; //~ ERROR use of moved value:
 }
 
 fn x(a: String) -> String {
     let b: String = "".to_string();
-    dbg!(a, b);
+    dbg!(a, b); //~ HELP consider borrowing instead of transferring ownership
     return a; //~ ERROR use of moved value:
 }
 
-macro_rules! my_dbg {
-    () => {
-        eprintln!("[{}:{}:{}]", file!(), line!(), column!())
-    };
-    ($val:expr $(,)?) => {
-        match $val {
-            tmp => {
-                eprintln!("[{}:{}:{}] {} = {:#?}",
-                    file!(), line!(), column!(), stringify!($val), &tmp);
-                tmp
-            }
-        }
-    };
-    ($($val:expr),+ $(,)?) => {
-        ($(my_dbg!($val)),+,)
-    };
-}
-
-fn test_my_dbg() -> String {
-    let b: String = "".to_string();
-    my_dbg!(b, 1);
-    return b; //~ ERROR use of moved value:
-}
-
-fn test_not_macro() -> String {
-    let a = String::new();
-    let _b = match a {
-        tmp => {
-            eprintln!("dbg: {}", tmp);
-            tmp
-        }
-    };
-    return a; //~ ERROR use of moved value:
+fn two_of_them(a: String) -> String {
+    dbg!(a, a); //~ ERROR use of moved value
+    //~| HELP consider borrowing instead of transferring ownership
+    //~| HELP consider borrowing instead of transferring ownership
+    return a; //~ ERROR use of moved value
 }
 
 fn get_expr(_s: String) {}
 
+// The suggestion is purely syntactic; applying it here will result in a type error.
 fn test() {
     let a: String = "".to_string();
-    let _res = get_expr(dbg!(a));
+    let _res = get_expr(dbg!(a)); //~ HELP consider borrowing instead of transferring ownership
     let _l = a.len(); //~ ERROR borrow of moved value
 }
 
diff --git a/tests/ui/borrowck/dbg-issue-120327.stderr b/tests/ui/borrowck/dbg-issue-120327.stderr
@@ -1,5 +1,5 @@
 error[E0382]: use of moved value: `a`
-  --> $DIR/dbg-issue-120327.rs:4:12
+  --> $DIR/dbg-issue-120327.rs:8:12
    |
 LL |     let a = String::new();
    |         - move occurs because `a` has type `String`, which does not implement the `Copy` trait
@@ -12,9 +12,13 @@ help: consider cloning the value if the performance cost is acceptable
    |
 LL |     dbg!(a.clone());
    |           ++++++++
+help: consider borrowing instead of transferring ownership
+   |
+LL |     dbg!(&a);
+   |          +
 
 error[E0382]: use of moved value: `a`
-  --> $DIR/dbg-issue-120327.rs:10:12
+  --> $DIR/dbg-issue-120327.rs:14:12
    |
 LL |     let a = String::new();
    |         - move occurs because `a` has type `String`, which does not implement the `Copy` trait
@@ -27,9 +31,13 @@ help: consider cloning the value if the performance cost is acceptable
    |
 LL |     dbg!(1, 2, a.clone(), 1, 2);
    |                 ++++++++
+help: consider borrowing instead of transferring ownership
+   |
+LL |     dbg!(1, 2, &a, 1, 2);
+   |                +
 
 error[E0382]: use of moved value: `b`
-  --> $DIR/dbg-issue-120327.rs:16:12
+  --> $DIR/dbg-issue-120327.rs:20:12
    |
 LL |     let b: String = "".to_string();
    |         - move occurs because `b` has type `String`, which does not implement the `Copy` trait
@@ -42,9 +50,13 @@ help: consider cloning the value if the performance cost is acceptable
    |
 LL |     dbg!(a, b.clone());
    |              ++++++++
+help: consider borrowing instead of transferring ownership
+   |
+LL |     dbg!(a, &b);
+   |             +
 
 error[E0382]: use of moved value: `a`
-  --> $DIR/dbg-issue-120327.rs:22:12
+  --> $DIR/dbg-issue-120327.rs:26:12
    |
 LL | fn x(a: String) -> String {
    |      - move occurs because `a` has type `String`, which does not implement the `Copy` trait
@@ -58,47 +70,52 @@ help: consider cloning the value if the performance cost is acceptable
    |
 LL |     dbg!(a.clone(), b);
    |           ++++++++
+help: consider borrowing instead of transferring ownership
+   |
+LL |     dbg!(&a, b);
+   |          +
 
-error[E0382]: use of moved value: `b`
-  --> $DIR/dbg-issue-120327.rs:46:12
+error[E0382]: use of moved value: `a`
+  --> $DIR/dbg-issue-120327.rs:30:13
    |
-LL |             tmp => {
-   |             --- value moved here
-...
-LL |     let b: String = "".to_string();
-   |         - move occurs because `b` has type `String`, which does not implement the `Copy` trait
-LL |     my_dbg!(b, 1);
-LL |     return b;
-   |            ^ value used here after move
+LL | fn two_of_them(a: String) -> String {
+   |                - move occurs because `a` has type `String`, which does not implement the `Copy` trait
+LL |     dbg!(a, a);
+   |          -  ^ value used here after move
+   |          |
+   |          value moved here
    |
-help: consider borrowing instead of transferring ownership
+help: consider cloning the value if the performance cost is acceptable
    |
-LL |     my_dbg!(&b, 1);
-   |             +
-help: borrow this binding in the pattern to avoid moving the value
+LL |     dbg!(a.clone(), a);
+   |           ++++++++
+help: consider borrowing instead of transferring ownership
    |
-LL |             ref tmp => {
-   |             +++
+LL |     dbg!(&a, a);
+   |          +
 
 error[E0382]: use of moved value: `a`
-  --> $DIR/dbg-issue-120327.rs:57:12
+  --> $DIR/dbg-issue-120327.rs:33:12
    |
-LL |     let a = String::new();
-   |         - move occurs because `a` has type `String`, which does not implement the `Copy` trait
-LL |     let _b = match a {
-LL |         tmp => {
-   |         --- value moved here
+LL | fn two_of_them(a: String) -> String {
+   |                - move occurs because `a` has type `String`, which does not implement the `Copy` trait
+LL |     dbg!(a, a);
+   |             - value moved here
 ...
 LL |     return a;
    |            ^ value used here after move
    |
-help: borrow this binding in the pattern to avoid moving the value
+help: consider cloning the value if the performance cost is acceptable
+   |
+LL |     dbg!(a, a.clone());
+   |              ++++++++
+help: consider borrowing instead of transferring ownership
    |
-LL |         ref tmp => {
-   |         +++
+LL |     dbg!(a, &a);
+   |             +
 
 error[E0382]: borrow of moved value: `a`
-  --> $DIR/dbg-issue-120327.rs:65:14
+  --> $DIR/dbg-issue-120327.rs:42:14
    |
 LL |     let a: String = "".to_string();
    |         - move occurs because `a` has type `String`, which does not implement the `Copy` trait
@@ -111,6 +128,10 @@ help: consider cloning the value if the performance cost is acceptable
    |
 LL |     let _res = get_expr(dbg!(a.clone()));
    |                               ++++++++
+help: consider borrowing instead of transferring ownership
+   |
+LL |     let _res = get_expr(dbg!(&a));
+   |                              +
 
 error: aborting due to 7 previous errors
 
diff --git a/tests/ui/rfcs/rfc-2361-dbg-macro/dbg-macro-move-semantics.stderr b/tests/ui/rfcs/rfc-2361-dbg-macro/dbg-macro-move-semantics.stderr
@@ -16,6 +16,10 @@ LL | struct NoCopy(usize);
 ...
 LL |     let _ = dbg!(a);
    |                  - you could clone this value
+help: consider borrowing instead of transferring ownership
+   |
+LL |     let _ = dbg!(&a);
+   |                  +
 
 error: aborting due to 1 previous error
 
