diff --git a/compiler/rustc_borrowck/src/diagnostics/conflict_errors.rs b/compiler/rustc_borrowck/src/diagnostics/conflict_errors.rs index fcf4bf4a7b7a4..9a64a063fcf5c 100644 --- a/compiler/rustc_borrowck/src/diagnostics/conflict_errors.rs +++ b/compiler/rustc_borrowck/src/diagnostics/conflict_errors.rs @@ -580,6 +580,8 @@ 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<'_>, @@ -593,41 +595,29 @@ impl<'infcx, 'tcx> MirBorrowckCtxt<'_, 'infcx, 'tcx> { }); let Some(var_info) = var_info else { return }; let arg_name = var_info.name; - struct MatchArgFinder<'tcx> { - tcx: TyCtxt<'tcx>, - move_span: Span, + struct MatchArgFinder { + expr_span: Span, + match_arg_span: Option, arg_name: Symbol, - match_arg_span: Option = 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(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) - }) - }) + 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) { - 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.match_arg_span = Some(path.span); } hir::intravisit::walk_expr(self, e); } } - let mut finder = MatchArgFinder { tcx: self.infcx.tcx, move_span, arg_name, .. }; + let mut finder = MatchArgFinder { expr_span: move_span, match_arg_span: None, 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 index 69ed7314855f9..9cc1c927427e3 100644 --- a/compiler/rustc_span/src/symbol.rs +++ b/compiler/rustc_span/src/symbol.rs @@ -752,7 +752,6 @@ symbols! { custom_mir, custom_test_frameworks, d32, - dbg_macro, dead_code, dead_code_pub_in_binary, dealloc, diff --git a/library/std/src/lib.rs b/library/std/src/lib.rs index 7a548dfb33da7..4c282af401b92 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) @@ -488,9 +491,7 @@ extern crate std as realstd; // The standard macros that are not built-in to the compiler. #[macro_use] -#[doc(hidden)] -#[unstable(feature = "std_internals", issue = "none")] -pub mod macros; +mod macros; // The runtime entry point and a few unstable public functions used by the // compiler diff --git a/library/std/src/macros.rs b/library/std/src/macros.rs index a23aa0d877018..8bcf870e9aeb4 100644 --- a/library/std/src/macros.rs +++ b/library/std/src/macros.rs @@ -350,76 +350,37 @@ 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),+ $(,)?) => { - $crate::macros::dbg_internal!(() () ($($val),+)) - }; -} - -/// 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. -/// -/// 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) -/// } -/// } -/// ``` -/// -/// This is necessary so that `dbg!` outputs don't get torn, see #136703. -#[doc(hidden)] -#[rustc_macro_transparency = "semiopaque"] -pub macro dbg_internal { - (($($piece:literal),+) ($($processed:expr => $bound:ident),+) ()) => { + ($val:expr $(,)?) => { // 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!() + 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, ); - // 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),+) - + tmp } } - }, - (($($piece:literal),*) ($($processed:expr => $bound:ident),*) ($val:expr $(,$rest:expr)*)) => { - $crate::macros::dbg_internal!( - ($($piece,)* "[{file}:{line}:{column}] {} = {:#?}\n") - ($($processed => $bound,)* $val => tmp) - ($($rest),*) - ) - }, + }; + ($($val:expr),+ $(,)?) => { + ($($crate::dbg!($val)),+,) + }; } #[doc(hidden)] 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..29d0b47002679 100644 --- a/src/tools/clippy/clippy_lints/src/dbg_macro.rs +++ b/src/tools/clippy/clippy_lints/src/dbg_macro.rs @@ -92,26 +92,33 @@ 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!(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, ..), + .. }, - // 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})") + .., + Expr { + kind: ExprKind::Match(last, ..), + .. }, - _ => unreachable!(), - }; - (macro_call.span, suggestion) + ], + ) => { + let snippet = snippet_with_applicability( + cx, + first.span.source_callsite().to(last.span.source_callsite()), + "..", + &mut applicability, + ); + (macro_call.span, format!("({snippet})")) }, _ => unreachable!(), }; diff --git a/src/tools/clippy/clippy_utils/src/sym.rs b/src/tools/clippy/clippy_utils/src/sym.rs index 6a7637fb18585..492eb89708aeb 100644 --- a/src/tools/clippy/clippy_utils/src/sym.rs +++ b/src/tools/clippy/clippy_utils/src/sym.rs @@ -202,6 +202,7 @@ generate! { cx, cycle, cyclomatic_complexity, + dbg_macro, de, debug_struct, deprecated_in_future, diff --git a/src/tools/miri/tests/fail/dangling_pointers/dangling_primitive.stderr b/src/tools/miri/tests/fail/dangling_pointers/dangling_primitive.stderr index 4f06a1afa50fe..afc2cb214842e 100644 --- a/src/tools/miri/tests/fail/dangling_pointers/dangling_primitive.stderr +++ b/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 diff --git a/src/tools/miri/tests/fail/function_calls/return_pointer_on_unwind.stderr b/src/tools/miri/tests/fail/function_calls/return_pointer_on_unwind.stderr index 88d5694c4736d..364a4b4a74418 100644 --- a/src/tools/miri/tests/fail/function_calls/return_pointer_on_unwind.stderr +++ b/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 diff --git a/tests/ui/borrowck/dbg-issue-120327.rs b/tests/ui/borrowck/dbg-issue-120327.rs index 45605acc34334..2de43f634877a 100644 --- a/tests/ui/borrowck/dbg-issue-120327.rs +++ b/tests/ui/borrowck/dbg-issue-120327.rs @@ -1,44 +1,67 @@ -//! Diagnostic test for : suggest borrowing -//! variables passed to `dbg!` that are later used. -//@ dont-require-annotations: HELP - fn s() -> String { let a = String::new(); - dbg!(a); //~ HELP consider borrowing instead of transferring ownership + dbg!(a); return a; //~ ERROR use of moved value: } fn m() -> String { let a = String::new(); - dbg!(1, 2, a, 1, 2); //~ HELP consider borrowing instead of transferring ownership + dbg!(1, 2, a, 1, 2); return a; //~ ERROR use of moved value: } fn t(a: String) -> String { let b: String = "".to_string(); - dbg!(a, b); //~ HELP consider borrowing instead of transferring ownership + dbg!(a, b); return b; //~ ERROR use of moved value: } fn x(a: String) -> String { let b: String = "".to_string(); - dbg!(a, b); //~ HELP consider borrowing instead of transferring ownership + dbg!(a, b); 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 +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 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)); //~ HELP consider borrowing instead of transferring ownership + let _res = get_expr(dbg!(a)); 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 index e7a7151e541dd..efacc0c3f1341 100644 --- a/tests/ui/borrowck/dbg-issue-120327.stderr +++ b/tests/ui/borrowck/dbg-issue-120327.stderr @@ -1,133 +1,112 @@ error[E0382]: use of moved value: `a` - --> $DIR/dbg-issue-120327.rs:8:12 + --> $DIR/dbg-issue-120327.rs:4:12 | LL | let a = String::new(); | - move occurs because `a` has type `String`, which does not implement the `Copy` trait LL | dbg!(a); - | - value moved here + | ------- value moved here LL | return a; | ^ value used here after move | -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:14:12 + --> $DIR/dbg-issue-120327.rs:10:12 | LL | let a = String::new(); | - move occurs because `a` has type `String`, which does not implement the `Copy` trait LL | dbg!(1, 2, a, 1, 2); - | - value moved here + | ------------------- value moved here LL | return a; | ^ value used here after move | -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:20:12 + --> $DIR/dbg-issue-120327.rs:16:12 | LL | let b: String = "".to_string(); | - move occurs because `b` has type `String`, which does not implement the `Copy` trait LL | dbg!(a, b); - | - value moved here + | ---------- value moved here LL | return b; | ^ value used here after move | -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:26:12 + --> $DIR/dbg-issue-120327.rs:22:12 | LL | fn x(a: String) -> String { | - move occurs because `a` has type `String`, which does not implement the `Copy` trait LL | let b: String = "".to_string(); LL | dbg!(a, b); - | - value moved here + | ---------- value moved here LL | return a; | ^ value used here after move | -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: `a` - --> $DIR/dbg-issue-120327.rs:30:13 - | -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 +error[E0382]: use of moved value: `b` + --> $DIR/dbg-issue-120327.rs:46:12 | -help: consider cloning the value if the performance cost is acceptable +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 | dbg!(a.clone(), a); - | ++++++++ help: consider borrowing instead of transferring ownership | -LL | dbg!(&a, a); - | + +LL | my_dbg!(&b, 1); + | + +help: borrow this binding in the pattern to avoid moving the value + | +LL | ref tmp => { + | +++ error[E0382]: use of moved value: `a` - --> $DIR/dbg-issue-120327.rs:33:12 + --> $DIR/dbg-issue-120327.rs:57:12 | -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 | 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 | return a; | ^ value used here after move | -help: consider cloning the value if the performance cost is acceptable - | -LL | dbg!(a, a.clone()); - | ++++++++ -help: consider borrowing instead of transferring ownership +help: borrow this binding in the pattern to avoid moving the value | -LL | dbg!(a, &a); - | + +LL | ref tmp => { + | +++ error[E0382]: borrow of moved value: `a` - --> $DIR/dbg-issue-120327.rs:42:14 + --> $DIR/dbg-issue-120327.rs:65:14 | LL | let a: String = "".to_string(); | - move occurs because `a` has type `String`, which does not implement the `Copy` trait LL | let _res = get_expr(dbg!(a)); - | - value moved here + | ------- value moved here LL | let _l = a.len(); | ^ value borrowed here after move | -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)); diff --git a/tests/ui/liveness/liveness-upvars.rs b/tests/ui/liveness/liveness-upvars.rs index 0e198f1dea10b..be58b48a40576 100644 --- a/tests/ui/liveness/liveness-upvars.rs +++ b/tests/ui/liveness/liveness-upvars.rs @@ -98,7 +98,7 @@ pub fn g(mut v: T) { } pub fn h() { - let mut z = T::default(); //~ WARN unused variable: `z` + let mut z = T::default(); let _ = move |b| { loop { if b { diff --git a/tests/ui/liveness/liveness-upvars.stderr b/tests/ui/liveness/liveness-upvars.stderr index 9f5a3de7365a3..96a922772c919 100644 --- a/tests/ui/liveness/liveness-upvars.stderr +++ b/tests/ui/liveness/liveness-upvars.stderr @@ -157,14 +157,6 @@ LL | z = T::default(); | = help: maybe it is overwritten before being read? -warning: unused variable: `z` - --> $DIR/liveness-upvars.rs:101:9 - | -LL | let mut z = T::default(); - | ^^^^^ help: if this is intentional, prefix it with an underscore: `_z` - | - = note: `#[warn(unused_variables)]` implied by `#[warn(unused)]` - warning: value captured by `state` is never read --> $DIR/liveness-upvars.rs:131:9 | @@ -206,5 +198,5 @@ LL | s = yield (); LL | s = 3; | ----- `s` is overwritten here before the previous value is read -warning: 25 warnings emitted +warning: 24 warnings emitted 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 index 7ce5ebf81e310..f8ef315b9cc78 100644 --- 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 @@ -1,21 +1,13 @@ error[E0382]: use of moved value: `a` - --> $DIR/dbg-macro-move-semantics.rs:9:18 + --> $DIR/dbg-macro-move-semantics.rs:9:13 | LL | let a = NoCopy(0); | - move occurs because `a` has type `NoCopy`, which does not implement the `Copy` trait LL | let _ = dbg!(a); - | - value moved here + | ------- value moved here LL | let _ = dbg!(a); - | ^ value used here after move + | ^^^^^^^ value used here after move | -note: if `NoCopy` implemented `Clone`, you could clone the value - --> $DIR/dbg-macro-move-semantics.rs:4:1 - | -LL | struct NoCopy(usize); - | ^^^^^^^^^^^^^ consider implementing `Clone` for this type -... -LL | let _ = dbg!(a); - | - you could clone this value help: consider borrowing instead of transferring ownership | LL | let _ = dbg!(&a);