Auto merge of #157576 - dianne:dbg-via-super-let-2, r=<try>

avoid tearing `dbg!` prints, implemented using `super let`

Author: bors

compiler/rustc_borrowck/src/diagnostics/conflict_errors.rs

@@ -595,31 +595,38 @@ 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 DbgArgFinder<'tcx> {
+            tcx: TyCtxt<'tcx>,
+            move_span: Span,
             arg_name: Symbol,
+            dbg_arg_span: Option<Span> = None,
         }
-        impl Visitor<'_> for MatchArgFinder {
+        impl Visitor<'_> for DbgArgFinder<'_> {
             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
+                // dbg! is expanded into assignments, we need to find the right argument span
+                if let hir::ExprKind::Assign(_, arg, _) = &e.kind
                     && let hir::ExprKind::Path(hir::QPath::Resolved(
                         _,
                         path @ Path { segments: [seg], .. },
-                    )) = &expr.kind
+                    )) = &arg.kind
                     && seg.ident.name == self.arg_name
-                    && self.expr_span.source_callsite().contains(expr.span)
+                    && self.move_span.source_equal(arg.span)
+                    && 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);
+                    self.dbg_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 = DbgArgFinder { tcx: self.infcx.tcx, move_span, arg_name, .. };
         finder.visit_expr(body);
