Auto merge of #156106 - dianne:dbg-crater, r=<try>

[do not merge] crater `dbg!` changes against stable-1.95.0

Author: bors

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,31 +558,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

@@ -763,6 +763,7 @@ symbols! {
         custom_test_frameworks,
         d,
         d32,
+        dbg_macro,
         dead_code,
         dealloc,
         debug,

library/std/src/lib.rs

@@ -255,7 +255,10 @@
 #![allow(unused_features)]
 //
 // Features:
-#![cfg_attr(test, feature(internal_output_capture, print_internals, update_panic_count, rt))]
+#![cfg_attr(
+    test,
+    feature(internal_output_capture, print_internals, super_let, update_panic_count, rt)
+)]
 #![cfg_attr(
     all(target_vendor = "fortanix", target_env = "sgx"),
     feature(slice_index_methods, coerce_unsized, sgx_platform)
@@ -469,7 +472,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

@@ -5,6 +5,9 @@
 //! library.
 // ignore-tidy-dbg
 
+#[cfg(test)]
+mod tests;
+
 #[doc = include_str!("../../core/src/macros/panic.md")]
 #[macro_export]
 #[rustc_builtin_macro(std_panic)]
@@ -347,35 +350,73 @@ 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 {
-    // NOTE: We cannot use `concat!` to make a static string as a format argument
-    // of `eprintln!` because `file!` could contain a `{` or
-    // `$val` expression could be a block (`{ .. }`), in which case the `eprintln!`
-    // will be malformed.
     () => {
         $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 the borrow checker
+/// thinks that tuple can outlive the `dbg!` invocation if dropping the temporary places the tuple's
+/// elements were moved out of panics (not actually possible; they've been moved from). 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),*)
+        )
+    },
+}

library/std/src/macros/tests.rs

@@ -0,0 +1,67 @@
+// ignore-tidy-dbg
+
+use core::fmt::Debug;
+
+/// Test for <https://github.com/rust-lang/rust/issues/153850>:
+/// `dbg!` shouldn't drop arguments' temporaries.
+#[test]
+fn no_dropping_temps() {
+    fn temp() {}
+
+    *dbg!(&temp());
+    *dbg!(&temp(), 1).0;
+    *dbg!(0, &temp()).1;
+    *dbg!(0, &temp(), 2).1;
+}
+
+/// Test for <https://github.com/rust-lang/rust/issues/154988>:
+/// `dbg!` shouldn't create a temporary that lives past its invocation.
+#[test]
+fn no_leaking_internal_temps_from_dbg() {
+    #[derive(Debug)]
+    struct Foo;
+
+    #[derive(Debug)]
+    struct Bar<'a>(#[allow(unused)] &'a Foo);
+    impl Drop for Bar<'_> {
+        fn drop(&mut self) {}
+    }
+
+    let foo = Foo;
+    let bar = Bar(&foo);
+    // If `dbg!` creates a `(Bar<'_>,)` temporary that lasts past its expansion, this will fail
+    // to compile, because it will be dropped after `foo`, which it borrows from. The tuple
+    // mimics the drop order of block tail expressions before Rust 2024: first the result of `dbg!`
+    // is dropped, then `foo`, then any temporaries left over from `dbg!` are dropped, if present.
+    (drop(dbg!(bar)), drop(foo));
+}
+
+/// Test for <https://github.com/rust-lang/rust/issues/155902>:
+/// `dbg!` shouldn't create a temporary that borrowck thinks can live past its invocation on a false
+/// unwind path.
+#[test]
+fn no_leaking_internal_temps_from_dbg_even_on_false_unwind() {
+    #[derive(Debug)]
+    struct Foo;
+    impl Drop for Foo {
+        fn drop(&mut self) {}
+    }
+
+    #[derive(Debug)]
+    struct Bar<'a>(#[allow(unused)] &'a Foo);
+    impl Drop for Bar<'_> {
+        fn drop(&mut self) {}
+    }
+
+    {
+        let foo = Foo;
+        let bar = Bar(&foo);
+        // The temporaries of this `super let`'s scrutinee will outlive `bar` and `foo`, emulating
+        // the drop order of block tail expressions before Rust 2024. If borrowck thinks that a
+        // panic from moving `bar` is possible and that a `Bar<'_>`-containing temporary lives past
+        // the end of the block because of that, this will fail to compile. Because `Foo` implements
+        // `Drop`, it's an error for `foo` to be dropped before such a temporary when unwinding;
+        // otherwise, `foo` would just live to the end of the stack frame when unwinding.
+        super let _ = drop(dbg!(bar));
+    }
+}

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

@@ -1,8 +1,8 @@
 use clippy_config::Conf;
 use clippy_utils::diagnostics::span_lint_and_then;
-use clippy_utils::{is_in_test, sym};
 use clippy_utils::macros::{MacroCall, macro_backtrace};
 use clippy_utils::source::snippet_with_applicability;
+use clippy_utils::{is_in_test, sym};
 use rustc_data_structures::fx::FxHashSet;
 use rustc_errors::Applicability;
 use rustc_hir::{Closure, ClosureKind, CoroutineKind, Expr, ExprKind, LetStmt, LocalSource, Node, Stmt, StmtKind};
@@ -75,13 +75,33 @@ 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
-                    {
+                    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!()
-                        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)
@@ -92,26 +112,15 @@ impl LateLintPass<'_> for DbgMacro {
                                 (macro_call.span, String::from("()"))
                             }
                         },
-                        // dbg!(1)
-                        ExprKind::Match(val, ..) => (
-                            macro_call.span,
-                            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, ..),
-                                    ..
-                                },
-                            ],
-                        ) => {
+                        // dbg!(1) => 1
+                        [val] => {
+                            let suggestion =
+                                snippet_with_applicability(cx, val.span.source_callsite(), "..", &mut applicability)
+                                    .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()),
@@ -120,7 +129,6 @@ impl LateLintPass<'_> for DbgMacro {
                             );
                             (macro_call.span, format!("({snippet})"))
                         },
-                        _ => unreachable!(),
                     };
 
                     diag.span_suggestion(

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,

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