diff --git a/compiler/rustc_borrowck/src/diagnostics/conflict_errors.rs b/compiler/rustc_borrowck/src/diagnostics/conflict_errors.rs index f7d35f3ff3b4b..d712203c12b50 100644 --- a/compiler/rustc_borrowck/src/diagnostics/conflict_errors.rs +++ b/compiler/rustc_borrowck/src/diagnostics/conflict_errors.rs @@ -594,43 +594,38 @@ impl<'infcx, 'tcx> MirBorrowckCtxt<'_, 'infcx, 'tcx> { }); let Some(var_info) = var_info else { return }; let arg_name = var_info.name; - struct MatchArgFinder<'tcx> { + struct DbgArgFinder<'tcx> { tcx: TyCtxt<'tcx>, move_span: Span, arg_name: Symbol, - match_arg_span: Option = None, + dbg_arg_span: Option = 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(scrutinee, ..) = &e.kind - && let hir::ExprKind::Tup(args) = scrutinee.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], .. }, + )) = &arg.kind + && seg.ident.name == self.arg_name + && 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) }) }) { - 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; - } - } + self.dbg_arg_span = Some(path.span); + return; } hir::intravisit::walk_expr(self, e); } } - let mut finder = MatchArgFinder { tcx: self.infcx.tcx, move_span, 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", diff --git a/library/std/src/lib.rs b/library/std/src/lib.rs index 37bb7c8f77762..f96261019e793 100644 --- a/library/std/src/lib.rs +++ b/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) diff --git a/library/std/src/macros.rs b/library/std/src/macros.rs index a23aa0d877018..eee81c274e3da 100644 --- a/library/std/src/macros.rs +++ b/library/std/src/macros.rs @@ -362,57 +362,56 @@ macro_rules! dbg { }; } -/// Internal macro that processes a list of expressions, binds their results -/// with `match`, calls `eprint!` with the collected information, and returns -/// all the evaluated expressions in a tuple. +/// 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 -/// match (1, 2) { -/// args => { -/// let (tmp_1, tmp_2) = args; -/// eprint!("...", &tmp_1, &tmp_2, /* some other arguments */); -/// (tmp_1, tmp_2) -/// } +/// { +/// 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),+) ()) => { - // Use of `match` here is intentional because it affects the lifetimes - // of temporaries - https://stackoverflow.com/a/48732525/1063961 - // Always put the arguments in a tuple to avoid an unused parens lint on the pattern. - match ($($processed,)+) { - // Move the entire tuple so it doesn't stick around as a temporary (#154988). - args => { - let ($($bound,)+) = args; - $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),+) ()) => {{ + $(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") diff --git a/library/std/src/macros/tests.rs b/library/std/src/macros/tests.rs index 230bfdf3c9836..1ad4cf8c4c039 100644 --- a/library/std/src/macros/tests.rs +++ b/library/std/src/macros/tests.rs @@ -35,3 +35,33 @@ fn no_leaking_internal_temps_from_dbg() { // is dropped, then `foo`, then any temporaries left over from `dbg!` are dropped, if present. (drop(dbg!(bar)), drop(foo)); } + +/// Test for : +/// `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)); + } +} diff --git a/src/tools/clippy/clippy_lints/src/dbg_macro.rs b/src/tools/clippy/clippy_lints/src/dbg_macro.rs index 63968a8b5e04e..f6f6316763357 100644 --- a/src/tools/clippy/clippy_lints/src/dbg_macro.rs +++ b/src/tools/clippy/clippy_lints/src/dbg_macro.rs @@ -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,28 +112,23 @@ impl LateLintPass<'_> for DbgMacro { (macro_call.span, String::from("()")) } }, - ExprKind::Match(args, _, _) => { - let suggestion = match args.kind { - // dbg!(1) => 1 - ExprKind::Tup([val]) => { - snippet_with_applicability(cx, val.span.source_callsite(), "..", &mut applicability) - .to_string() - }, - // dbg!(2, 3) => (2, 3) - ExprKind::Tup([first, .., last]) => { - let snippet = snippet_with_applicability( - cx, - first.span.source_callsite().to(last.span.source_callsite()), - "..", - &mut applicability, - ); - format!("({snippet})") - }, - _ => unreachable!(), - }; + // dbg!(1) => 1 + [val] => { + let suggestion = + snippet_with_applicability(cx, val.span.source_callsite(), "..", &mut applicability) + .to_string(); (macro_call.span, suggestion) }, - _ => unreachable!(), + // 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( diff --git a/tests/ui/mismatched_types/mismatched-types-issue-126222.fixed b/tests/ui/mismatched_types/mismatched-types-issue-126222.fixed index 30fd0028f198f..d857a0f6c7761 100644 --- a/tests/ui/mismatched_types/mismatched-types-issue-126222.fixed +++ b/tests/ui/mismatched_types/mismatched-types-issue-126222.fixed @@ -12,7 +12,7 @@ fn main() { fn mismatch_types2() -> i32 { match 2 { x => { - return dbg!(x) //~ ERROR mismatched types + return dbg!(x); //~ ERROR mismatched types } } todo!() @@ -27,7 +27,7 @@ fn main() { fn mismatch_types4() -> i32 { match 1 { - _ => {return dbg!(1)} //~ ERROR mismatched types + _ => {return dbg!(1);} //~ ERROR mismatched types } todo!() } diff --git a/tests/ui/mismatched_types/mismatched-types-issue-126222.stderr b/tests/ui/mismatched_types/mismatched-types-issue-126222.stderr index 09ba15253dc58..ca38d8324c989 100644 --- a/tests/ui/mismatched_types/mismatched-types-issue-126222.stderr +++ b/tests/ui/mismatched_types/mismatched-types-issue-126222.stderr @@ -1,8 +1,11 @@ error[E0308]: mismatched types --> $DIR/mismatched-types-issue-126222.rs:7:18 | -LL | x => dbg!(x), - | ^^^^^^^ expected `()`, found integer +LL | / match 1 { +LL | | x => dbg!(x), + | | ^^^^^^^ expected `()`, found integer +LL | | } + | |_________- expected this to be `()` | help: you might have meant to return this value | @@ -12,19 +15,27 @@ LL | x => return dbg!(x), error[E0308]: mismatched types --> $DIR/mismatched-types-issue-126222.rs:15:17 | -LL | dbg!(x) - | ^^^^^^^ expected `()`, found integer +LL | / match 2 { +LL | | x => { +LL | | dbg!(x) + | | ^^^^^^^ expected `()`, found integer +LL | | } +LL | | } + | |_________- expected this to be `()` | help: you might have meant to return this value | -LL | return dbg!(x) - | ++++++ +LL | return dbg!(x); + | ++++++ + error[E0308]: mismatched types --> $DIR/mismatched-types-issue-126222.rs:23:18 | -LL | _ => dbg!(1) - | ^^^^^^^ expected `()`, found integer +LL | / match 1 { +LL | | _ => dbg!(1) + | | ^^^^^^^ expected `()`, found integer +LL | | } + | |_________- expected this to be `()` | help: you might have meant to return this value | @@ -34,13 +45,16 @@ LL | _ => return dbg!(1) error[E0308]: mismatched types --> $DIR/mismatched-types-issue-126222.rs:30:19 | -LL | _ => {dbg!(1)} - | ^^^^^^^ expected `()`, found integer +LL | / match 1 { +LL | | _ => {dbg!(1)} + | | ^^^^^^^ expected `()`, found integer +LL | | } + | |_________- expected this to be `()` | help: you might have meant to return this value | -LL | _ => {return dbg!(1)} - | ++++++ +LL | _ => {return dbg!(1);} + | ++++++ + error: aborting due to 4 previous errors