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
29 changes: 18 additions & 11 deletions compiler/rustc_borrowck/src/diagnostics/conflict_errors.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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",
Expand Down
1 change: 1 addition & 0 deletions compiler/rustc_span/src/symbol.rs
Original file line number Diff line number Diff line change
Expand Up @@ -753,6 +753,7 @@ symbols! {
custom_mir,
custom_test_frameworks,
d32,
dbg_macro,
dead_code,
dead_code_pub_in_binary,
dealloc,
Expand Down
4 changes: 3 additions & 1 deletion library/std/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
79 changes: 60 additions & 19 deletions library/std/src/macros.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand All @@ -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

@clarfonthey clarfonthey Jun 7, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I would clarify this further to the sequence:

  • dbg_internal!(() () (1, 2))
  • dbg_internal!(("...") (1 => tmp_1) (2))
  • dbg_internal!(("...", "...") (1 => tmp_1, 2 => tmp_2) ()

Followed by the code generated.

I would also note that the _1 and _2 suffixes are not actually used, and that we rely on macro hygiene to make multiple tmp idents distinct. Again, technically something you should be able to figure out from the implementation, but it speeds up reading it a bit more.

View changes since the review

/// ```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 */);

@clarfonthey clarfonthey Jun 7, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I would replace "..." with concat!("...", "...") to clarify that we do genuinely just concatenate all the strings together, so it's clear that the \n on each string is load-bearing.

View changes since the review

/// (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);+;

@clarfonthey clarfonthey Jun 7, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Minor nit, but I personally would prefer calling this just var or ident or really anything other than bound, since the term "bound" makes my mind immediately go toward trait bounds and that's not what this is. It's technically clear by itself, but that would make the macro code a bit easier to read.

View changes since the review

$(super let _ = ($bound = $processed));+;

@clarfonthey clarfonthey Jun 7, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think that this syntax is very confusing and should probably be explained a bit better. To me, I see this as the unit return value of the compound expression ($bound = $processed) being given higher scope, not $bound, which doesn't make any sense. I assume that some heavier lifting is being done but there should be some comments explaining why this works, so people will understand if we later change the semantics of super let before stabilisation and it breaks.

View changes since the review

$crate::eprint!(
$crate::concat!($($piece),+),

@clarfonthey clarfonthey Jun 7, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Similarly, I think piece should be reworded as maybe fmt_piece or something that clarifies that this is for the formatting string being used, since otherwise it immediately begs "piece of what?"

Again, this is clear from the implementation alone, but it would be nice to have a grasp on what is being done before reading the full implementation.

View changes since the review

$(
$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)]
Expand Down
98 changes: 54 additions & 44 deletions src/tools/clippy/clippy_lints/src/dbg_macro.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down
1 change: 0 additions & 1 deletion src/tools/clippy/clippy_utils/src/sym.rs
Original file line number Diff line number Diff line change
Expand Up @@ -202,7 +202,6 @@ 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: 15 additions & 38 deletions tests/ui/borrowck/dbg-issue-120327.rs
Original file line number Diff line number Diff line change
@@ -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
}

Expand Down
Loading
Loading