don't drop arguments' temporaries in `dbg!` by dianne · Pull Request #154074 · rust-lang/rust

dianne · 2026-03-19T03:55:12Z

View all comments

Fixes #153850

Credit to @theemathas for help with macro engineering ^^

r? libs

rustbot · 2026-03-19T03:55:16Z

Some changes occurred in src/tools/clippy

cc @rust-lang/clippy

dianne · 2026-03-19T03:55:43Z

library/std/src/macros/tests.rs

I'm not totally sure where this should go. Conceptually, it makes the most sense for it to be a std test, but I can't find where dbg! is actually tested. It also only needs to pass borrowck, not be built and executed, so we could maybe save CI time by making it a //@ check-pass ui test or such.

I think this is OK. It probably doesn't need to be an integration test though? i.e., you could add it to macros/test.rs or so?

Oh, I wasn't aware there was a difference! Done.

Unit tests get compiled as part of the general crate whereas integ tests (in tests/ at the top level) each get compiled separately, so they're generally slower to compile (on a per-test amortized basis).

dianne · 2026-03-19T03:56:01Z

library/std/src/macros/tests.rs

+    *dbg!(&temp());
+    *dbg!(&temp(), 1).0;
+    *dbg!(0, &temp()).1;
+    *dbg!(0, &temp(), 2).1;


Only the last couple of these actually regressed, but I figure we may as well be resilient to changes in dbg!.

Also, the dereferences of the temporary aren't necessary to reproduce this and weren't present in the reproducer found by crater, but I feel like being explicit makes the intent of the test clearer.

theemathas · 2026-03-19T04:02:59Z

library/std/src/macros.rs

+                        $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


Why do we need a double reference here? Is it to support some crazy code doing impl Debug for &Thing ?

I'm not sure! The double reference just coerces away as part of the cast, right? But it's been that way since #142594, so I figured I'd leave it be.

.. though on second thought, just removing the & would be a very small PR on its own, so maybe it could be bundled in here, especially if the decision is to backport a revert of #149869 instead of this.

Huh. This code has compiled ever since the stabilization of dbg!() in 1.32.0.

use std::fmt::{self, Debug, Formatter}; struct Thing; impl Debug for &Thing { fn fmt(&self, _: &mut Formatter) -> fmt::Result { Ok(()) } } fn main() { dbg!(Thing); }

Removing the double reference would break this code.

That feels probably accidental ^^ but it definitely shouldn't be changed as part of this, then.

The dbg! macro has always done the equivalent of println!("{:?}", &x). The double reference is necessary to keep those semantics.

You can do something like

#![feature(impl_trait_in_bindings)] fn main() { let x = 4_u8; let y: impl std::fmt::Debug = &x; }

if you really want to get rid of the double reference/cast, though.

rustbot · 2026-03-19T04:48:18Z

The Miri subtree was changed

cc @rust-lang/miri

Mark-Simulacrum

Generally this looks fine to me. I'm also not sure where (or whether they exist) there's more tests, but I think as long as you confirmed the one you added fails CI if the changes are reverted it feels OK to me.

r=me, I think it would probably make sense to revert #149869 as the beta backport but I'll leave that up to T-libs backport decision making.

View changes since this review

Mark-Simulacrum · 2026-03-27T20:44:15Z

library/std/src/macros/tests.rs

I think this is OK. It probably doesn't need to be an integration test though? i.e., you could add it to macros/test.rs or so?

Mark-Simulacrum · 2026-03-27T20:45:16Z

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

   |            ^ value used here after move
   |
-help: consider borrowing instead of transferring ownership
+help: consider cloning the value if the performance cost is acceptable


This feels like an unfortunate regression (fine for this PR but maybe file an issue for it)? It seems highly likely that borrowing is better here.

Apparently that suggestion was actually specifically meant for dbg!; I'd assumed it was generic. Maybe it's worth handling it in this PR to keep its expectation of what dbg! looks like in sync with reality—same as the clippy lint change? That test is specifically for the diagnostic too; if I'd taken care to read the linked issue (or if the test was documented or tested for the suggestion message) I'd have updated the diagnostic from the start ^^

