Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
40 changes: 15 additions & 25 deletions compiler/rustc_borrowck/src/diagnostics/conflict_errors.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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<'_>,
Expand All @@ -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<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(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(
Expand Down
1 change: 0 additions & 1 deletion compiler/rustc_span/src/symbol.rs
Original file line number Diff line number Diff line change
Expand Up @@ -752,7 +752,6 @@ symbols! {
custom_mir,
custom_test_frameworks,
d32,
dbg_macro,
dead_code,
dead_code_pub_in_binary,
dealloc,
Expand Down
9 changes: 5 additions & 4 deletions library/std/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down Expand Up @@ -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
Expand Down
79 changes: 20 additions & 59 deletions library/std/src/macros.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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)]
Expand Down
30 changes: 30 additions & 0 deletions library/std/src/macros/tests.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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 <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));
}
}
43 changes: 25 additions & 18 deletions src/tools/clippy/clippy_lints/src/dbg_macro.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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!(),
};
Expand Down
1 change: 1 addition & 0 deletions src/tools/clippy/clippy_utils/src/sym.rs
Original file line number Diff line number Diff line change
Expand Up @@ -202,6 +202,7 @@ generate! {
cx,
cycle,
cyclomatic_complexity,
dbg_macro,
de,
debug_struct,
deprecated_in_future,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
53 changes: 38 additions & 15 deletions tests/ui/borrowck/dbg-issue-120327.rs
Original file line number Diff line number Diff line change
@@ -1,44 +1,67 @@
//! 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); //~ 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
}

Expand Down
Loading
Loading