Skip to content

Conversation

@joboet
Copy link
Member

@joboet joboet commented Dec 10, 2025

Fixes #136703.

This is an alternative to #149859. Instead of formatting everything into a string, this PR makes multi-expression dbg! expand into multiple nested matches, with the final match containing a single eprint!. By using macro recursion and relying on hygiene, this allows naming every bound value in that eprint!.

CC @orlp

r? libs

@rustbot rustbot added S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. T-libs Relevant to the library team, which will review and decide on the PR/issue. labels Dec 10, 2025
@rust-log-analyzer

This comment has been minimized.

@orlp
Copy link
Contributor

orlp commented Dec 11, 2025

Neat solution :) I wasn't sure if we could use helper macros.

@rustbot rustbot added the T-clippy Relevant to the Clippy team. label Dec 12, 2025
@joboet joboet marked this pull request as ready for review December 12, 2025 12:25
@rustbot rustbot added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Dec 12, 2025
@rustbot
Copy link
Collaborator

rustbot commented Dec 12, 2025

Some changes occurred in src/tools/clippy

cc @rust-lang/clippy

@rustbot rustbot removed the S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. label Dec 12, 2025
loop {
let [arm] = arms else { unreachable!("dbg! macro expansion only has single-arm matches") };

match is_async_move_desugar(arm.body).unwrap_or(arm.body).peel_drop_temps().kind {
Copy link
Member 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 whether this is really necessary, I just copied this from above.

@rust-log-analyzer

This comment has been minimized.

@rust-log-analyzer

This comment has been minimized.

@orlp
Copy link
Contributor

orlp commented Dec 13, 2025

Is it allowed to change those error messages to leak those internal details?

@orlp
Copy link
Contributor

orlp commented Dec 13, 2025

An alternative is to stuff the helper macro into the main macro with some kind of marker that would otherwise never be legal, e.g. starting with the tokens => __internal_recursion:

macro_rules! foo {
    (=> __internal_recursion $x:expr) => {
        // ...
    };
    
    ($x:expr) => {
        foo!(=> __internal_recursion $x);
    };
}

@joboet
Copy link
Member Author

joboet commented Dec 13, 2025

Is it allowed to change those error messages to leak those internal details?

I don't think there's currently a way to tell the compiler to hide them unfortunately. But yes, this is fine, or at least there is precedent – the same happens for thread_local!: If you try

thread_local! {
    #[rustc_align_static(42)]
    static LOCAL: i32 = 42;
}

(playground) on stable, the error message will reference $crate::thread::local_impl::thread_local_inner.

An alternative is to stuff the helper macro into the main macro with some kind of marker that would otherwise never be legal, e.g. starting with the tokens => __internal_recursion:

macro_rules! foo {
    (=> __internal_recursion $x:expr) => {
        // ...
    };
    
    ($x:expr) => {
        foo!(=> __internal_recursion $x);
    };
}

It would be legal to write these tokens in macro invocations in user code, so this would be an even worse leak of implementation details. I'm sure there is a way to abuse #[allow_internal_unstable] to make some macro arms unusable from stable, but I don't think its worth the hassle.

@orlp
Copy link
Contributor

orlp commented Dec 13, 2025

It would be legal to write these tokens in macro invocations in user code, so this would be an even worse leak of implementation details.

It would also be legal to directly write std::macros::dbg_internal!, no? So I don't see how it's better/worse in that regard.

@rustbot
Copy link
Collaborator

rustbot commented Dec 13, 2025

The Miri subtree was changed

cc @rust-lang/miri

@joboet
Copy link
Member Author

joboet commented Dec 13, 2025

It would be legal to write these tokens in macro invocations in user code, so this would be an even worse leak of implementation details.

It would also be legal to directly write std::macros::dbg_internal!, no? So I don't see how it's better/worse in that regard.

No, since the macro is perma-unstable 😏

@orlp
Copy link
Contributor

orlp commented Dec 13, 2025

How does that work once the macro expands? Does it not expand in the user's context? Or is this just stdlib magic sprinkles?

@joboet
Copy link
Member Author

joboet commented Dec 13, 2025

How does that work once the macro expands? Does it not expand in the user's context? Or is this just stdlib magic sprinkles?

dbg! is marked with #[allow_internal_unstable(std_internals)] which allows dbg_internal! to be used from code generated by the macro (but not in spans passed to the macro) despite it being an unstable feature. We use the same trick for thread_local!, since it too needs to invoke unstable helper macros, which need to use unstable features like #[thread_local] themselves.

Edit: So yes, essentially just stdlib magic sprinkles 😄

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

Labels

S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. T-clippy Relevant to the Clippy team. 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.

dbg! prints can tear in multi-threading code

5 participants