Skip to content
Closed
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
31 changes: 18 additions & 13 deletions compiler/rustc_borrowck/src/diagnostics/conflict_errors.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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<'_>,
Expand All @@ -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",
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 @@ -763,6 +763,7 @@ symbols! {
custom_test_frameworks,
d,
d32,
dbg_macro,
dead_code,
dealloc,
debug,
Expand Down
9 changes: 7 additions & 2 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 @@ -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
Expand Down
87 changes: 64 additions & 23 deletions library/std/src/macros.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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)]
Expand Down Expand Up @@ -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),*)
)
},
}
67 changes: 67 additions & 0 deletions library/std/src/macros/tests.rs
Original file line number Diff line number Diff line change
@@ -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));
}
}
2 changes: 1 addition & 1 deletion src/ci/channel
Original file line number Diff line number Diff line change
@@ -1 +1 @@
stable
beta
64 changes: 36 additions & 28 deletions src/tools/clippy/clippy_lints/src/dbg_macro.rs
Original file line number Diff line number Diff line change
@@ -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};
Expand Down Expand Up @@ -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)
Expand All @@ -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()),
Expand All @@ -120,7 +129,6 @@ impl LateLintPass<'_> for DbgMacro {
);
(macro_call.span, format!("({snippet})"))
},
_ => unreachable!(),
};

diag.span_suggestion(
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 @@ -199,7 +199,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
2 changes: 1 addition & 1 deletion src/version
Original file line number Diff line number Diff line change
@@ -1 +1 @@
1.95.0
1.96.0
Loading
Loading