Skip to content

Commit 286782c

Browse files
committed
Auto merge of #156772 - cuviper:beta-next, r=cuviper
[beta] Revert tearing changes to dbg! (#156589) This is a straight cherry-pick of the beta-approved #156589.
2 parents 6be5f81 + 10a3a2b commit 286782c

14 files changed

Lines changed: 176 additions & 201 deletions

File tree

compiler/rustc_borrowck/src/diagnostics/conflict_errors.rs

Lines changed: 15 additions & 25 deletions
Original file line numberDiff line numberDiff line change
@@ -588,6 +588,8 @@ impl<'infcx, 'tcx> MirBorrowckCtxt<'_, 'infcx, 'tcx> {
588588
}
589589

590590
// for dbg!(x) which may take ownership, suggest dbg!(&x) instead
591+
// but here we actually do not check whether the macro name is `dbg!`
592+
// so that we may extend the scope a bit larger to cover more cases
591593
fn suggest_ref_for_dbg_args(
592594
&self,
593595
body: &hir::Expr<'_>,
@@ -601,41 +603,29 @@ impl<'infcx, 'tcx> MirBorrowckCtxt<'_, 'infcx, 'tcx> {
601603
});
602604
let Some(var_info) = var_info else { return };
603605
let arg_name = var_info.name;
604-
struct MatchArgFinder<'tcx> {
605-
tcx: TyCtxt<'tcx>,
606-
move_span: Span,
606+
struct MatchArgFinder {
607+
expr_span: Span,
608+
match_arg_span: Option<Span>,
607609
arg_name: Symbol,
608-
match_arg_span: Option<Span> = None,
609610
}
610-
impl Visitor<'_> for MatchArgFinder<'_> {
611+
impl Visitor<'_> for MatchArgFinder {
611612
fn visit_expr(&mut self, e: &hir::Expr<'_>) {
612613
// dbg! is expanded into a match pattern, we need to find the right argument span
613-
if let hir::ExprKind::Match(scrutinee, ..) = &e.kind
614-
&& let hir::ExprKind::Tup(args) = scrutinee.kind
615-
&& e.span.macro_backtrace().any(|expn| {
616-
expn.macro_def_id.is_some_and(|macro_def_id| {
617-
self.tcx.is_diagnostic_item(sym::dbg_macro, macro_def_id)
618-
})
619-
})
614+
if let hir::ExprKind::Match(expr, ..) = &e.kind
615+
&& let hir::ExprKind::Path(hir::QPath::Resolved(
616+
_,
617+
path @ Path { segments: [seg], .. },
618+
)) = &expr.kind
619+
&& seg.ident.name == self.arg_name
620+
&& self.expr_span.source_callsite().contains(expr.span)
620621
{
621-
for arg in args {
622-
if let hir::ExprKind::Path(hir::QPath::Resolved(
623-
_,
624-
path @ Path { segments: [seg], .. },
625-
)) = &arg.kind
626-
&& seg.ident.name == self.arg_name
627-
&& self.move_span.source_equal(arg.span)
628-
{
629-
self.match_arg_span = Some(path.span);
630-
return;
631-
}
632-
}
622+
self.match_arg_span = Some(path.span);
633623
}
634624
hir::intravisit::walk_expr(self, e);
635625
}
636626
}
637627