-        if let Some(macro_arg_span) = finder.match_arg_span {
+        if let Some(macro_arg_span) = finder.dbg_arg_span {
             err.span_suggestion_verbose(
                 macro_arg_span.shrink_to_lo(),
                 "consider borrowing instead of transferring ownership",

compiler/rustc_span/src/symbol.rs

@@ -753,6 +753,7 @@ symbols! {
         custom_mir,
         custom_test_frameworks,
         d32,
+        dbg_macro,
         dead_code,
         dead_code_pub_in_binary,
         dealloc,

library/std/src/lib.rs

@@ -496,7 +496,9 @@ extern crate std as realstd;
 
 // The standard macros that are not built-in to the compiler.
 #[macro_use]
-mod macros;
+#[doc(hidden)]
+#[unstable(feature = "std_internals", issue = "none")]
+pub mod macros;
 
 // The runtime entry point and a few unstable public functions used by the
 // compiler

library/std/src/macros.rs

@@ -350,6 +350,7 @@ macro_rules! eprintln {
 /// [`debug!`]: https://docs.rs/log/*/log/macro.debug.html
 /// [`log`]: https://crates.io/crates/log
 #[macro_export]
+#[allow_internal_unstable(std_internals)]
 #[cfg_attr(not(test), rustc_diagnostic_item = "dbg_macro")]
 #[stable(feature = "dbg_macro", since = "1.32.0")]
 macro_rules! dbg {
@@ -360,29 +361,69 @@ macro_rules! dbg {
     () => {
         $crate::eprintln!("[{}:{}:{}]", $crate::file!(), $crate::line!(), $crate::column!())
     };
-    ($val:expr $(,)?) => {
-        // Use of `match` here is intentional because it affects the lifetimes
-        // of temporaries - https://stackoverflow.com/a/48732525/1063961
-        match $val {
-            tmp => {
-                $crate::eprintln!("[{}:{}:{}] {} = {:#?}",
-                    $crate::file!(),
-                    $crate::line!(),
-                    $crate::column!(),
-                    $crate::stringify!($val),
-                    // The `&T: Debug` check happens here (not in the format literal desugaring)
-                    // to avoid format literal related messages and suggestions.
-                    &&tmp as &dyn $crate::fmt::Debug,
-                );
-                tmp
-            }
-        }
-    };
     ($($val:expr),+ $(,)?) => {
-        ($($crate::dbg!($val)),+,)
+        $crate::macros::dbg_internal!(() () ($($val),+))
     };
 }
 
+/// Internal macro that processes a list of expressions, binds their results, calls `eprint!` with
+/// the collected information, and returns all the evaluated expressions in a tuple.
+///
+/// E.g. `dbg_internal!(() () (1, 2))` expands into
+/// ```rust, ignore
+/// {
+///     let tmp_1;
+///     let tmp_2;
+///     super let _ = (tmp_1 = 1);
+///     super let _ = (tmp_2 = 2);
+///     eprint!("...", &tmp_1, &tmp_2, /* some other arguments */);
+///     (tmp_1, tmp_2)
+/// }
+/// ```
+///
+/// This is necessary so that `dbg!` outputs don't get torn, see #136703.
+/// `super let` is used to avoid creating a temporary scope around `dbg!`'s arguments. Nested
+/// `match` is insufficient because match arms introduce temporary scopes (#153850) and using a
+/// single match on a tuple containing all the arguments is insufficient because of an imprecision
+/// in the borrow-checker (see #155902).
+#[doc(hidden)]
+#[allow_internal_unstable(std_internals, super_let)]
+#[rustc_macro_transparency = "semiopaque"]
+#[unstable(feature = "std_internals", issue = "none")]
+pub macro dbg_internal {
+    (($($piece:literal),+) ($($processed:expr => $bound:ident),+) ()) => {{
+        $(let $bound);+;
+        $(super let _ = ($bound = $processed));+;
+        $crate::eprint!(
+            $crate::concat!($($piece),+),
+            $(
+                $crate::stringify!($processed),
+                // The `&T: Debug` check happens here (not in the format literal desugaring)
+                // to avoid format literal related messages and suggestions.
+                &&$bound as &dyn $crate::fmt::Debug
+            ),+,
+            // The location returned here is that of the macro invocation, so
+            // it will be the same for all expressions. Thus, label these
+            // arguments so that they can be reused in every piece of the
+            // formatting template.
+            file=$crate::file!(),
+            line=$crate::line!(),
+            column=$crate::column!()
+        );
+        // Comma separate the variables only when necessary so that this will
+        // not yield a tuple for a single expression, but rather just parenthesize
+        // the expression.
+        ($($bound),+)
+    }},
+    (($($piece:literal),*) ($($processed:expr => $bound:ident),*) ($val:expr $(,$rest:expr)*)) => {
+        $crate::macros::dbg_internal!(
+            ($($piece,)* "[{file}:{line}:{column}] {} = {:#?}\n")
+            ($($processed => $bound,)* $val => tmp)
+            ($($rest),*)
+        )
+    },
+}
+
 #[doc(hidden)]
 #[macro_export]
 #[allow_internal_unstable(hash_map_internals)]

src/tools/clippy/clippy_lints/src/dbg_macro.rs

@@ -75,51 +75,61 @@ impl LateLintPass<'_> for DbgMacro {
                 "the `dbg!` macro is intended as a debugging tool",
                 |diag| {
                     let mut applicability = Applicability::MachineApplicable;
-                    let (sugg_span, suggestion) =
-                        match is_async_move_desugar(expr).unwrap_or(expr).peel_drop_temps().kind {
-                            // dbg!()
-                            ExprKind::Block(..) => {
-                                // If the `dbg!` macro is a "free" statement and not contained within other expressions,
-                                // remove the whole statement.
-                                if let Node::Stmt(_) = cx.tcx.parent_hir_node(expr.hir_id)
-                                    && let Some(semi_span) =
-                                        cx.sess().source_map().mac_call_stmt_semi_span(macro_call.span)
-                                {
-                                    (macro_call.span.to(semi_span), String::new())
-                                } else {
-                                    (macro_call.span, String::from("()"))
-                                }
-                            },
-                            // dbg!(1)
-                            ExprKind::Match(val, ..) => (
-                                macro_call.span,
+                    let dbg_expn = is_async_move_desugar(expr).unwrap_or(expr).peel_drop_temps();
+                    // `dbg!` always expands to a block. If it was given arguments, it assigns names to them
+                    // using `super let _ = (tmp = $arg);` statements.
+                    let ExprKind::Block(block, _) = dbg_expn.kind else {
+                        unreachable!()
+                    };
+                    let args: Vec<_> = block
+                        .stmts
+                        .iter()
+                        .filter_map(|stmt| {
+                            if let StmtKind::Let(LetStmt {
+                                super_: Some(_),
+                                init: Some(init),
+                                ..
+                            }) = stmt.kind
+                                && let ExprKind::Assign(_, arg, _) = init.kind
+                            {
+                                Some(arg)
+                            } else {
+                                None
+                            }
+                        })
+                        .collect();
+
+                    let (sugg_span, suggestion) = match args.as_slice() {
+                        // dbg!()
+                        [] => {
+                            // If the `dbg!` macro is a "free" statement and not contained within other expressions,
+                            // remove the whole statement.
+                            if let Node::Stmt(_) = cx.tcx.parent_hir_node(expr.hir_id)
+                                && let Some(semi_span) = cx.sess().source_map().mac_call_stmt_semi_span(macro_call.span)
+                            {
+                                (macro_call.span.to(semi_span), String::new())
+                            } else {
+                                (macro_call.span, String::from("()"))
+                            }
+                        },
+                        // dbg!(1) => 1
+                        [val] => {
+                            let suggestion =
                                 snippet_with_applicability(cx, val.span.source_callsite(), "..", &mut applicability)
-                                    .to_string(),
-                            ),
-                            // dbg!(2, 3)
-                            ExprKind::Tup(
-                                [
-                                    Expr {
-                                        kind: ExprKind::Match(first, ..),
-                                        ..
-                                    },
-                                    ..,
-                                    Expr {
-                                        kind: ExprKind::Match(last, ..),
-                                        ..
-                                    },
-                                ],
-                            ) => {
-                                let snippet = snippet_with_applicability(
-                                    cx,
-                                    first.span.source_callsite().to(last.span.source_callsite()),
-                                    "..",
-                                    &mut applicability,
-                                );
-                                (macro_call.span, format!("({snippet})"))
-                            },
-                            _ => unreachable!(),
-                        };
+                                    .to_string();
+                            (macro_call.span, suggestion)
+                        },
+                        // dbg!(2, 3) => (2, 3)
+                        [first, .., last] => {
+                            let snippet = snippet_with_applicability(
+                                cx,
+                                first.span.source_callsite().to(last.span.source_callsite()),
+                                "..",
+                                &mut applicability,
+                            );
+                            (macro_call.span, format!("({snippet})"))
+                        },
+                    };
 
                     diag.span_suggestion(
                         sugg_span,

src/tools/clippy/clippy_utils/src/sym.rs

@@ -202,7 +202,6 @@ generate! {
     cx,
     cycle,
     cyclomatic_complexity,
-    dbg_macro,
     de,
     debug_struct,
     deprecated_in_future,

src/tools/miri/tests/fail/dangling_pointers/dangling_primitive.stderr

@@ -2,7 +2,7 @@ error: Undefined Behavior: memory access failed: ALLOC has been freed, so this p
   --> tests/fail/dangling_pointers/dangling_primitive.rs:LL:CC
    |
 LL |         dbg!(*ptr);
-   |         ^^^^^^^^^^ Undefined Behavior occurred here
+   |              ^^^^ Undefined Behavior occurred here
    |
    = help: this indicates a bug in the program: it performed an invalid operation, and caused Undefined Behavior
    = help: see https://doc.rust-lang.org/nightly/reference/behavior-considered-undefined.html for further information

src/tools/miri/tests/fail/function_calls/return_pointer_on_unwind.stderr

@@ -7,7 +7,7 @@ error: Undefined Behavior: reading memory at ALLOC[0x0..0x4], but memory is unin
   --> tests/fail/function_calls/return_pointer_on_unwind.rs:LL:CC
    |
 LL |     dbg!(x.0);
-   |     ^^^^^^^^^ Undefined Behavior occurred here
+   |          ^^^ Undefined Behavior occurred here
    |
    = help: this indicates a bug in the program: it performed an invalid operation, and caused Undefined Behavior
    = help: see https://doc.rust-lang.org/nightly/reference/behavior-considered-undefined.html for further information

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
 }