Skip to content

Commit 4f50466

Browse files
diannecuviper
authored andcommitted
don't drop arguments' temporaries in dbg!
(cherry picked from commit 51816ef)
1 parent 73caca4 commit 4f50466

File tree

9 files changed

+103
-111
lines changed

9 files changed

+103
-111
lines changed

‎library/std/src/macros.rs‎

Lines changed: 43 additions & 39 deletions
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,9 @@
55
//! library.
66
// ignore-tidy-dbg
77

8+
#[cfg(test)]
9+
mod tests;
10+
811
#[doc = include_str!("../../core/src/macros/panic.md")]
912
#[macro_export]
1013
#[rustc_builtin_macro(std_panic)]
@@ -359,19 +362,16 @@ macro_rules! dbg {
359362
};
360363
}
361364

362-
/// Internal macro that processes a list of expressions and produces a chain of
363-
/// nested `match`es, one for each expression, before finally calling `eprint!`
364-
/// with the collected information and returning all the evaluated expressions
365-
/// in a tuple.
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.
366368
///
367369
/// E.g. `dbg_internal!(() () (1, 2))` expands into
368370
/// ```rust, ignore
369-
/// match 1 {
370-
/// tmp_1 => match 2 {
371-
/// tmp_2 => {
372-
/// eprint!("...", &tmp_1, &tmp_2, /* some other arguments */);
373-
/// (tmp_1, tmp_2)
374-
/// }
371+
/// match (1, 2) {
372+
/// (tmp_1, tmp_2) => {
373+
/// eprint!("...", &tmp_1, &tmp_2, /* some other arguments */);
374+
/// (tmp_1, tmp_2)
375375
/// }
376376
/// }
377377
/// ```
@@ -380,37 +380,41 @@ macro_rules! dbg {
380380
#[doc(hidden)]
381381
#[rustc_macro_transparency = "semiopaque"]
382382
pub macro dbg_internal {
383-
(($($piece:literal),+) ($($processed:expr => $bound:expr),+) ()) => {{
384-
$crate::eprint!(
385-
$crate::concat!($($piece),+),
386-
$(
387-
$crate::stringify!($processed),
388-
// The `&T: Debug` check happens here (not in the format literal desugaring)
389-
// to avoid format literal related messages and suggestions.
390-
&&$bound as &dyn $crate::fmt::Debug
391-
),+,
392-
// The location returned here is that of the macro invocation, so
393-
// it will be the same for all expressions. Thus, label these
394-
// arguments so that they can be reused in every piece of the
395-
// formatting template.
396-
file=$crate::file!(),
397-
line=$crate::line!(),
398-
column=$crate::column!()
399-
);
400-
// Comma separate the variables only when necessary so that this will
401-
// not yield a tuple for a single expression, but rather just parenthesize
402-
// the expression.
403-
($($bound),+)
404-
}},
405-
(($($piece:literal),*) ($($processed:expr => $bound:expr),*) ($val:expr $(,$rest:expr)*)) => {
383+
(($($piece:literal),+) ($($processed:expr => $bound:ident),+) ()) => {
406384
// Use of `match` here is intentional because it affects the lifetimes
407385
// of temporaries - https://stackoverflow.com/a/48732525/1063961
408-
match $val {
409-
tmp => $crate::macros::dbg_internal!(
410-
($($piece,)* "[{file}:{line}:{column}] {} = {:#?}\n")
411-
($($processed => $bound,)* $val => tmp)
412-
($($rest),*)
413-
),
386+
// Always put the arguments in a tuple to avoid an unused parens lint on the pattern.
387+
match ($($processed,)+) {
388+
($($bound,)+) => {
389+
$crate::eprint!(
390+
$crate::concat!($($piece),+),
391+
$(
392+
$crate::stringify!($processed),
393+
// The `&T: Debug` check happens here (not in the format literal desugaring)
394+
// to avoid format literal related messages and suggestions.
395+
&&$bound as &dyn $crate::fmt::Debug
396+
),+,
397+
// The location returned here is that of the macro invocation, so
398+
// it will be the same for all expressions. Thus, label these
399+
// arguments so that they can be reused in every piece of the
400+
// formatting template.
401+
file=$crate::file!(),
402+
line=$crate::line!(),
403+
column=$crate::column!()
404+
);
405+
// Comma separate the variables only when necessary so that this will
406+
// not yield a tuple for a single expression, but rather just parenthesize
407+
// the expression.
408+
($($bound),+)
409+
410+
}
414411
}
415412
},
413+
(($($piece:literal),*) ($($processed:expr => $bound:ident),*) ($val:expr $(,$rest:expr)*)) => {
414+
$crate::macros::dbg_internal!(
415+
($($piece,)* "[{file}:{line}:{column}] {} = {:#?}\n")
416+
($($processed => $bound,)* $val => tmp)
417+
($($rest),*)
418+
)
419+
},
416420
}

‎library/std/src/macros/tests.rs‎

Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,13 @@
1+
// ignore-tidy-dbg
2+
3+
/// Test for <https://github.com/rust-lang/rust/issues/153850>:
4+
/// `dbg!` shouldn't drop arguments' temporaries.
5+
#[test]
6+
fn no_dropping_temps() {
7+
fn temp() {}
8+
9+
*dbg!(&temp());
10+
*dbg!(&temp(), 1).0;
11+
*dbg!(0, &temp()).1;
12+
*dbg!(0, &temp(), 2).1;
13+
}

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

Lines changed: 5 additions & 42 deletions
Original file line numberDiff line numberDiff line change
@@ -5,7 +5,7 @@ use clippy_utils::macros::{MacroCall, macro_backtrace};
55
use clippy_utils::source::snippet_with_applicability;
66
use rustc_data_structures::fx::FxHashSet;
77
use rustc_errors::Applicability;
8-
use rustc_hir::{Arm, Closure, ClosureKind, CoroutineKind, Expr, ExprKind, LetStmt, LocalSource, Node, Stmt, StmtKind};
8+
use rustc_hir::{Closure, ClosureKind, CoroutineKind, Expr, ExprKind, LetStmt, LocalSource, Node, Stmt, StmtKind};
99
use rustc_lint::{LateContext, LateLintPass, LintContext};
1010
use rustc_session::impl_lint_pass;
1111
use rustc_span::{Span, SyntaxContext};
@@ -92,16 +92,15 @@ impl LateLintPass<'_> for DbgMacro {
9292
(macro_call.span, String::from("()"))
9393
}
9494
},
95-
ExprKind::Match(first, arms, _) => {
96-
let vals = collect_vals(first, arms);
97-
let suggestion = match *vals.as_slice() {
95+
ExprKind::Match(args, _, _) => {
96+
let suggestion = match args.kind {
9897
// dbg!(1) => 1
99-
[val] => {
98+
ExprKind::Tup([val]) => {
10099
snippet_with_applicability(cx, val.span.source_callsite(), "..", &mut applicability)
101100
.to_string()
102101
},
103102
// dbg!(2, 3) => (2, 3)
104-
[first, .., last] => {
103+
ExprKind::Tup([first, .., last]) => {
105104
let snippet = snippet_with_applicability(
106105
cx,
107106
first.span.source_callsite().to(last.span.source_callsite()),
@@ -165,39 +164,3 @@ fn is_async_move_desugar<'tcx>(expr: &'tcx Expr<'tcx>) -> Option<&'tcx Expr<'tcx
165164
fn first_dbg_macro_in_expansion(cx: &LateContext<'_>, span: Span) -> Option<MacroCall> {
166165
macro_backtrace(span).find(|mc| cx.tcx.is_diagnostic_item(sym::dbg_macro, mc.def_id))
167166
}
168-
169-
/// Extracts all value expressions from the `match`-tree generated by `dbg!`.
170-
///
171-
/// E.g. from
172-
/// ```rust, ignore
173-
/// match 1 {
174-
/// tmp_1 => match 2 {
175-
/// tmp_2 => {
176-
/// /* printing */
177-
/// (tmp_1, tmp_2)
178-
/// }
179-
/// }
180-
/// }
181-
/// ```
182-
/// this extracts `1` and `2`.
183-
fn collect_vals<'hir>(first: &'hir Expr<'hir>, mut arms: &'hir [Arm<'hir>]) -> Vec<&'hir Expr<'hir>> {
184-
let mut vals = vec![first];
185-
loop {
186-
let [arm] = arms else {
187-
unreachable!("dbg! macro expansion only has single-arm matches")
188-
};
189-
190-
match is_async_move_desugar(arm.body)
191-
.unwrap_or(arm.body)
192-
.peel_drop_temps()
193-
.kind
194-
{
195-
ExprKind::Block(..) => return vals,
196-
ExprKind::Match(val, a, _) => {
197-
vals.push(val);
198-
arms = a;
199-
},
200-
_ => unreachable!("dbg! macro expansion only results in block or match expressions"),
201-
}
202-
}
203-
}

‎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.stderr‎

Lines changed: 20 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -4,44 +4,44 @@ error[E0382]: use of moved value: `a`
44
LL | let a = String::new();
55
| - move occurs because `a` has type `String`, which does not implement the `Copy` trait
66
LL | dbg!(a);
7-
| ------- value moved here
7+
| - value moved here
88
LL | return a;
99
| ^ value used here after move
1010
|
11-
help: consider borrowing instead of transferring ownership
11+
help: consider cloning the value if the performance cost is acceptable
1212
|
13-
LL | dbg!(&a);
14-
| +
13+
LL | dbg!(a.clone());
14+
| ++++++++
1515

1616
error[E0382]: use of moved value: `a`
1717
--> $DIR/dbg-issue-120327.rs:10:12
1818
|
1919
LL | let a = String::new();
2020
| - move occurs because `a` has type `String`, which does not implement the `Copy` trait
2121
LL | dbg!(1, 2, a, 1, 2);
22-
| ------------------- value moved here
22+
| - value moved here
2323
LL | return a;
2424
| ^ value used here after move
2525
|
26-
help: consider borrowing instead of transferring ownership
26+
help: consider cloning the value if the performance cost is acceptable
2727
|
28-
LL | dbg!(1, 2, &a, 1, 2);
29-
| +
28+
LL | dbg!(1, 2, a.clone(), 1, 2);
29+
| ++++++++
3030

3131
error[E0382]: use of moved value: `b`
3232
--> $DIR/dbg-issue-120327.rs:16:12
3333
|
3434
LL | let b: String = "".to_string();
3535
| - move occurs because `b` has type `String`, which does not implement the `Copy` trait
3636
LL | dbg!(a, b);
37-
| ---------- value moved here
37+
| - value moved here
3838
LL | return b;
3939
| ^ value used here after move
4040
|
41-
help: consider borrowing instead of transferring ownership
41+
help: consider cloning the value if the performance cost is acceptable
4242
|
43-
LL | dbg!(a, &b);
44-
| +
43+
LL | dbg!(a, b.clone());
44+
| ++++++++
4545

4646
error[E0382]: use of moved value: `a`
4747
--> $DIR/dbg-issue-120327.rs:22:12
@@ -50,14 +50,14 @@ LL | fn x(a: String) -> String {
5050
| - move occurs because `a` has type `String`, which does not implement the `Copy` trait
5151
LL | let b: String = "".to_string();
5252
LL | dbg!(a, b);
53-
| ---------- value moved here
53+
| - value moved here
5454
LL | return a;
5555
| ^ value used here after move
5656
|
57-
help: consider borrowing instead of transferring ownership
57+
help: consider cloning the value if the performance cost is acceptable
5858
|
59-
LL | dbg!(&a, b);
60-
| +
59+
LL | dbg!(a.clone(), b);
60+
| ++++++++
6161

6262
error[E0382]: use of moved value: `b`
6363
--> $DIR/dbg-issue-120327.rs:46:12
@@ -103,14 +103,14 @@ error[E0382]: borrow of moved value: `a`
103103
LL | let a: String = "".to_string();
104104
| - move occurs because `a` has type `String`, which does not implement the `Copy` trait
105105
LL | let _res = get_expr(dbg!(a));
106-
| ------- value moved here
106+
| - value moved here
107107
LL | let _l = a.len();
108108
| ^ value borrowed here after move
109109
|
110-
help: consider borrowing instead of transferring ownership
110+
help: consider cloning the value if the performance cost is acceptable
111111
|
112-
LL | let _res = get_expr(dbg!(&a));
113-
| +
112+
LL | let _res = get_expr(dbg!(a.clone()));
113+
| ++++++++
114114

115115
error: aborting due to 7 previous errors
116116

‎tests/ui/liveness/liveness-upvars.rs‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -98,7 +98,7 @@ pub fn g<T: Default>(mut v: T) {
9898
}
9999

100100
pub fn h<T: Copy + Default + std::fmt::Debug>() {
101-
let mut z = T::default();
101+
let mut z = T::default(); //~ WARN unused variable: `z`
102102
let _ = move |b| {
103103
loop {
104104
if b {

‎tests/ui/liveness/liveness-upvars.stderr‎

Lines changed: 9 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -156,6 +156,14 @@ LL | z = T::default();
156156
|
157157
= help: maybe it is overwritten before being read?
158158

159+
warning: unused variable: `z`
160+
--> $DIR/liveness-upvars.rs:101:9
161+
|
162+
LL | let mut z = T::default();
163+
| ^^^^^ help: if this is intentional, prefix it with an underscore: `_z`
164+
|
165+
= note: `#[warn(unused_variables)]` implied by `#[warn(unused)]`
166+
159167
warning: value captured by `state` is never read
160168
--> $DIR/liveness-upvars.rs:131:9
161169
|
@@ -196,5 +204,5 @@ LL | s = yield ();
196204
|
197205
= help: maybe it is overwritten before being read?
198206

199-
warning: 24 warnings emitted
207+
warning: 25 warnings emitted
200208

‎tests/ui/rfcs/rfc-2361-dbg-macro/dbg-macro-move-semantics.stderr‎

Lines changed: 10 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -1,17 +1,21 @@
11
error[E0382]: use of moved value: `a`
2-
--> $DIR/dbg-macro-move-semantics.rs:9:13
2+
--> $DIR/dbg-macro-move-semantics.rs:9:18
33
|
44
LL | let a = NoCopy(0);
55
| - move occurs because `a` has type `NoCopy`, which does not implement the `Copy` trait
66
LL | let _ = dbg!(a);
7-
| ------- value moved here
7+
| - value moved here
88
LL | let _ = dbg!(a);
9-
| ^^^^^^^ value used here after move
9+
| ^ value used here after move
1010
|
11-
help: consider borrowing instead of transferring ownership
11+
note: if `NoCopy` implemented `Clone`, you could clone the value
12+
--> $DIR/dbg-macro-move-semantics.rs:4:1
1213
|
13-
LL | let _ = dbg!(&a);
14-
| +
14+
LL | struct NoCopy(usize);
15+
| ^^^^^^^^^^^^^ consider implementing `Clone` for this type
16+
...
17+
LL | let _ = dbg!(a);
18+
| - you could clone this value
1519

1620
error: aborting due to 1 previous error
1721

0 commit comments

Comments
 (0)