I've added a fix, but I can move it to its own PR if that'd be better.

dianne · 2026-03-29T16:19:52Z

compiler/rustc_borrowck/src/diagnostics/conflict_errors.rs

This already only worked for expansions, so I'd like to keep its scope narrow for now. If a more general "borrow here" suggestion would be helpful, I think it should be designed intentionally.

dianne · 2026-03-29T16:20:06Z

compiler/rustc_borrowck/src/diagnostics/conflict_errors.rs

Here's why it only worked for expansions: self.expr_span was the move_span, which previously pointed to the match's pattern (specifically a by-move binding), which is normally separate from the match scrutinee (where dbg!'s args expand).

dianne · 2026-03-29T16:21:20Z

compiler/rustc_borrowck/src/diagnostics/conflict_errors.rs

+                            path @ Path { segments: [seg], .. },
+                        )) = &arg.kind
+                            && seg.ident.name == self.arg_name
+                            && self.move_span.source_equal(arg.span)


This incidentally fixes a minor bug: if given dbg!(a, a); a;, it would produce two suggestions to borrow the second argument rather than one for each. Now that the move_span points to the user-provided argument, we can check the scrutinee for it to find the correct argument for each suggestion.

dianne · 2026-03-29T16:21:32Z

compiler/rustc_borrowck/src/diagnostics/conflict_errors.rs

+                    && 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)
+                        })


This could avoid iterating by making dbg_internal! a diagnostic item. I figured it'd be cleaner to reuse the dbg_macro diagnostic item instead since efficiency isn't a priority, but I'm not sure.

I think as-is this will also apply to e.g. dbg!(some_other_macro!(a)), right? Not sure that's desirable. But I think it's probably fairly rare that someone does that, so we can go with this for now.

I don't think so? I could be wrong, but I don't think the argument to dbg! would have the dbg! expansion in its macro backtrace, since it's not part of dbg!. I tested the spans of the moved-from arguments to dbg! in dbg-issue-120327.rs; they all have empty macro backtraces.

dianne · 2026-03-29T16:21:59Z

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

 }

 fn get_expr(_s: String) {}

+// The suggestion is purely syntactic; applying it here will result in a type error.


This was a pre-existing issue; I consider fixing it out-of-scope. I think other suggestions to borrow often produce incorrect code, but if we want to be extra-safe, it could only fire when the dbg! invocation is on a statement of its own.

dianne · 2026-03-29T16:28:06Z

Re-adding the T-compiler label since I've added in a diagnostic fix. I'll re-remove it if it'd be better as its own PR; I can add a FIXME to the diagnostic and its test in that case.

Also, it looks like my self-review notes didn't all end up rendering properly? Oops. Hopefully they still make sense.

Mark-Simulacrum

@bors r+

I think my sense is that especially with compiler changes also involved we should land the revert on beta rather than trying to backport this.

View changes since this review

rust-bors · 2026-03-29T16:29:04Z

📌 Commit be31b0a has been approved by Mark-Simulacrum

It is now in the queue for this repository.

dianne · 2026-03-29T16:47:34Z

Forgot to re-test clippy 😔 @bors r-
Easy fix, at least.

dianne · 2026-03-29T16:51:22Z

@bors r=Mark-Simulacrum

rust-bors · 2026-03-29T16:51:25Z

📌 Commit 7d1b41c has been approved by Mark-Simulacrum

It is now in the queue for this repository.

…uwer Rollup of 11 pull requests Successful merges: - #154074 (don't drop arguments' temporaries in `dbg!`) - #154328 (rustdoc: add missing {os,target,target_env} values for cfg pretty printer) - #154540 (Fix invalid type suggestion for item nested in function) - #154549 (Add regression test for recursive lazy type alias normalization ICE) - #153373 (Fix LegacyKeyValueFormat report from docker build: powerpc) - #154322 (feat: reimplement `hash_map!` macro) - #154416 (Add `IoSplit` diagnostic item for `std::io::Split`) - #154486 (std_detect on AArch64 Darwin: Detect FEAT_SVE_B16B16) - #154508 (Fix ambiguous parsing in bootstrap.py) - #154518 (Panic in Hermit clock_gettime) - #154530 (update zulip link in `std` documentation)

