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
35 changes: 15 additions & 20 deletions compiler/rustc_borrowck/src/diagnostics/conflict_errors.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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<Span> = None,
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(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",
Expand Down
5 changes: 4 additions & 1 deletion 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
81 changes: 40 additions & 41 deletions library/std/src/macros.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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")
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));
}
}
67 changes: 41 additions & 26 deletions src/tools/clippy/clippy_lints/src/dbg_macro.rs
Original file line number Diff line number Diff line change
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,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(
Expand Down
4 changes: 2 additions & 2 deletions tests/ui/mismatched_types/mismatched-types-issue-126222.fixed
Original file line number Diff line number Diff line change
Expand Up @@ -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!()
Expand All @@ -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!()
}
Expand Down
38 changes: 26 additions & 12 deletions tests/ui/mismatched_types/mismatched-types-issue-126222.stderr
Original file line number Diff line number Diff line change
@@ -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
|
Expand All @@ -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
|
Expand All @@ -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

Expand Down
Loading