638-
let mut finder = MatchArgFinder { tcx: self.infcx.tcx, move_span, arg_name, .. };
628+
let mut finder = MatchArgFinder { expr_span: move_span, match_arg_span: None, arg_name };
639629
finder.visit_expr(body);
640630
if let Some(macro_arg_span) = finder.match_arg_span {
641631
err.span_suggestion_verbose(

compiler/rustc_span/src/symbol.rs

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -748,7 +748,6 @@ symbols! {
748748
custom_mir,
749749
custom_test_frameworks,
750750
d32,
751-
dbg_macro,
752751
dead_code,
753752
dealloc,
754753
debug,

library/std/src/lib.rs

Lines changed: 5 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -255,7 +255,10 @@
255255
#![allow(unused_features)]
256256
//
257257
// Features:
258-
#![cfg_attr(test, feature(internal_output_capture, print_internals, update_panic_count, rt))]
258+
#![cfg_attr(
259+
test,
260+
feature(internal_output_capture, print_internals, super_let, update_panic_count, rt)
261+
)]
259262
#![cfg_attr(
260263
all(target_vendor = "fortanix", target_env = "sgx"),
261264
feature(slice_index_methods, coerce_unsized, sgx_platform)
@@ -471,9 +474,7 @@ extern crate std as realstd;
471474

472475
// The standard macros that are not built-in to the compiler.
473476
#[macro_use]
474-
#[doc(hidden)]
475-
#[unstable(feature = "std_internals", issue = "none")]
476-
pub mod macros;
477+
mod macros;
477478

478479
// The runtime entry point and a few unstable public functions used by the
479480
// compiler

library/std/src/macros.rs

Lines changed: 20 additions & 59 deletions
Original file line numberDiff line numberDiff line change
@@ -350,76 +350,37 @@ macro_rules! eprintln {
350350
/// [`debug!`]: https://docs.rs/log/*/log/macro.debug.html
351351
/// [`log`]: https://crates.io/crates/log
352352
#[macro_export]
353-
#[allow_internal_unstable(std_internals)]
354353
#[cfg_attr(not(test), rustc_diagnostic_item = "dbg_macro")]
355354
#[stable(feature = "dbg_macro", since = "1.32.0")]
356355
macro_rules! dbg {
356+
// NOTE: We cannot use `concat!` to make a static string as a format argument
357+
// of `eprintln!` because `file!` could contain a `{` or
358+
// `$val` expression could be a block (`{ .. }`), in which case the `eprintln!`
359+
// will be malformed.
357360
() => {
358361
$crate::eprintln!("[{}:{}:{}]", $crate::file!(), $crate::line!(), $crate::column!())
359362
};
360-
($($val:expr),+ $(,)?) => {
361-
$crate::macros::dbg_internal!(() () ($($val),+))
362-
};
363-
}
364-
365-
/// Internal macro that processes a list of expressions, binds their results
366-
/// with `match`, calls `eprint!` with the collected information, and returns
367-
/// all the evaluated expressions in a tuple.
368-
///
369-
/// E.g. `dbg_internal!(() () (1, 2))` expands into
370-
/// ```rust, ignore
371-
/// match (1, 2) {
372-
/// args => {
373-
/// let (tmp_1, tmp_2) = args;
374-
/// eprint!("...", &tmp_1, &tmp_2, /* some other arguments */);
375-
/// (tmp_1, tmp_2)
376-
/// }
377-
/// }
378-
/// ```
379-
///
380-
/// This is necessary so that `dbg!` outputs don't get torn, see #136703.
381-
#[doc(hidden)]
382-
#[rustc_macro_transparency = "semiopaque"]
383-
pub macro dbg_internal {
384-
(($($piece:literal),+) ($($processed:expr => $bound:ident),+) ()) => {
363+
($val:expr $(,)?) => {
385364
// Use of `match` here is intentional because it affects the lifetimes
386365
// of temporaries - https://stackoverflow.com/a/48732525/1063961
387-
// Always put the arguments in a tuple to avoid an unused parens lint on the pattern.
388-
match ($($processed,)+) {
389-
// Move the entire tuple so it doesn't stick around as a temporary (#154988).
390-
args => {
391-
let ($($bound,)+) = args;
392-
$crate::eprint!(
393-
$crate::concat!($($piece),+),
394-
$(
395-
$crate::stringify!($processed),
396-
// The `&T: Debug` check happens here (not in the format literal desugaring)
397-
// to avoid format literal related messages and suggestions.
398-
&&$bound as &dyn $crate::fmt::Debug
399-
),+,
400-
// The location returned here is that of the macro invocation, so
401-
// it will be the same for all expressions. Thus, label these
402-
// arguments so that they can be reused in every piece of the
403-
// formatting template.
404-
file=$crate::file!(),
405-
line=$crate::line!(),
406-
column=$crate::column!()
366+
match $val {
367+
tmp => {
368+
$crate::eprintln!("[{}:{}:{}] {} = {:#?}",
369+
$crate::file!(),
370+
$crate::line!(),
371+
$crate::column!(),
372+
$crate::stringify!($val),
373+
// The `&T: Debug` check happens here (not in the format literal desugaring)
374+
// to avoid format literal related messages and suggestions.
375+
&&tmp as &dyn $crate::fmt::Debug,
407376
);
408-
// Comma separate the variables only when necessary so that this will
409-
// not yield a tuple for a single expression, but rather just parenthesize
410-
// the expression.
411-
($($bound),+)
412-
377+
tmp
413378
}
414379
}
415-
},
416-
(($($piece:literal),*) ($($processed:expr => $bound:ident),*) ($val:expr $(,$rest:expr)*)) => {
417-
$crate::macros::dbg_internal!(
418-
($($piece,)* "[{file}:{line}:{column}] {} = {:#?}\n")
419-
($($processed => $bound,)* $val => tmp)
420-
($($rest),*)
421-
)
422-
},
380+
};
381+
($($val:expr),+ $(,)?) => {
382+
($($crate::dbg!($val)),+,)
383+
};
423384
}
424385

425386
#[doc(hidden)]

library/std/src/macros/tests.rs

Lines changed: 30 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -35,3 +35,33 @@ fn no_leaking_internal_temps_from_dbg() {
3535
// is dropped, then `foo`, then any temporaries left over from `dbg!` are dropped, if present.
3636
(drop(dbg!(bar)), drop(foo));
3737
}
38+
39+
/// Test for <https://github.com/rust-lang/rust/issues/155902>:
40+
/// `dbg!` shouldn't create a temporary that borrowck thinks can live past its invocation on a false
41+
/// unwind path.
42+
#[test]
43+
fn no_leaking_internal_temps_from_dbg_even_on_false_unwind() {
44+
#[derive(Debug)]
45+
struct Foo;
46+
impl Drop for Foo {
47+
fn drop(&mut self) {}
48+
}
49+
50+
#[derive(Debug)]
51+
struct Bar<'a>(#[allow(unused)] &'a Foo);
52+
impl Drop for Bar<'_> {
53+
fn drop(&mut self) {}
54+
}
55+
56+
{
57+
let foo = Foo;
58+
let bar = Bar(&foo);
59+
// The temporaries of this `super let`'s scrutinee will outlive `bar` and `foo`, emulating
60+
// the drop order of block tail expressions before Rust 2024. If borrowck thinks that a
61+
// panic from moving `bar` is possible and that a `Bar<'_>`-containing temporary lives past
62+
// the end of the block because of that, this will fail to compile. Because `Foo` implements
63+
// `Drop`, it's an error for `foo` to be dropped before such a temporary when unwinding;
64+
// otherwise, `foo` would just live to the end of the stack frame when unwinding.
65+
super let _ = drop(dbg!(bar));
66+
}
67+
}

src/tools/clippy/clippy_lints/src/dbg_macro.rs

Lines changed: 25 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -92,26 +92,33 @@ impl LateLintPass<'_> for DbgMacro {
9292
(macro_call.span, String::from("()"))
9393
}
9494
},
95-
ExprKind::Match(args, _, _) => {
96-
let suggestion = match args.kind {
97-
// dbg!(1) => 1
98-
ExprKind::Tup([val]) => {
99-
snippet_with_applicability(cx, val.span.source_callsite(), "..", &mut applicability)
100-
.to_string()
95+
// dbg!(1)
96+
ExprKind::Match(val, ..) => (
97+
macro_call.span,
98+
snippet_with_applicability(cx, val.span.source_callsite(), "..", &mut applicability)
99+
.to_string(),
100+
),
101+
// dbg!(2, 3)
102+
ExprKind::Tup(
103+
[
104+
Expr {
105+
kind: ExprKind::Match(first, ..),
106+
..
101107
},
102-
// dbg!(2, 3) => (2, 3)
103-
ExprKind::Tup([first, .., last]) => {
104-
let snippet = snippet_with_applicability(
105-
cx,
106-
first.span.source_callsite().to(last.span.source_callsite()),
107-
"..",
108-
&mut applicability,
109-
);
110-
format!("({snippet})")
108+
..,
109+
Expr {
110+
kind: ExprKind::Match(last, ..),
111+
..
111112
},
112-
_ => unreachable!(),
113-
};
114-
(macro_call.span, suggestion)
113+
],
114+
) => {
115+
let snippet = snippet_with_applicability(
116+
cx,
117+
first.span.source_callsite().to(last.span.source_callsite()),
118+
"..",
119+
&mut applicability,
120+
);
121+
(macro_call.span, format!("({snippet})"))
115122
},
116123
_ => unreachable!(),
117124
};

src/tools/clippy/clippy_utils/src/sym.rs

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -201,6 +201,7 @@ generate! {
201201
cx,
202202
cycle,
203203
cyclomatic_complexity,
204+
dbg_macro,
204205
de,
205206
debug_struct,
206207
deprecated_in_future,

src/tools/miri/tests/fail/dangling_pointers/dangling_primitive.stderr

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,7 @@ error: Undefined Behavior: memory access failed: ALLOC has been freed, so this p
22
--> tests/fail/dangling_pointers/dangling_primitive.rs:LL:CC
33
|
44
LL | dbg!(*ptr);
5-
| ^^^^ Undefined Behavior occurred here
5+
| ^^^^^^^^^^ Undefined Behavior occurred here
66
|
77
= help: this indicates a bug in the program: it performed an invalid operation, and caused Undefined Behavior
88
= help: see https://doc.rust-lang.org/nightly/reference/behavior-considered-undefined.html for further information

src/tools/miri/tests/fail/function_calls/return_pointer_on_unwind.stderr

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -7,7 +7,7 @@ error: Undefined Behavior: reading memory at ALLOC[0x0..0x4], but memory is unin
77
--> tests/fail/function_calls/return_pointer_on_unwind.rs:LL:CC
88
|
99
LL | dbg!(x.0);
10-
| ^^^ Undefined Behavior occurred here
10+
| ^^^^^^^^^ Undefined Behavior occurred here
1111
|
1212
= help: this indicates a bug in the program: it performed an invalid operation, and caused Undefined Behavior
1313
= help: see https://doc.rust-lang.org/nightly/reference/behavior-considered-undefined.html for further information

tests/ui/borrowck/dbg-issue-120327.rs

Lines changed: 38 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -1,44 +1,67 @@
1-
//! Diagnostic test for <https://github.com/rust-lang/rust/issues/120327>: suggest borrowing
2-
//! variables passed to `dbg!` that are later used.
3-
//@ dont-require-annotations: HELP
4-
51
fn s() -> String {
62
let a = String::new();
7-
dbg!(a); //~ HELP consider borrowing instead of transferring ownership
3+
dbg!(a);
84
return a; //~ ERROR use of moved value:
95
}
106

117
fn m() -> String {
128
let a = String::new();
13-
dbg!(1, 2, a, 1, 2); //~ HELP consider borrowing instead of transferring ownership
9+
dbg!(1, 2, a, 1, 2);
1410
return a; //~ ERROR use of moved value:
1511
}
1612

1713
fn t(a: String) -> String {
1814
let b: String = "".to_string();
19-
dbg!(a, b); //~ HELP consider borrowing instead of transferring ownership
15+
dbg!(a, b);
2016
return b; //~ ERROR use of moved value:
2117
}
2218

2319
fn x(a: String) -> String {
2420
let b: String = "".to_string();
25-
dbg!(a, b); //~ HELP consider borrowing instead of transferring ownership
21+
dbg!(a, b);
2622
return a; //~ ERROR use of moved value:
2723
}
2824

29-
fn two_of_them(a: String) -> String {
30-
dbg!(a, a); //~ ERROR use of moved value
31-
//~| HELP consider borrowing instead of transferring ownership
32-
//~| HELP consider borrowing instead of transferring ownership
33-
return a; //~ ERROR use of moved value
25+
macro_rules! my_dbg {
26+
() => {
27+
eprintln!("[{}:{}:{}]", file!(), line!(), column!())
28+
};
29+
($val:expr $(,)?) => {
30+
match $val {
31+
tmp => {
32+
eprintln!("[{}:{}:{}] {} = {:#?}",
33+
file!(), line!(), column!(), stringify!($val), &tmp);
34+
tmp
35+
}
36+
}
37+
};
38+
($($val:expr),+ $(,)?) => {
39+
($(my_dbg!($val)),+,)
40+
};
41+
}
42+
43+
fn test_my_dbg() -> String {
44+
let b: String = "".to_string();
45+
my_dbg!(b, 1);
46+
return b; //~ ERROR use of moved value:
47+
}
48+
49+
fn test_not_macro() -> String {
50+
let a = String::new();
51+
let _b = match a {
52+
tmp => {
53+
eprintln!("dbg: {}", tmp);
54+
tmp
55+
}
56+
};
57+
return a; //~ ERROR use of moved value:
3458
}
3559

3660
fn get_expr(_s: String) {}
3761

38-
// The suggestion is purely syntactic; applying it here will result in a type error.
3962
fn test() {
4063
let a: String = "".to_string();
41-
let _res = get_expr(dbg!(a)); //~ HELP consider borrowing instead of transferring ownership
64+
let _res = get_expr(dbg!(a));
4265
let _l = a.len(); //~ ERROR borrow of moved value
4366
}
4467

0 commit comments

Comments
 (0)