-
-
Notifications
You must be signed in to change notification settings - Fork 14.3k
std: avoid tearing dbg! prints
#149869
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?
std: avoid tearing dbg! prints
#149869
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 |
|---|---|---|
|
|
@@ -5,7 +5,7 @@ use clippy_utils::macros::{MacroCall, macro_backtrace}; | |
| use clippy_utils::source::snippet_with_applicability; | ||
| use rustc_data_structures::fx::FxHashSet; | ||
| use rustc_errors::Applicability; | ||
| use rustc_hir::{Closure, ClosureKind, CoroutineKind, Expr, ExprKind, LetStmt, LocalSource, Node, Stmt, StmtKind}; | ||
| use rustc_hir::{Arm, Closure, ClosureKind, CoroutineKind, Expr, ExprKind, LetStmt, LocalSource, Node, Stmt, StmtKind}; | ||
| use rustc_lint::{LateContext, LateLintPass, LintContext}; | ||
| use rustc_session::impl_lint_pass; | ||
| use rustc_span::{Span, SyntaxContext, sym}; | ||
|
|
@@ -90,33 +90,27 @@ impl LateLintPass<'_> for DbgMacro { | |
| (macro_call.span, String::from("()")) | ||
| } | ||
| }, | ||
| // dbg!(1) | ||
| ExprKind::Match(val, ..) => ( | ||
| macro_call.span, | ||
| snippet_with_applicability(cx, val.span.source_callsite(), "..", &mut applicability) | ||
| .to_string(), | ||
| ), | ||
| // dbg!(2, 3) | ||
| ExprKind::Tup( | ||
| [ | ||
| Expr { | ||
| kind: ExprKind::Match(first, ..), | ||
| .. | ||
| }, | ||
| .., | ||
| Expr { | ||
| kind: ExprKind::Match(last, ..), | ||
| .. | ||
| }, | ||
| ], | ||
| ) => { | ||
| let snippet = snippet_with_applicability( | ||
| cx, | ||
| first.span.source_callsite().to(last.span.source_callsite()), | ||
| "..", | ||
| &mut applicability, | ||
| ); | ||
| (macro_call.span, format!("({snippet})")) | ||
| ExprKind::Match(first, arms, _) => { | ||
| let vals = collect_vals(first, arms); | ||
| let suggestion = match vals.as_slice() { | ||
| // dbg!(1) => 1 | ||
| &[val] => { | ||
| snippet_with_applicability(cx, val.span.source_callsite(), "..", &mut applicability) | ||
| .to_string() | ||
| } | ||
| // dbg!(2, 3) => (2, 3) | ||
| &[first, .., last] => { | ||
| let snippet = snippet_with_applicability( | ||
| cx, | ||
| first.span.source_callsite().to(last.span.source_callsite()), | ||
| "..", | ||
| &mut applicability, | ||
| ); | ||
| format!("({snippet})") | ||
| } | ||
| _ => unreachable!(), | ||
| }; | ||
| (macro_call.span, suggestion) | ||
| }, | ||
| _ => unreachable!(), | ||
| }; | ||
|
|
@@ -169,3 +163,33 @@ fn is_async_move_desugar<'tcx>(expr: &'tcx Expr<'tcx>) -> Option<&'tcx Expr<'tcx | |
| fn first_dbg_macro_in_expansion(cx: &LateContext<'_>, span: Span) -> Option<MacroCall> { | ||
| macro_backtrace(span).find(|mc| cx.tcx.is_diagnostic_item(sym::dbg_macro, mc.def_id)) | ||
| } | ||
|
|
||
| /// Extracts all value expressions from the `match`-tree generated by `dbg!`. | ||
| /// | ||
| /// E.g. from | ||
| /// ```rust, ignore | ||
| /// match 1 { | ||
| /// tmp_1 => match 2 { | ||
| /// tmp_2 => { | ||
| /// /* printing */ | ||
| /// (tmp_1, tmp_2) | ||
| /// } | ||
| /// } | ||
| /// } | ||
| /// ``` | ||
| /// this extracts `1` and `2`. | ||
| fn collect_vals<'hir>(first: &'hir Expr<'hir>, mut arms: &'hir [Arm<'hir>]) -> Vec<&'hir Expr<'hir>> { | ||
| let mut vals = vec![first]; | ||
| 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 { | ||
|
Member
Author
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'm not sure whether this is really necessary, I just copied this from above.
Member
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 don't think this is needed here. You can probably use takes_async_fn(async move |val| { dbg!(val, val + 1); val });
//~^ dbg_macro |
||
| ExprKind::Block(..) => return vals, | ||
| ExprKind::Match(val, a, _) => { | ||
| vals.push(val); | ||
| arms = a; | ||
| } | ||
| _ => unreachable!("dbg! macro expansion only results in block or match expressions"), | ||
| } | ||
| } | ||
| } | ||
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.
How does this actually help avoid tearing?
So we should expect this to lower to a series of writes to stderr, i.e., it will in fact tear if there's concurrency. We'd need to add a buffer somewhere to avoid that.
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.
Ah, I see, we're taking a lock in fmt::Write for Stderr before calling write_fmt.
Should we avoid the matches and just make dbg! expand to
write!(std::io::stderr().lock(), ...)then? That seems simpler and achieves the same goal, right? I guess it'd need to inline the capturing (print_to_buffer_if_capture_used)...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.
@Mark-Simulacrum What would you propose would go in that
...? That's precisely what the recursivematches generate.