Skip to content

don't drop arguments' temporaries in dbg!#154074

Open
dianne wants to merge 1 commit intorust-lang:mainfrom
dianne:dbg-temp-scopes
Open

don't drop arguments' temporaries in dbg!#154074
dianne wants to merge 1 commit intorust-lang:mainfrom
dianne:dbg-temp-scopes

Conversation

@dianne
Copy link
Contributor

@dianne dianne commented Mar 19, 2026

Fixes #153850

Credit to @theemathas for help with macro engineering ^^

r? libs

@rustbot
Copy link
Collaborator

rustbot commented Mar 19, 2026

Some changes occurred in src/tools/clippy

cc @rust-lang/clippy

@rustbot rustbot added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. T-clippy Relevant to the Clippy team. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. T-libs Relevant to the library team, which will review and decide on the PR/issue. labels Mar 19, 2026
@rustbot rustbot added the beta-nominated Nominated for backporting to the compiler in the beta channel. label Mar 19, 2026
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Comment on lines +9 to +12
*dbg!(&temp());
*dbg!(&temp(), 1).0;
*dbg!(0, &temp()).1;
*dbg!(0, &temp(), 2).1;
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

$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
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

Copy link
Contributor

@theemathas theemathas Mar 19, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

@rust-log-analyzer

This comment has been minimized.

@rustbot
Copy link
Collaborator

rustbot commented Mar 19, 2026

The Miri subtree was changed

cc @rust-lang/miri

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

beta-nominated Nominated for backporting to the compiler in the beta channel. S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. T-clippy Relevant to the Clippy team. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. T-libs Relevant to the library team, which will review and decide on the PR/issue.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1.95 beta regression: "temporary value dropped while borrowed"

5 participants