-
-
Notifications
You must be signed in to change notification settings - Fork 15k
avoid tearing dbg! prints, implemented using super let
#157576
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -350,6 +350,7 @@ macro_rules! eprintln { | |
| /// [`debug!`]: https://docs.rs/log/*/log/macro.debug.html | ||
| /// [`log`]: https://crates.io/crates/log | ||
| #[macro_export] | ||
| #[allow_internal_unstable(std_internals)] | ||
| #[cfg_attr(not(test), rustc_diagnostic_item = "dbg_macro")] | ||
| #[stable(feature = "dbg_macro", since = "1.32.0")] | ||
| macro_rules! dbg { | ||
|
|
@@ -360,29 +361,69 @@ macro_rules! dbg { | |
| () => { | ||
| $crate::eprintln!("[{}:{}:{}]", $crate::file!(), $crate::line!(), $crate::column!()) | ||
| }; | ||
| ($val:expr $(,)?) => { | ||
| // Use of `match` here is intentional because it affects the lifetimes | ||
| // of temporaries - https://stackoverflow.com/a/48732525/1063961 | ||
| match $val { | ||
| tmp => { | ||
| $crate::eprintln!("[{}:{}:{}] {} = {:#?}", | ||
| $crate::file!(), | ||
| $crate::line!(), | ||
| $crate::column!(), | ||
| $crate::stringify!($val), | ||
| // The `&T: Debug` check happens here (not in the format literal desugaring) | ||
| // to avoid format literal related messages and suggestions. | ||
| &&tmp as &dyn $crate::fmt::Debug, | ||
| ); | ||
| tmp | ||
| } | ||
| } | ||
| }; | ||
| ($($val:expr),+ $(,)?) => { | ||
| ($($crate::dbg!($val)),+,) | ||
| $crate::macros::dbg_internal!(() () ($($val),+)) | ||
| }; | ||
| } | ||
|
|
||
| /// Internal macro that processes a list of expressions, binds their results, calls `eprint!` with | ||
| /// the collected information, and returns all the evaluated expressions in a tuple. | ||
| /// | ||
| /// E.g. `dbg_internal!(() () (1, 2))` expands into | ||
| /// ```rust, ignore | ||
| /// { | ||
| /// let tmp_1; | ||
| /// let tmp_2; | ||
| /// super let _ = (tmp_1 = 1); | ||
| /// super let _ = (tmp_2 = 2); | ||
| /// eprint!("...", &tmp_1, &tmp_2, /* some other arguments */); | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I would replace |
||
| /// (tmp_1, tmp_2) | ||
| /// } | ||
| /// ``` | ||
| /// | ||
| /// This is necessary so that `dbg!` outputs don't get torn, see #136703. | ||
| /// `super let` is used to avoid creating a temporary scope around `dbg!`'s arguments. Nested | ||
| /// `match` is insufficient because match arms introduce temporary scopes (#153850) and using a | ||
| /// single match on a tuple containing all the arguments is insufficient because of an imprecision | ||
| /// in the borrow-checker (see #155902). | ||
| #[doc(hidden)] | ||
| #[allow_internal_unstable(std_internals, super_let)] | ||
| #[rustc_macro_transparency = "semiopaque"] | ||
| #[unstable(feature = "std_internals", issue = "none")] | ||
| pub macro dbg_internal { | ||
| (($($piece:literal),+) ($($processed:expr => $bound:ident),+) ()) => {{ | ||
| $(let $bound);+; | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Minor nit, but I personally would prefer calling this just |
||
| $(super let _ = ($bound = $processed));+; | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I think that this syntax is very confusing and should probably be explained a bit better. To me, I see this as the unit return value of the compound expression |
||
| $crate::eprint!( | ||
| $crate::concat!($($piece),+), | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Similarly, I think Again, this is clear from the implementation alone, but it would be nice to have a grasp on what is being done before reading the full implementation. |
||
| $( | ||
| $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 | ||
| ),+, | ||
| // The location returned here is that of the macro invocation, so | ||
| // it will be the same for all expressions. Thus, label these | ||
| // arguments so that they can be reused in every piece of the | ||
| // formatting template. | ||
| file=$crate::file!(), | ||
| line=$crate::line!(), | ||
| column=$crate::column!() | ||
| ); | ||
| // Comma separate the variables only when necessary so that this will | ||
| // not yield a tuple for a single expression, but rather just parenthesize | ||
| // the expression. | ||
| ($($bound),+) | ||
| }}, | ||
| (($($piece:literal),*) ($($processed:expr => $bound:ident),*) ($val:expr $(,$rest:expr)*)) => { | ||
| $crate::macros::dbg_internal!( | ||
| ($($piece,)* "[{file}:{line}:{column}] {} = {:#?}\n") | ||
| ($($processed => $bound,)* $val => tmp) | ||
| ($($rest),*) | ||
| ) | ||
| }, | ||
| } | ||
|
|
||
| #[doc(hidden)] | ||
| #[macro_export] | ||
| #[allow_internal_unstable(hash_map_internals)] | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -202,7 +202,6 @@ generate! { | |
| cx, | ||
| cycle, | ||
| cyclomatic_complexity, | ||
| dbg_macro, | ||
| de, | ||
| debug_struct, | ||
| deprecated_in_future, | ||
|
|
||
Uh oh!
There was an error while loading. Please reload this page.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I would clarify this further to the sequence:
dbg_internal!(() () (1, 2))dbg_internal!(("...") (1 => tmp_1) (2))dbg_internal!(("...", "...") (1 => tmp_1, 2 => tmp_2) ()Followed by the code generated.
I would also note that the
_1and_2suffixes are not actually used, and that we rely on macro hygiene to make multipletmpidents distinct. Again, technically something you should be able to figure out from the implementation, but it speeds up reading it a bit more.View changes since the review