@theemathas

Rollup merge of #154074 - dianne:dbg-temp-scopes, r=Mark-Simulacrum don't drop arguments' temporaries in `dbg!` Fixes #153850 Credit to @theemathas for help with macro engineering ^^ r? libs

…uwer Rollup of 11 pull requests Successful merges: - rust-lang/rust#154074 (don't drop arguments' temporaries in `dbg!`) - rust-lang/rust#154328 (rustdoc: add missing {os,target,target_env} values for cfg pretty printer) - rust-lang/rust#154540 (Fix invalid type suggestion for item nested in function) - rust-lang/rust#154549 (Add regression test for recursive lazy type alias normalization ICE) - rust-lang/rust#153373 (Fix LegacyKeyValueFormat report from docker build: powerpc) - rust-lang/rust#154322 (feat: reimplement `hash_map!` macro) - rust-lang/rust#154416 (Add `IoSplit` diagnostic item for `std::io::Split`) - rust-lang/rust#154486 (std_detect on AArch64 Darwin: Detect FEAT_SVE_B16B16) - rust-lang/rust#154508 (Fix ambiguous parsing in bootstrap.py) - rust-lang/rust#154518 (Panic in Hermit clock_gettime) - rust-lang/rust#154530 (update zulip link in `std` documentation)

rustbot assigned Mark-Simulacrum Mar 19, 2026

rustbot added the beta-nominated Nominated for backporting to the compiler in the beta channel. label Mar 19, 2026

dianne commented Mar 19, 2026

View reviewed changes

theemathas reviewed Mar 19, 2026

View reviewed changes

This comment has been minimized.

Sign in to view

dianne force-pushed the dbg-temp-scopes branch from 21dff86 to aea73b5 Compare March 19, 2026 04:48

apiraino removed the T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. label Mar 26, 2026

Mark-Simulacrum approved these changes Mar 27, 2026

View reviewed changes

don't drop arguments' temporaries in dbg!

51816ef

dianne force-pushed the dbg-temp-scopes branch from aea73b5 to be31b0a Compare March 29, 2026 16:19

dianne commented Mar 29, 2026

View reviewed changes

dianne added the T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. label Mar 29, 2026

Mark-Simulacrum approved these changes Mar 29, 2026

View reviewed changes

rust-bors bot added S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Mar 29, 2026

This comment has been minimized.

Sign in to view

rust-bors bot added S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. and removed S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. labels Mar 29, 2026

update diagnostic for variables moved by dbg!

7d1b41c

dianne force-pushed the dbg-temp-scopes branch from be31b0a to 7d1b41c Compare March 29, 2026 16:50

rust-bors bot added S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. and removed S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. labels Mar 29, 2026

JonathanBrouwer mentioned this pull request Mar 29, 2026

Rollup of 11 pull requests #154559

Merged

rust-bors bot merged commit 2a18b88 into rust-lang:main Mar 30, 2026
11 checks passed

theemathas mentioned this pull request Mar 30, 2026

dbg!() only requires &T (not T) to implement Debug. #154589

Open

Uh oh!

Conversation

dianne commented Mar 19, 2026 • edited by rustbot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

rustbot commented Mar 19, 2026

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

theemathas Mar 19, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

This comment has been minimized.

rustbot commented Mar 19, 2026

Uh oh!

Mark-Simulacrum left a comment • edited by rustbot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

dianne commented Mar 29, 2026

Uh oh!

Mark-Simulacrum left a comment • edited by rustbot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

rust-bors bot commented Mar 29, 2026

Uh oh!

This comment has been minimized.

dianne commented Mar 29, 2026

Uh oh!

dianne commented Mar 29, 2026

Uh oh!

rust-bors bot commented Mar 29, 2026

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

dianne commented Mar 19, 2026 •

edited by rustbot

Loading

theemathas Mar 19, 2026 •

edited

Loading

Mark-Simulacrum left a comment •

edited by rustbot

Loading

Mark-Simulacrum left a comment •

edited by rustbot

Loading