From 3aed0b7ebb25ba110dd96bad57d33f3a041c0d4e Mon Sep 17 00:00:00 2001 From: MarcusGrass Date: Fri, 23 Feb 2024 16:27:47 +0100 Subject: [PATCH 01/23] Can run multithreaded --- src/bin/main.rs | 104 ++++++++++++++++++++++++++++++------------------ 1 file changed, 66 insertions(+), 38 deletions(-) diff --git a/src/bin/main.rs b/src/bin/main.rs index 88281d296be..408f8ae7e6c 100644 --- a/src/bin/main.rs +++ b/src/bin/main.rs @@ -8,7 +8,7 @@ use thiserror::Error; use rustfmt_nightly as rustfmt; use tracing_subscriber::EnvFilter; -use std::collections::HashMap; +use std::collections::{HashMap, VecDeque}; use std::env; use std::fs::File; use std::io::{self, stdout, Read, Write}; @@ -314,56 +314,84 @@ fn format( } } - let out = &mut stdout(); - let mut session = Session::new(config, Some(out)); + let num_cpus = 32; + + let exit_code = std::thread::scope(|scope| { + let mut exit_code = 0; + let mut handles = VecDeque::with_capacity(files.len()); + for file in files { + let cfg = config.clone(); + let handle = scope.spawn(|| { + let mut output = Vec::new(); + let mut session = Session::new(cfg, Some(&mut output)); + if !file.exists() { + eprintln!("Error: file `{}` does not exist", file.to_str().unwrap()); + session.add_operational_error(); + } else if file.is_dir() { + eprintln!("Error: `{}` is a directory", file.to_str().unwrap()); + session.add_operational_error(); + } else { + // Check the file directory if the config-path could not be read or not provided + if config_path.is_none() { + let (local_config, config_path) = + load_config(Some(file.parent().unwrap()), Some(options.clone()))?; + if local_config.verbose() == Verbosity::Verbose { + if let Some(path) = config_path { + println!( + "Using rustfmt config file {} for {}", + path.display(), + file.display() + ); + } + } - for file in files { - if !file.exists() { - eprintln!("Error: file `{}` does not exist", file.to_str().unwrap()); - session.add_operational_error(); - } else if file.is_dir() { - eprintln!("Error: `{}` is a directory", file.to_str().unwrap()); - session.add_operational_error(); - } else { - // Check the file directory if the config-path could not be read or not provided - if config_path.is_none() { - let (local_config, config_path) = - load_config(Some(file.parent().unwrap()), Some(options.clone()))?; - if local_config.verbose() == Verbosity::Verbose { - if let Some(path) = config_path { - println!( - "Using rustfmt config file {} for {}", - path.display(), - file.display() - ); + session.override_config(local_config, |sess| { + format_and_emit_report(sess, Input::File(file)) + }); + } else { + format_and_emit_report(&mut session, Input::File(file)); } } - - session.override_config(local_config, |sess| { - format_and_emit_report(sess, Input::File(file)) - }); - } else { - format_and_emit_report(&mut session, Input::File(file)); + let exit_code = if session.has_operational_errors() + || session.has_parsing_errors() + || ((session.has_diff() || session.has_check_errors()) && options.check) + { + 1 + } else { + 0 + }; + drop(session); + Ok::<_, std::io::Error>((output, exit_code)) + }); + handles.push_back(handle); + if handles.len() >= num_cpus { + let (output, exit) = handles.pop_front().unwrap().join().unwrap().unwrap(); + let out = &mut stdout(); + out.write_all(&output).unwrap(); + if exit != 0 { + exit_code = exit; + } } } - } + let out = &mut stdout(); + for handle in handles { + let (output, exit) = handle.join().unwrap().unwrap(); + out.write_all(&output).unwrap(); + if exit != 0 { + exit_code = exit; + } + } + exit_code + }); // If we were given a path via dump-minimal-config, output any options // that were used during formatting as TOML. if let Some(path) = minimal_config_path { let mut file = File::create(path)?; - let toml = session.config.used_options().to_toml()?; + let toml = config.used_options().to_toml()?; file.write_all(toml.as_bytes())?; } - let exit_code = if session.has_operational_errors() - || session.has_parsing_errors() - || ((session.has_diff() || session.has_check_errors()) && options.check) - { - 1 - } else { - 0 - }; Ok(exit_code) } From 9e28f789c4389a428e512dcee096c3b62e257ee0 Mon Sep 17 00:00:00 2001 From: MarcusGrass Date: Fri, 23 Feb 2024 17:28:15 +0100 Subject: [PATCH 02/23] Use a map to collect thread panics --- src/bin/main.rs | 146 +++++++++++++++++++++++++++++------------------- 1 file changed, 88 insertions(+), 58 deletions(-) diff --git a/src/bin/main.rs b/src/bin/main.rs index 408f8ae7e6c..ff049758ff6 100644 --- a/src/bin/main.rs +++ b/src/bin/main.rs @@ -8,12 +8,13 @@ use thiserror::Error; use rustfmt_nightly as rustfmt; use tracing_subscriber::EnvFilter; -use std::collections::{HashMap, VecDeque}; +use std::collections::HashMap; use std::env; use std::fs::File; use std::io::{self, stdout, Read, Write}; use std::path::{Path, PathBuf}; use std::str::FromStr; +use std::thread::JoinHandle; use getopts::{Matches, Options}; @@ -307,6 +308,7 @@ fn format( ) -> Result { options.verify_file_lines(&files); let (config, config_path) = load_config(None, Some(options.clone()))?; + let cfg_path_is_none = config_path.is_none(); if config.verbose() == Verbosity::Verbose { if let Some(path) = config_path.as_ref() { @@ -314,75 +316,103 @@ fn format( } } - let num_cpus = 32; - - let exit_code = std::thread::scope(|scope| { - let mut exit_code = 0; - let mut handles = VecDeque::with_capacity(files.len()); - for file in files { - let cfg = config.clone(); - let handle = scope.spawn(|| { - let mut output = Vec::new(); - let mut session = Session::new(cfg, Some(&mut output)); - if !file.exists() { - eprintln!("Error: file `{}` does not exist", file.to_str().unwrap()); - session.add_operational_error(); - } else if file.is_dir() { - eprintln!("Error: `{}` is a directory", file.to_str().unwrap()); - session.add_operational_error(); - } else { - // Check the file directory if the config-path could not be read or not provided - if config_path.is_none() { - let (local_config, config_path) = - load_config(Some(file.parent().unwrap()), Some(options.clone()))?; - if local_config.verbose() == Verbosity::Verbose { - if let Some(path) = config_path { - println!( - "Using rustfmt config file {} for {}", - path.display(), - file.display() - ); + let num_cpus = 16; + let (send, recv) = std::sync::mpsc::channel(); + + let mut exit_code = 0; + let mut outstanding = 0; + // If the thread panics, the channel will just be dropped, + // so keep track of the spinning threads + let mut id = 0; + let mut handles: HashMap> = HashMap::new(); + let check = options.check; + + for file in files { + let cfg = config.clone(); + let opts = options.clone(); + let s = send.clone(); + let handle = std::thread::spawn(move || { + let my_id = id; + let mut session_out = Vec::new(); + let mut session = Session::new(cfg, Some(&mut session_out)); + if !file.exists() { + eprintln!("Error: file `{}` does not exist", file.to_str().unwrap()); + session.add_operational_error(); + } else if file.is_dir() { + eprintln!("Error: `{}` is a directory", file.to_str().unwrap()); + session.add_operational_error(); + } else { + // Check the file directory if the config-path could not be read or not provided + if cfg_path_is_none { + let (local_config, config_path) = + match load_config(Some(file.parent().unwrap()), Some(opts)) { + Ok((lc, cf)) => (lc, cf), + Err(e) => { + let _ = s.send((my_id, Err(e))); + return Ok::<_, std::io::Error>(my_id); } + }; + if local_config.verbose() == Verbosity::Verbose { + if let Some(path) = config_path { + println!( + "Using rustfmt config file {} for {}", + path.display(), + file.display() + ); } - - session.override_config(local_config, |sess| { - format_and_emit_report(sess, Input::File(file)) - }); - } else { - format_and_emit_report(&mut session, Input::File(file)); } - } - let exit_code = if session.has_operational_errors() - || session.has_parsing_errors() - || ((session.has_diff() || session.has_check_errors()) && options.check) - { - 1 + + session.override_config(local_config, |sess| { + format_and_emit_report(sess, Input::File(file)) + }); } else { - 0 - }; - drop(session); - Ok::<_, std::io::Error>((output, exit_code)) - }); - handles.push_back(handle); - if handles.len() >= num_cpus { - let (output, exit) = handles.pop_front().unwrap().join().unwrap().unwrap(); + format_and_emit_report(&mut session, Input::File(file)); + } + } + let exit_code = if session.has_operational_errors() + || session.has_parsing_errors() + || ((session.has_diff() || session.has_check_errors()) && check) + { + 1 + } else { + 0 + }; + drop(session); + let _ = s.send((my_id, Ok((session_out, exit_code)))); + Ok(my_id) + }); + handles.insert(id, handle); + id += 1; + outstanding += 1; + if outstanding >= num_cpus { + if let Ok((id, res)) = recv.recv() { + handles.remove(&id).unwrap().join().unwrap().unwrap(); + let (output, exit) = res?; let out = &mut stdout(); out.write_all(&output).unwrap(); if exit != 0 { exit_code = exit; } + outstanding -= 1; + } else { + break; } } - let out = &mut stdout(); - for handle in handles { - let (output, exit) = handle.join().unwrap().unwrap(); - out.write_all(&output).unwrap(); - if exit != 0 { - exit_code = exit; - } + } + drop(send); + let out = &mut stdout(); + while let Ok((id, res)) = recv.recv() { + handles.remove(&id).unwrap().join().unwrap().unwrap(); + let (output, exit) = res?; + out.write_all(&output).unwrap(); + if exit != 0 { + exit_code = exit; } - exit_code - }); + } + // These have errors + for (_id, jh) in handles { + jh.join().unwrap().unwrap(); + } // If we were given a path via dump-minimal-config, output any options // that were used during formatting as TOML. From 75151ccbd5315d36690df64920d90df22f582b71 Mon Sep 17 00:00:00 2001 From: MarcusGrass Date: Sun, 25 Feb 2024 02:59:54 +0100 Subject: [PATCH 03/23] Pass printer down the stack --- src/attr.rs | 6 +++-- src/bin/main.rs | 2 +- src/chains.rs | 2 +- src/closures.rs | 2 +- src/comment.rs | 19 +++++++++----- src/expr.rs | 5 ++-- src/formatting.rs | 10 ++++++-- src/imports.rs | 2 +- src/items.rs | 8 +++--- src/lib.rs | 13 +++++++--- src/lists.rs | 7 +++-- src/macros.rs | 7 ++--- src/matches.rs | 4 +-- src/missed_spans.rs | 4 +-- src/overflow.rs | 2 +- src/patterns.rs | 4 +-- src/print.rs | 62 +++++++++++++++++++++++++++++++++++++++++++++ src/reorder.rs | 2 +- src/rewrite.rs | 2 ++ src/types.rs | 2 +- src/vertical.rs | 2 +- src/visitor.rs | 10 ++++++-- 22 files changed, 136 insertions(+), 41 deletions(-) create mode 100644 src/print.rs diff --git a/src/attr.rs b/src/attr.rs index 4d83547d664..3fa5d97b2ed 100644 --- a/src/attr.rs +++ b/src/attr.rs @@ -147,7 +147,7 @@ fn format_derive( .tactic(tactic) .trailing_separator(trailing_separator) .ends_with_newline(false); - let item_str = write_list(&all_items, &fmt)?; + let item_str = write_list(&all_items, &fmt, context.printer)?; debug!("item_str: '{}'", item_str); @@ -235,6 +235,7 @@ fn rewrite_initial_doc_comments( &snippet, shape.comment(context.config), context.config, + context.printer, )?), )); } @@ -318,7 +319,7 @@ impl Rewrite for ast::Attribute { fn rewrite(&self, context: &RewriteContext<'_>, shape: Shape) -> Option { let snippet = context.snippet(self.span); if self.is_doc_comment() { - rewrite_doc_comment(snippet, shape.comment(context.config), context.config) + rewrite_doc_comment(snippet, shape.comment(context.config), context.config, context.printer) } else { let should_skip = self .ident() @@ -347,6 +348,7 @@ impl Rewrite for ast::Attribute { &doc_comment, shape.comment(context.config), context.config, + context.printer, ); } } diff --git a/src/bin/main.rs b/src/bin/main.rs index ff049758ff6..949a69e6da1 100644 --- a/src/bin/main.rs +++ b/src/bin/main.rs @@ -316,7 +316,7 @@ fn format( } } - let num_cpus = 16; + let num_cpus = 32; let (send, recv) = std::sync::mpsc::channel(); let mut exit_code = 0; diff --git a/src/chains.rs b/src/chains.rs index ea23690caed..73a8ab9bcb7 100644 --- a/src/chains.rs +++ b/src/chains.rs @@ -293,7 +293,7 @@ impl Rewrite for ChainItem { ), ChainItemKind::Await => ".await".to_owned(), ChainItemKind::Comment(ref comment, _) => { - rewrite_comment(comment, false, shape, context.config)? + rewrite_comment(comment, false, shape, context.config, context.printer)? } }; Some(format!("{rewrite}{}", "?".repeat(self.tries))) diff --git a/src/closures.rs b/src/closures.rs index 5bf29441b54..50b2427f392 100644 --- a/src/closures.rs +++ b/src/closures.rs @@ -323,7 +323,7 @@ fn rewrite_closure_fn_decl( let fmt = ListFormatting::new(param_shape, context.config) .tactic(tactic) .preserve_newline(true); - let list_str = write_list(&item_vec, &fmt)?; + let list_str = write_list(&item_vec, &fmt, context.printer)?; let mut prefix = format!("{binder}{const_}{immovable}{coro}{mover}|{list_str}|"); if !ret_str.is_empty() { diff --git a/src/comment.rs b/src/comment.rs index 55e710bd27d..c66480727ac 100644 --- a/src/comment.rs +++ b/src/comment.rs @@ -16,6 +16,7 @@ use crate::utils::{ trimmed_last_line_width, unicode_str_width, }; use crate::{ErrorKind, FormattingError}; +use crate::print::Printer; lazy_static! { /// A regex matching reference doc links. @@ -248,8 +249,8 @@ pub(crate) fn combine_strs_with_missing_comments( Some(result) } -pub(crate) fn rewrite_doc_comment(orig: &str, shape: Shape, config: &Config) -> Option { - identify_comment(orig, false, shape, config, true) +pub(crate) fn rewrite_doc_comment(orig: &str, shape: Shape, config: &Config, printer: &Printer) -> Option { + identify_comment(orig, false, shape, config, true, printer) } pub(crate) fn rewrite_comment( @@ -257,8 +258,9 @@ pub(crate) fn rewrite_comment( block_style: bool, shape: Shape, config: &Config, + printer: &Printer, ) -> Option { - identify_comment(orig, block_style, shape, config, false) + identify_comment(orig, block_style, shape, config, false, printer) } fn identify_comment( @@ -267,6 +269,7 @@ fn identify_comment( shape: Shape, config: &Config, is_doc_comment: bool, + printer: &Printer, ) -> Option { let style = comment_style(orig, false); @@ -377,6 +380,7 @@ fn identify_comment( shape, config, is_doc_comment || style.is_doc_comment(), + printer, )? }; if rest.is_empty() { @@ -388,6 +392,7 @@ fn identify_comment( shape, config, is_doc_comment, + printer, ) .map(|rest_str| { format!( @@ -725,6 +730,7 @@ impl<'a> CommentRewrite<'a> { line: &'a str, has_leading_whitespace: bool, is_doc_comment: bool, + printer: &Printer, ) -> bool { let num_newlines = count_newlines(orig); let is_last = i == num_newlines; @@ -776,7 +782,7 @@ impl<'a> CommentRewrite<'a> { .min(config.max_width()); config.set().max_width(comment_max_width); if let Some(s) = - crate::format_code_block(&self.code_block_buffer, &config, false) + crate::format_code_block(&self.code_block_buffer, &config, false, printer) { trim_custom_comment_prefix(&s.snippet) } else { @@ -912,6 +918,7 @@ fn rewrite_comment_inner( shape: Shape, config: &Config, is_doc_comment: bool, + printer: &Printer, ) -> Option { let mut rewriter = CommentRewrite::new(orig, block_style, shape, config); @@ -941,7 +948,7 @@ fn rewrite_comment_inner( }); for (i, (line, has_leading_whitespace)) in lines.enumerate() { - if rewriter.handle_line(orig, i, line, has_leading_whitespace, is_doc_comment) { + if rewriter.handle_line(orig, i, line, has_leading_whitespace, is_doc_comment, printer) { break; } } @@ -1008,7 +1015,7 @@ pub(crate) fn rewrite_missing_comment( // check the span starts with a comment let pos = trimmed_snippet.find('/'); if !trimmed_snippet.is_empty() && pos.is_some() { - rewrite_comment(trimmed_snippet, false, shape, context.config) + rewrite_comment(trimmed_snippet, false, shape, context.config, context.printer) } else { Some(String::new()) } diff --git a/src/expr.rs b/src/expr.rs index 7808f891336..1bcd4deac17 100644 --- a/src/expr.rs +++ b/src/expr.rs @@ -496,6 +496,7 @@ fn block_prefix(context: &RewriteContext<'_>, block: &ast::Block, shape: Shape) true, Shape::legacy(budget, shape.indent + 7), context.config, + context.printer, )? ) } else { @@ -1688,7 +1689,7 @@ fn rewrite_struct_lit<'a>( force_no_trailing_comma || has_base_or_rest || !context.use_block_indent(), ); - write_list(&item_vec, &fmt)? + write_list(&item_vec, &fmt, context.printer)? }; let fields_str = @@ -1835,7 +1836,7 @@ fn rewrite_tuple_in_visual_indent_style<'a, T: 'a + IntoOverflowableItem<'a>>( let fmt = ListFormatting::new(nested_shape, context.config) .tactic(tactic) .ends_with_newline(false); - let list_str = write_list(&item_vec, &fmt)?; + let list_str = write_list(&item_vec, &fmt, context.printer)?; Some(format!("({list_str})")) } diff --git a/src/formatting.rs b/src/formatting.rs index 60361505a3f..9a9c753bc09 100644 --- a/src/formatting.rs +++ b/src/formatting.rs @@ -17,6 +17,7 @@ use crate::parse::session::ParseSess; use crate::utils::{contains_skip, count_newlines}; use crate::visitor::FmtVisitor; use crate::{modules, source_file, ErrorKind, FormatReport, Input, Session}; +use crate::print::Printer; mod generated; mod newline_style; @@ -45,7 +46,7 @@ impl<'b, T: Write + 'b> Session<'b, T> { } let config = &self.config.clone(); - let format_result = format_project(input, config, self, is_macro_def); + let format_result = format_project(input, config, self, is_macro_def, self.printer); format_result.map(|report| { self.errors.add(&report.internal.borrow().1); @@ -103,6 +104,7 @@ fn format_project( config: &Config, handler: &mut T, is_macro_def: bool, + printer: &Printer, ) -> Result { let mut timer = Timer::start(); @@ -130,7 +132,7 @@ fn format_project( } }; - let mut context = FormatContext::new(&krate, report, parse_session, config, handler); + let mut context = FormatContext::new(&krate, report, parse_session, config, handler, printer); let files = modules::ModResolver::new( &context.parse_session, directory_ownership.unwrap_or(DirectoryOwnership::UnownedViaBlock), @@ -181,6 +183,7 @@ struct FormatContext<'a, T: FormatHandler> { parse_session: ParseSess, config: &'a Config, handler: &'a mut T, + printer: &'a Printer, } impl<'a, T: FormatHandler + 'a> FormatContext<'a, T> { @@ -190,6 +193,7 @@ impl<'a, T: FormatHandler + 'a> FormatContext<'a, T> { parse_session: ParseSess, config: &'a Config, handler: &'a mut T, + printer: &'a Printer, ) -> Self { FormatContext { krate, @@ -197,6 +201,7 @@ impl<'a, T: FormatHandler + 'a> FormatContext<'a, T> { parse_session, config, handler, + printer, } } @@ -216,6 +221,7 @@ impl<'a, T: FormatHandler + 'a> FormatContext<'a, T> { &self.parse_session, self.config, &snippet_provider, + &self.printer, self.report.clone(), ); visitor.skip_context.update_with_attrs(&self.krate.attrs); diff --git a/src/imports.rs b/src/imports.rs index 05195553c08..00bbf5243d5 100644 --- a/src/imports.rs +++ b/src/imports.rs @@ -1030,7 +1030,7 @@ fn rewrite_nested_use_tree( .preserve_newline(true) .nested(has_nested_list); - let list_str = write_list(&list_items, &fmt)?; + let list_str = write_list(&list_items, &fmt, context.printer)?; let result = if (list_str.contains('\n') || list_str.len() > remaining_width diff --git a/src/items.rs b/src/items.rs index e7ff5ff818b..f581fdc9657 100644 --- a/src/items.rs +++ b/src/items.rs @@ -635,7 +635,7 @@ impl<'a> FmtVisitor<'a> { .trailing_separator(self.config.trailing_comma()) .preserve_newline(true); - let list = write_list(&items, &fmt)?; + let list = write_list(&items, &fmt, self.printer)?; result.push_str(&list); result.push_str(&original_offset.to_string_with_newline(self.config)); result.push('}'); @@ -2769,7 +2769,7 @@ fn rewrite_params( .trailing_separator(trailing_separator) .ends_with_newline(tactic.ends_with_newline(context.config.indent_style())) .preserve_newline(true); - write_list(¶m_items, &fmt) + write_list(¶m_items, &fmt, context.printer) } fn compute_budgets_for_params( @@ -3026,7 +3026,7 @@ fn rewrite_bounds_on_where_clause( .tactic(shape_tactic) .trailing_separator(comma_tactic) .preserve_newline(preserve_newline); - write_list(&items.collect::>(), &fmt) + write_list(&items.collect::>(), &fmt, context.printer) } fn rewrite_where_clause( @@ -3102,7 +3102,7 @@ fn rewrite_where_clause( .trailing_separator(comma_tactic) .ends_with_newline(tactic.ends_with_newline(context.config.indent_style())) .preserve_newline(true); - let preds_str = write_list(&item_vec, &fmt)?; + let preds_str = write_list(&item_vec, &fmt, context.printer)?; let end_length = if terminator == "{" { // If the brace is on the next line we don't need to count it otherwise it needs two diff --git a/src/lib.rs b/src/lib.rs index a67adb1478f..978f4d4e9f8 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -55,6 +55,7 @@ pub use crate::config::{ }; pub use crate::format_report_formatter::{FormatReportFormatter, FormatReportFormatterBuilder}; +use crate::print::Printer; pub use crate::rustfmt_diff::{ModifiedChunk, ModifiedLines}; @@ -99,6 +100,7 @@ mod test; mod types; mod vertical; pub(crate) mod visitor; +mod print; /// The various errors that can occur during formatting. Note that not all of /// these can currently be propagated to clients. @@ -298,7 +300,7 @@ impl fmt::Display for FormatReport { /// Format the given snippet. The snippet is expected to be *complete* code. /// When we cannot parse the given snippet, this function returns `None`. -fn format_snippet(snippet: &str, config: &Config, is_macro_def: bool) -> Option { +fn format_snippet(snippet: &str, config: &Config, is_macro_def: bool, printer: &Printer) -> Option { let mut config = config.clone(); panic::catch_unwind(|| { let mut out: Vec = Vec::with_capacity(snippet.len() * 2); @@ -311,7 +313,7 @@ fn format_snippet(snippet: &str, config: &Config, is_macro_def: bool) -> Option< let (formatting_error, result) = { let input = Input::Text(snippet.into()); - let mut session = Session::new(config, Some(&mut out)); + let mut session = Session::new(config, Some(&mut out), printer); let result = session.format_input_inner(input, is_macro_def); ( session.errors.has_macro_format_failure @@ -343,6 +345,7 @@ fn format_code_block( code_snippet: &str, config: &Config, is_macro_def: bool, + printer: &Printer, ) -> Option { const FN_MAIN_PREFIX: &str = "fn main() {\n"; @@ -376,7 +379,7 @@ fn format_code_block( config_with_unix_newline .set() .newline_style(NewlineStyle::Unix); - let mut formatted = format_snippet(&snippet, &config_with_unix_newline, is_macro_def)?; + let mut formatted = format_snippet(&snippet, &config_with_unix_newline, is_macro_def, printer)?; // Remove wrapping main block formatted.unwrap_code_block(); @@ -436,13 +439,14 @@ fn format_code_block( pub struct Session<'b, T: Write> { pub config: Config, pub out: Option<&'b mut T>, + pub printer: &'b Printer, pub(crate) errors: ReportedErrors, source_file: SourceFile, emitter: Box, } impl<'b, T: Write + 'b> Session<'b, T> { - pub fn new(config: Config, mut out: Option<&'b mut T>) -> Session<'b, T> { + pub fn new(config: Config, mut out: Option<&'b mut T>, printer: &'b Printer) -> Session<'b, T> { let emitter = create_emitter(&config); if let Some(ref mut out) = out { @@ -455,6 +459,7 @@ impl<'b, T: Write + 'b> Session<'b, T> { emitter, errors: ReportedErrors::default(), source_file: SourceFile::new(), + printer, } } diff --git a/src/lists.rs b/src/lists.rs index 41afef279e9..9afff476fa7 100644 --- a/src/lists.rs +++ b/src/lists.rs @@ -8,6 +8,7 @@ use rustc_span::BytePos; use crate::comment::{find_comment_end, rewrite_comment, FindUncommented}; use crate::config::lists::*; use crate::config::{Config, IndentStyle}; +use crate::print::Printer; use crate::rewrite::RewriteContext; use crate::shape::{Indent, Shape}; use crate::utils::{ @@ -257,7 +258,7 @@ where } // Format a list of commented items into a string. -pub(crate) fn write_list(items: I, formatting: &ListFormatting<'_>) -> Option +pub(crate) fn write_list(items: I, formatting: &ListFormatting<'_>, printer: &Printer) -> Option where I: IntoIterator + Clone, T: AsRef, @@ -363,7 +364,7 @@ where let block_mode = tactic == DefinitiveListTactic::Horizontal; // Width restriction is only relevant in vertical mode. let comment = - rewrite_comment(comment, block_mode, formatting.shape, formatting.config)?; + rewrite_comment(comment, block_mode, formatting.shape, formatting.config, printer)?; result.push_str(&comment); if !inner_item.is_empty() { @@ -409,6 +410,7 @@ where true, Shape::legacy(formatting.shape.width, Indent::empty()), formatting.config, + printer, )?; result.push(' '); @@ -457,6 +459,7 @@ where block_style, comment_shape, formatting.config, + printer, ) }; diff --git a/src/macros.rs b/src/macros.rs index 6e114c76f26..7ea5c900812 100644 --- a/src/macros.rs +++ b/src/macros.rs @@ -31,6 +31,7 @@ use crate::lists::{itemize_list, write_list, ListFormatting}; use crate::overflow; use crate::parse::macros::lazy_static::parse_lazy_static; use crate::parse::macros::{parse_expr, parse_macro_args, ParsedMacroArgs}; +use crate::print::Printer; use crate::rewrite::{Rewrite, RewriteContext}; use crate::shape::{Indent, Shape}; use crate::source_map::SpanUtils; @@ -476,7 +477,7 @@ pub(crate) fn rewrite_macro_def( result += &arm_shape.indent.to_string_with_newline(context.config); } - match write_list(&branch_items, &fmt) { + match write_list(&branch_items, &fmt, context.printer) { Some(ref s) => result += s, None => return snippet, } @@ -1289,12 +1290,12 @@ impl MacroBranch { config.set().max_width(new_width); // First try to format as items, then as statements. - let new_body_snippet = match crate::format_snippet(&body_str, &config, true) { + let new_body_snippet = match crate::format_snippet(&body_str, &config, true, context.printer) { Some(new_body) => new_body, None => { let new_width = new_width + config.tab_spaces(); config.set().max_width(new_width); - match crate::format_code_block(&body_str, &config, true) { + match crate::format_code_block(&body_str, &config, true, context.printer) { Some(new_body) => new_body, None => return None, } diff --git a/src/matches.rs b/src/matches.rs index 5e36d9e857d..25bb928d89e 100644 --- a/src/matches.rs +++ b/src/matches.rs @@ -211,7 +211,7 @@ fn rewrite_match_arms( .separator("") .preserve_newline(true); - write_list(&arms_vec, &fmt) + write_list(&arms_vec, &fmt, context.printer) } fn rewrite_match_arm( @@ -408,7 +408,7 @@ fn rewrite_match_body( if comment_str.is_empty() { String::new() } else { - rewrite_comment(comment_str, false, shape, context.config)? + rewrite_comment(comment_str, false, shape, context.config, context.printer)? } }; diff --git a/src/missed_spans.rs b/src/missed_spans.rs index 28edcb784b4..05eb04ea670 100644 --- a/src/missed_spans.rs +++ b/src/missed_spans.rs @@ -281,13 +281,13 @@ impl<'a> FmtVisitor<'a> { let other_lines = &subslice[offset + 1..]; let comment_str = - rewrite_comment(other_lines, false, comment_shape, self.config) + rewrite_comment(other_lines, false, comment_shape, self.config, self.printer) .unwrap_or_else(|| String::from(other_lines)); self.push_str(&comment_str); } } } else { - let comment_str = rewrite_comment(subslice, false, comment_shape, self.config) + let comment_str = rewrite_comment(subslice, false, comment_shape, self.config, self.printer) .unwrap_or_else(|| String::from(subslice)); self.push_str(&comment_str); } diff --git a/src/overflow.rs b/src/overflow.rs index c44f3788d97..a2fb2d881b3 100644 --- a/src/overflow.rs +++ b/src/overflow.rs @@ -642,7 +642,7 @@ impl<'a> Context<'a> { .trailing_separator(trailing_separator) .ends_with_newline(ends_with_newline); - write_list(&list_items, &fmt) + write_list(&list_items, &fmt, self.context.printer) .map(|items_str| (tactic == DefinitiveListTactic::Horizontal, items_str)) } diff --git a/src/patterns.rs b/src/patterns.rs index 0fa6edaa5d7..4ffecd8836e 100644 --- a/src/patterns.rs +++ b/src/patterns.rs @@ -100,7 +100,7 @@ impl Rewrite for Pat { .separator(" |") .separator_place(context.config.binop_separator()) .ends_with_newline(false); - write_list(&items, &fmt) + write_list(&items, &fmt, context.printer) } PatKind::Box(ref pat) => rewrite_unary_prefix(context, "box ", &**pat, shape), PatKind::Ident(BindingAnnotation(by_ref, mutability), ident, ref sub_pat) => { @@ -325,7 +325,7 @@ fn rewrite_struct_pat( let nested_shape = shape_for_tactic(tactic, h_shape, v_shape); let fmt = struct_lit_formatting(nested_shape, tactic, context, false); - let mut fields_str = write_list(&item_vec, &fmt)?; + let mut fields_str = write_list(&item_vec, &fmt, context.printer)?; let one_line_width = h_shape.map_or(0, |shape| shape.width); let has_trailing_comma = fmt.needs_trailing_separator(); diff --git a/src/print.rs b/src/print.rs new file mode 100644 index 00000000000..7246a23a145 --- /dev/null +++ b/src/print.rs @@ -0,0 +1,62 @@ +use std::sync::Mutex; +use crate::Color; + +pub struct Printer { + inner: Mutex, +} + +struct PrinterInner { + color: Color, + messages: Vec +} + +impl Printer { + #[inline] + pub fn push_msg(&self, msg: PrintMessage) { + self.inner.lock().unwrap().messages.push(msg); + } +} + +#[macro_export] +macro_rules! buf_println { + ($pb: expr, $($arg:tt)*) => {{ + let mut msg_buf = Vec::new(); + let _ = writeln!(&mut msg_buf, $($arg)*); + $pb.push_msg(PrintMessage::Stdout(msg_buf)); + }}; +} + +#[macro_export] +macro_rules! buf_eprintln { + ($pb: expr, $($arg:tt)*) => {{ + let mut msg_buf = Vec::new(); + let _ = writeln!(&mut msg_buf, $($arg)*); + $pb.push_msg(PrintMessage::StdErr(msg_buf)); + }}; +} + +#[macro_export] +macro_rules! buf_term_println { + ($pb: expr, $col:expr, $($arg:tt)*) => {{ + let mut msg_buf = Vec::new(); + let _ = writeln!(&mut msg_buf, $($arg)*); + $pb.push_msg(PrintMessage::Term(TermMessage::new(msg_buf, $col))); + }}; +} + +pub enum PrintMessage { + Stdout(Vec), + StdErr(Vec), + Term(TermMessage) +} + +pub struct TermMessage { + message: Vec, + color: Option, +} + +impl TermMessage { + pub fn new(message: Vec, color: Option) -> Self { + Self { message, color } + } +} diff --git a/src/reorder.rs b/src/reorder.rs index 3e14f9f1272..c6d999d9ce3 100644 --- a/src/reorder.rs +++ b/src/reorder.rs @@ -59,7 +59,7 @@ fn wrap_reorderable_items( let fmt = ListFormatting::new(shape, context.config) .separator("") .align_comments(false); - write_list(list_items, &fmt) + write_list(list_items, &fmt, context.printer) } fn rewrite_reorderable_item( diff --git a/src/rewrite.rs b/src/rewrite.rs index 4a3bd129d16..fbaf6ccd5e3 100644 --- a/src/rewrite.rs +++ b/src/rewrite.rs @@ -12,6 +12,7 @@ use crate::shape::Shape; use crate::skip::SkipContext; use crate::visitor::SnippetProvider; use crate::FormatReport; +use crate::print::Printer; pub(crate) trait Rewrite { /// Rewrite self into shape. @@ -43,6 +44,7 @@ pub(crate) struct RewriteContext<'a> { pub(crate) report: FormatReport, pub(crate) skip_context: SkipContext, pub(crate) skipped_range: Rc>>, + pub(crate) printer: &'a Printer, } pub(crate) struct InsideMacroGuard { diff --git a/src/types.rs b/src/types.rs index cd2582e66be..cec07936e3e 100644 --- a/src/types.rs +++ b/src/types.rs @@ -365,7 +365,7 @@ where .trailing_separator(trailing_separator) .ends_with_newline(tactic.ends_with_newline(context.config.indent_style())) .preserve_newline(true); - (write_list(&item_vec, &fmt)?, tactic) + (write_list(&item_vec, &fmt, context.printer)?, tactic) }; let args = if tactic == DefinitiveListTactic::Horizontal diff --git a/src/vertical.rs b/src/vertical.rs index a06bc995aa5..f3c20c9e806 100644 --- a/src/vertical.rs +++ b/src/vertical.rs @@ -267,7 +267,7 @@ fn rewrite_aligned_items_inner( .tactic(tactic) .trailing_separator(separator_tactic) .preserve_newline(true); - write_list(&items, &fmt) + write_list(&items, &fmt, context.printer) } /// Returns the index in `fields` up to which a field belongs to the current group. diff --git a/src/visitor.rs b/src/visitor.rs index 61e147ed8f5..8ccbd7eaae3 100644 --- a/src/visitor.rs +++ b/src/visitor.rs @@ -28,6 +28,7 @@ use crate::utils::{ last_line_width, mk_sp, ptr_vec_to_ref_vec, rewrite_ident, starts_with_newline, stmt_expr, }; use crate::{ErrorKind, FormatReport, FormattingError}; +use crate::print::Printer; /// Creates a string slice corresponding to the specified span. pub(crate) struct SnippetProvider { @@ -87,6 +88,7 @@ pub(crate) struct FmtVisitor<'a> { pub(crate) report: FormatReport, pub(crate) skip_context: SkipContext, pub(crate) is_macro_def: bool, + pub(crate) printer: &'a Printer, } impl<'a> Drop for FmtVisitor<'a> { @@ -315,7 +317,7 @@ impl<'b, 'a: 'b> FmtVisitor<'a> { // put the other lines below it, shaping it as needed let other_lines = &sub_slice[offset + 1..]; let comment_str = - rewrite_comment(other_lines, false, comment_shape, config); + rewrite_comment(other_lines, false, comment_shape, config, self.printer); match comment_str { Some(ref s) => self.push_str(s), None => self.push_str(other_lines), @@ -345,7 +347,7 @@ impl<'b, 'a: 'b> FmtVisitor<'a> { self.push_str(&self.block_indent.to_string_with_newline(config)); } - let comment_str = rewrite_comment(&sub_slice, false, comment_shape, config); + let comment_str = rewrite_comment(&sub_slice, false, comment_shape, config, self.printer); match comment_str { Some(ref s) => self.push_str(s), None => self.push_str(&sub_slice), @@ -757,6 +759,7 @@ impl<'b, 'a: 'b> FmtVisitor<'a> { ctx.parse_sess, ctx.config, ctx.snippet_provider, + ctx.printer, ctx.report.clone(), ); visitor.skip_context.update(ctx.skip_context.clone()); @@ -768,6 +771,7 @@ impl<'b, 'a: 'b> FmtVisitor<'a> { parse_session: &'a ParseSess, config: &'a Config, snippet_provider: &'a SnippetProvider, + printer: &'a Printer, report: FormatReport, ) -> FmtVisitor<'a> { let mut skip_context = SkipContext::default(); @@ -794,6 +798,7 @@ impl<'b, 'a: 'b> FmtVisitor<'a> { macro_rewrite_failure: false, report, skip_context, + printer, } } @@ -1014,6 +1019,7 @@ impl<'b, 'a: 'b> FmtVisitor<'a> { report: self.report.clone(), skip_context: self.skip_context.clone(), skipped_range: self.skipped_range.clone(), + printer: self.printer, } } } From 7456b978e4caa4da464052b7856753cf66120acf Mon Sep 17 00:00:00 2001 From: MarcusGrass Date: Sun, 25 Feb 2024 03:09:16 +0100 Subject: [PATCH 04/23] Compiles --- src/bin/main.rs | 50 +++++++++++++++++++++++++++++------------ src/git-rustfmt/main.rs | 4 +++- src/lib.rs | 2 +- src/print.rs | 15 ++++++++++--- 4 files changed, 52 insertions(+), 19 deletions(-) diff --git a/src/bin/main.rs b/src/bin/main.rs index 949a69e6da1..6f3a24e718a 100644 --- a/src/bin/main.rs +++ b/src/bin/main.rs @@ -17,6 +17,8 @@ use std::str::FromStr; use std::thread::JoinHandle; use getopts::{Matches, Options}; +use rustfmt_nightly::buf_eprintln; +use rustfmt_nightly::print::Printer; use crate::rustfmt::{ load_config, CliOptions, Color, Config, Edition, EmitMode, FileLines, FileName, @@ -290,7 +292,8 @@ fn format_string(input: String, options: GetOptsOptions) -> Result { } let out = &mut stdout(); - let mut session = Session::new(config, Some(out)); + let printer = Printer::new(config.color()); + let mut session = Session::new(config, Some(out), &printer); format_and_emit_report(&mut session, Input::Text(input)); let exit_code = if session.has_operational_errors() || session.has_parsing_errors() { @@ -301,6 +304,13 @@ fn format_string(input: String, options: GetOptsOptions) -> Result { Ok(exit_code) } +struct ThreadedFileOutput { + session_result: Result, std::io::Error>, + id: i32, + exit_code: i32, + printer: Printer, +} + fn format( files: Vec, minimal_config_path: Option, @@ -326,7 +336,7 @@ fn format( let mut id = 0; let mut handles: HashMap> = HashMap::new(); let check = options.check; - + let color = config.color(); for file in files { let cfg = config.clone(); let opts = options.clone(); @@ -334,9 +344,10 @@ fn format( let handle = std::thread::spawn(move || { let my_id = id; let mut session_out = Vec::new(); - let mut session = Session::new(cfg, Some(&mut session_out)); + let printer = Printer::new(color); + let mut session = Session::new(cfg, Some(&mut session_out), &printer); if !file.exists() { - eprintln!("Error: file `{}` does not exist", file.to_str().unwrap()); + buf_eprintln!(printer, "Error: file `{}` does not exist", file.to_str().unwrap()); session.add_operational_error(); } else if file.is_dir() { eprintln!("Error: `{}` is a directory", file.to_str().unwrap()); @@ -348,7 +359,13 @@ fn format( match load_config(Some(file.parent().unwrap()), Some(opts)) { Ok((lc, cf)) => (lc, cf), Err(e) => { - let _ = s.send((my_id, Err(e))); + drop(session); + let _ = s.send(ThreadedFileOutput { + session_result: Err(e), + id, + exit_code, + printer, + }); return Ok::<_, std::io::Error>(my_id); } }; @@ -378,20 +395,25 @@ fn format( 0 }; drop(session); - let _ = s.send((my_id, Ok((session_out, exit_code)))); + let _ = s.send(ThreadedFileOutput { + session_result: Ok(session_out), + id, + exit_code, + printer, + }); Ok(my_id) }); handles.insert(id, handle); id += 1; outstanding += 1; if outstanding >= num_cpus { - if let Ok((id, res)) = recv.recv() { + if let Ok(thread_out) = recv.recv() { handles.remove(&id).unwrap().join().unwrap().unwrap(); - let (output, exit) = res?; + let output = thread_out.session_result?; let out = &mut stdout(); out.write_all(&output).unwrap(); - if exit != 0 { - exit_code = exit; + if thread_out.exit_code != 0 { + exit_code = thread_out.exit_code; } outstanding -= 1; } else { @@ -401,12 +423,12 @@ fn format( } drop(send); let out = &mut stdout(); - while let Ok((id, res)) = recv.recv() { + while let Ok(thread_out) = recv.recv() { handles.remove(&id).unwrap().join().unwrap().unwrap(); - let (output, exit) = res?; + let output = thread_out.session_result?; out.write_all(&output).unwrap(); - if exit != 0 { - exit_code = exit; + if thread_out.exit_code != 0 { + exit_code = thread_out.exit_code; } } // These have errors diff --git a/src/git-rustfmt/main.rs b/src/git-rustfmt/main.rs index 3059d917c6b..e6f662e83ab 100644 --- a/src/git-rustfmt/main.rs +++ b/src/git-rustfmt/main.rs @@ -10,6 +10,7 @@ use std::str::FromStr; use getopts::{Matches, Options}; use rustfmt_nightly as rustfmt; use tracing_subscriber::EnvFilter; +use rustfmt_nightly::print::Printer; use crate::rustfmt::{load_config, CliOptions, FormatReportFormatterBuilder, Input, Session}; @@ -63,7 +64,8 @@ fn fmt_files(files: &[&str]) -> i32 { let mut exit_code = 0; let mut out = stdout(); - let mut session = Session::new(config, Some(&mut out)); + let printer = Printer::new(config.color()); + let mut session = Session::new(config, Some(&mut out), &printer); for file in files { let report = session.format(Input::File(PathBuf::from(file))).unwrap(); if report.has_warnings() { diff --git a/src/lib.rs b/src/lib.rs index 978f4d4e9f8..7d7eac64d50 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -100,7 +100,7 @@ mod test; mod types; mod vertical; pub(crate) mod visitor; -mod print; +pub mod print; /// The various errors that can occur during formatting. Note that not all of /// these can currently be propagated to clients. diff --git a/src/print.rs b/src/print.rs index 7246a23a145..7e2a0b6a5a6 100644 --- a/src/print.rs +++ b/src/print.rs @@ -11,6 +11,15 @@ struct PrinterInner { } impl Printer { + + pub fn new(term_output_color: Color) -> Self { + Self { + inner: Mutex::new(PrinterInner { + color: term_output_color, + messages: vec![], + }), + } + } #[inline] pub fn push_msg(&self, msg: PrintMessage) { self.inner.lock().unwrap().messages.push(msg); @@ -22,7 +31,7 @@ macro_rules! buf_println { ($pb: expr, $($arg:tt)*) => {{ let mut msg_buf = Vec::new(); let _ = writeln!(&mut msg_buf, $($arg)*); - $pb.push_msg(PrintMessage::Stdout(msg_buf)); + $pb.push_msg($crate::print::PrintMessage::Stdout(msg_buf)); }}; } @@ -31,7 +40,7 @@ macro_rules! buf_eprintln { ($pb: expr, $($arg:tt)*) => {{ let mut msg_buf = Vec::new(); let _ = writeln!(&mut msg_buf, $($arg)*); - $pb.push_msg(PrintMessage::StdErr(msg_buf)); + $pb.push_msg($crate::print::PrintMessage::StdErr(msg_buf)); }}; } @@ -40,7 +49,7 @@ macro_rules! buf_term_println { ($pb: expr, $col:expr, $($arg:tt)*) => {{ let mut msg_buf = Vec::new(); let _ = writeln!(&mut msg_buf, $($arg)*); - $pb.push_msg(PrintMessage::Term(TermMessage::new(msg_buf, $col))); + $pb.push_msg($crate::print::PrintMessage::Term(TermMessage::new(msg_buf, $col))); }}; } From 378e10658257ccec0826cab6d34272fb656cc748 Mon Sep 17 00:00:00 2001 From: MarcusGrass Date: Sun, 25 Feb 2024 03:47:28 +0100 Subject: [PATCH 05/23] Rip out auto term-printing --- Cargo.lock | 10 ++++++++ Cargo.toml | 1 + src/bin/main.rs | 18 +++++-------- src/formatting.rs | 6 ++--- src/macros.rs | 1 - src/parse/session.rs | 8 ++++-- src/print.rs | 61 ++++++++++++++++++++++++++++++++++++-------- src/rustfmt_diff.rs | 15 ++++++----- 8 files changed, 86 insertions(+), 34 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 7465a4f9521..66e1840eb86 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -547,6 +547,7 @@ dependencies = [ "serde", "serde_json", "term", + "termcolor", "thiserror", "toml", "tracing", @@ -669,6 +670,15 @@ dependencies = [ "winapi", ] +[[package]] +name = "termcolor" +version = "1.4.1" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "06794f8f6c5c898b3275aebefa6b8a1cb24cd2c6c79397ab15774837a0bc5755" +dependencies = [ + "winapi-util", +] + [[package]] name = "thiserror" version = "1.0.40" diff --git a/Cargo.toml b/Cargo.toml index bcd3b420acb..8f866de4719 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -49,6 +49,7 @@ regex = "1.7" serde = { version = "1.0.160", features = ["derive"] } serde_json = "1.0" term = "0.7" +termcolor = "1.4.1" thiserror = "1.0.40" toml = "0.7.4" tracing = "0.1.37" diff --git a/src/bin/main.rs b/src/bin/main.rs index 6f3a24e718a..3f60eff35cd 100644 --- a/src/bin/main.rs +++ b/src/bin/main.rs @@ -17,7 +17,7 @@ use std::str::FromStr; use std::thread::JoinHandle; use getopts::{Matches, Options}; -use rustfmt_nightly::buf_eprintln; +use rustfmt_nightly::{buf_eprintln, buf_println}; use rustfmt_nightly::print::Printer; use crate::rustfmt::{ @@ -350,7 +350,7 @@ fn format( buf_eprintln!(printer, "Error: file `{}` does not exist", file.to_str().unwrap()); session.add_operational_error(); } else if file.is_dir() { - eprintln!("Error: `{}` is a directory", file.to_str().unwrap()); + buf_eprintln!(printer, "Error: `{}` is a directory", file.to_str().unwrap()); session.add_operational_error(); } else { // Check the file directory if the config-path could not be read or not provided @@ -371,11 +371,9 @@ fn format( }; if local_config.verbose() == Verbosity::Verbose { if let Some(path) = config_path { - println!( - "Using rustfmt config file {} for {}", + buf_println!(printer, "Using rustfmt config file {} for {}", path.display(), - file.display() - ); + file.display()); } } @@ -451,16 +449,14 @@ fn format_and_emit_report(session: &mut Session<'_, T>, input: Input) match session.format(input) { Ok(report) => { if report.has_warnings() { - eprintln!( - "{}", + buf_eprintln!(session.printer, "{}", FormatReportFormatterBuilder::new(&report) .enable_colors(should_print_with_colors(session)) - .build() - ); + .build()); } } Err(msg) => { - eprintln!("Error writing files: {msg}"); + buf_eprintln!(session.printer, "Error writing files: {msg}"); session.add_operational_error(); } } diff --git a/src/formatting.rs b/src/formatting.rs index 9a9c753bc09..2825d8bc502 100644 --- a/src/formatting.rs +++ b/src/formatting.rs @@ -16,7 +16,7 @@ use crate::parse::parser::{DirectoryOwnership, Parser, ParserError}; use crate::parse::session::ParseSess; use crate::utils::{contains_skip, count_newlines}; use crate::visitor::FmtVisitor; -use crate::{modules, source_file, ErrorKind, FormatReport, Input, Session}; +use crate::{modules, source_file, ErrorKind, FormatReport, Input, Session, buf_eprintln}; use crate::print::Printer; mod generated; @@ -111,7 +111,7 @@ fn format_project( let main_file = input.file_name(); let input_is_stdin = main_file == FileName::Stdin; - let parse_session = ParseSess::new(config)?; + let parse_session = ParseSess::new(config, printer)?; if config.skip_children() && parse_session.ignore_file(&main_file) { return Ok(FormatReport::new()); } @@ -125,7 +125,7 @@ fn format_project( Err(e) => { let forbid_verbose = input_is_stdin || e != ParserError::ParsePanicError; should_emit_verbose(forbid_verbose, config, || { - eprintln!("The Rust parser panicked"); + buf_eprintln!(printer, "The Rust parser panicked"); }); report.add_parsing_error(); return Ok(report); diff --git a/src/macros.rs b/src/macros.rs index 7ea5c900812..a4ca34890db 100644 --- a/src/macros.rs +++ b/src/macros.rs @@ -31,7 +31,6 @@ use crate::lists::{itemize_list, write_list, ListFormatting}; use crate::overflow; use crate::parse::macros::lazy_static::parse_lazy_static; use crate::parse::macros::{parse_expr, parse_macro_args, ParsedMacroArgs}; -use crate::print::Printer; use crate::rewrite::{Rewrite, RewriteContext}; use crate::shape::{Indent, Shape}; use crate::source_map::SpanUtils; diff --git a/src/parse/session.rs b/src/parse/session.rs index 5de6ee49acf..7779c0d2574 100644 --- a/src/parse/session.rs +++ b/src/parse/session.rs @@ -19,6 +19,7 @@ use crate::source_map::LineRangeUtils; use crate::utils::starts_with_newline; use crate::visitor::SnippetProvider; use crate::{Config, ErrorKind, FileName}; +use crate::print::Printer; /// ParseSess holds structs necessary for constructing a parser. pub(crate) struct ParseSess { @@ -124,6 +125,7 @@ fn default_dcx( can_reset: Lrc, show_parse_errors: bool, color: Color, + printer: &Printer, ) -> DiagCtxt { let supports_color = term::stderr().map_or(false, |term| term.supports_color()); let emit_color = if supports_color { @@ -139,7 +141,8 @@ fn default_dcx( rustc_driver::DEFAULT_LOCALE_RESOURCES.to_vec(), false, ); - Box::new(EmitterWriter::stderr(emit_color, fallback_bundle).sm(Some(source_map.clone()))) + Box::new(EmitterWriter::new(Box::new(printer.clone()), fallback_bundle)) + //Box::new(EmitterWriter::stderr(emit_color, fallback_bundle).sm(Some(source_map.clone()))) }; DiagCtxt::with_emitter(Box::new(SilentOnIgnoredFilesEmitter { has_non_ignorable_parser_errors: false, @@ -151,7 +154,7 @@ fn default_dcx( } impl ParseSess { - pub(crate) fn new(config: &Config) -> Result { + pub(crate) fn new(config: &Config, printer: &Printer) -> Result { let ignore_path_set = match IgnorePathSet::from_ignore_list(&config.ignore()) { Ok(ignore_path_set) => Lrc::new(ignore_path_set), Err(e) => return Err(ErrorKind::InvalidGlobPattern(e)), @@ -165,6 +168,7 @@ impl ParseSess { Lrc::clone(&can_reset_errors), config.show_parse_errors(), config.color(), + printer, ); let parse_sess = RawParseSess::with_dcx(dcx, source_map); diff --git a/src/print.rs b/src/print.rs index 7e2a0b6a5a6..3aa7aaf115e 100644 --- a/src/print.rs +++ b/src/print.rs @@ -1,25 +1,66 @@ -use std::sync::Mutex; +use std::io::Write; +use std::sync::{Arc, Mutex}; +use rustc_errors::{ColorSpec, WriteColor, Color as RustColor}; use crate::Color; +#[derive(Clone)] pub struct Printer { - inner: Mutex, + inner: Arc>, +} + +impl Write for Printer { + fn write(&mut self, buf: &[u8]) -> std::io::Result { + let mut inner = self.inner.lock().unwrap(); + let col = inner.current_color; + inner.messages.push(PrintMessage::Term(TermMessage::new(buf.to_vec(), col))); + Ok(buf.len()) + } + + #[inline] + fn flush(&mut self) -> std::io::Result<()> { + Ok(()) + } +} + +impl WriteColor for Printer { + #[inline] + fn supports_color(&self) -> bool { + self.inner.lock().unwrap().supports_color + } + + #[inline] + fn set_color(&mut self, spec: &ColorSpec) -> std::io::Result<()> { + self.inner.lock().unwrap().current_color = spec.fg().copied(); + Ok(()) + } + + #[inline] + fn reset(&mut self) -> std::io::Result<()> { + self.inner.lock().unwrap().current_color.take(); + Ok(()) + } } struct PrinterInner { - color: Color, - messages: Vec + color_setting: Color, + current_color: Option, + messages: Vec, + supports_color: bool, } impl Printer { pub fn new(term_output_color: Color) -> Self { Self { - inner: Mutex::new(PrinterInner { - color: term_output_color, + inner: Arc::new(Mutex::new(PrinterInner { + color_setting: term_output_color, + current_color: None, messages: vec![], - }), + supports_color: true, // Todo: Actually check + })), } } + #[inline] pub fn push_msg(&self, msg: PrintMessage) { self.inner.lock().unwrap().messages.push(msg); @@ -49,7 +90,7 @@ macro_rules! buf_term_println { ($pb: expr, $col:expr, $($arg:tt)*) => {{ let mut msg_buf = Vec::new(); let _ = writeln!(&mut msg_buf, $($arg)*); - $pb.push_msg($crate::print::PrintMessage::Term(TermMessage::new(msg_buf, $col))); + $pb.push_msg($crate::print::PrintMessage::Term($crate::print::TermMessage::new(msg_buf, $col))); }}; } @@ -61,11 +102,11 @@ pub enum PrintMessage { pub struct TermMessage { message: Vec, - color: Option, + color: Option, } impl TermMessage { - pub fn new(message: Vec, color: Option) -> Self { + pub fn new(message: Vec, color: Option) -> Self { Self { message, color } } } diff --git a/src/rustfmt_diff.rs b/src/rustfmt_diff.rs index c9883452185..9e117cdcf88 100644 --- a/src/rustfmt_diff.rs +++ b/src/rustfmt_diff.rs @@ -2,8 +2,10 @@ use std::collections::VecDeque; use std::fmt; use std::io; use std::io::Write; +use crate::buf_term_println; use crate::config::{Color, Config, Verbosity}; +use crate::print::Printer; #[derive(Debug, PartialEq)] pub(crate) enum DiffLine { @@ -245,7 +247,7 @@ pub(crate) fn make_diff(expected: &str, actual: &str, context_size: usize) -> Ve results } -pub(crate) fn print_diff(diff: Vec, get_section_title: F, config: &Config) +pub(crate) fn print_diff(diff: Vec, get_section_title: F, config: &Config, printer: &Printer) where F: Fn(u32) -> String, { @@ -265,14 +267,13 @@ where for line in mismatch.lines { match line { DiffLine::Context(ref str) => { - writer.writeln(&format!(" {str}{line_terminator}"), None) + buf_term_println!(printer, None, " {str}{line_terminator}"); } - DiffLine::Expected(ref str) => writer.writeln( - &format!("+{str}{line_terminator}"), - Some(term::color::GREEN), - ), + DiffLine::Expected(ref str) => { + buf_term_println!(printer, Some(rustc_errors::Color::Green), "+{str}{line_terminator}"); + }, DiffLine::Resulting(ref str) => { - writer.writeln(&format!("-{str}{line_terminator}"), Some(term::color::RED)) + buf_term_println!(printer, Some(rustc_errors::Color::Red), "-{str}{line_terminator}"); } } } From 4496f286849d4094cc245823cc07f8ef3a325d92 Mon Sep 17 00:00:00 2001 From: MarcusGrass Date: Sun, 25 Feb 2024 04:02:40 +0100 Subject: [PATCH 06/23] Emitters dont print --- src/emitter.rs | 2 ++ src/emitter/checkstyle.rs | 1 + src/emitter/diff.rs | 2 ++ src/emitter/files.rs | 6 ++++-- src/emitter/files_with_backup.rs | 1 + src/emitter/json.rs | 1 + src/emitter/modified_lines.rs | 1 + src/emitter/stdout.rs | 8 +++++--- src/formatting.rs | 1 + src/source_file.rs | 4 +++- 10 files changed, 21 insertions(+), 6 deletions(-) diff --git a/src/emitter.rs b/src/emitter.rs index 9c335314d75..1536328e2e7 100644 --- a/src/emitter.rs +++ b/src/emitter.rs @@ -8,6 +8,7 @@ pub(crate) use self::stdout::*; use crate::FileName; use std::io::{self, Write}; use std::path::Path; +use crate::print::Printer; mod checkstyle; mod diff; @@ -32,6 +33,7 @@ pub(crate) trait Emitter { fn emit_formatted_file( &mut self, output: &mut dyn Write, + printer: &Printer, formatted_file: FormattedFile<'_>, ) -> Result; diff --git a/src/emitter/checkstyle.rs b/src/emitter/checkstyle.rs index 9385ae59a06..d2b23811ddb 100644 --- a/src/emitter/checkstyle.rs +++ b/src/emitter/checkstyle.rs @@ -21,6 +21,7 @@ impl Emitter for CheckstyleEmitter { fn emit_formatted_file( &mut self, output: &mut dyn Write, + _printer: &Printer, FormattedFile { filename, original_text, diff --git a/src/emitter/diff.rs b/src/emitter/diff.rs index 4e48c59ea29..f4389358586 100644 --- a/src/emitter/diff.rs +++ b/src/emitter/diff.rs @@ -16,6 +16,7 @@ impl Emitter for DiffEmitter { fn emit_formatted_file( &mut self, output: &mut dyn Write, + printer: &Printer, FormattedFile { filename, original_text, @@ -34,6 +35,7 @@ impl Emitter for DiffEmitter { mismatch, |line_num| format!("Diff in {}:{}:", filename, line_num), &self.config, + printer, ); } } else if original_text != formatted_text { diff --git a/src/emitter/files.rs b/src/emitter/files.rs index 6360b73ee61..1bc6d0179d9 100644 --- a/src/emitter/files.rs +++ b/src/emitter/files.rs @@ -1,5 +1,6 @@ use super::*; use std::fs; +use crate::buf_println; #[derive(Debug, Default)] pub(crate) struct FilesEmitter { @@ -17,7 +18,8 @@ impl FilesEmitter { impl Emitter for FilesEmitter { fn emit_formatted_file( &mut self, - output: &mut dyn Write, + _output: &mut dyn Write, + printer: &Printer, FormattedFile { filename, original_text, @@ -29,7 +31,7 @@ impl Emitter for FilesEmitter { if original_text != formatted_text { fs::write(filename, formatted_text)?; if self.print_misformatted_file_names { - writeln!(output, "{}", filename.display())?; + buf_println!(printer, "{}", filename.display()); } } Ok(EmitterResult::default()) diff --git a/src/emitter/files_with_backup.rs b/src/emitter/files_with_backup.rs index 4c15f6fa5ec..4b239c339cd 100644 --- a/src/emitter/files_with_backup.rs +++ b/src/emitter/files_with_backup.rs @@ -8,6 +8,7 @@ impl Emitter for FilesWithBackupEmitter { fn emit_formatted_file( &mut self, _output: &mut dyn Write, + _printer: &Printer, FormattedFile { filename, original_text, diff --git a/src/emitter/json.rs b/src/emitter/json.rs index 084f565804c..957fb411d21 100644 --- a/src/emitter/json.rs +++ b/src/emitter/json.rs @@ -32,6 +32,7 @@ impl Emitter for JsonEmitter { fn emit_formatted_file( &mut self, _output: &mut dyn Write, + _printer: &Printer, FormattedFile { filename, original_text, diff --git a/src/emitter/modified_lines.rs b/src/emitter/modified_lines.rs index 81f0a31b974..f9e300eb087 100644 --- a/src/emitter/modified_lines.rs +++ b/src/emitter/modified_lines.rs @@ -8,6 +8,7 @@ impl Emitter for ModifiedLinesEmitter { fn emit_formatted_file( &mut self, output: &mut dyn Write, + _printer: &Printer, FormattedFile { original_text, formatted_text, diff --git a/src/emitter/stdout.rs b/src/emitter/stdout.rs index 0b635a28bf2..153bd679621 100644 --- a/src/emitter/stdout.rs +++ b/src/emitter/stdout.rs @@ -1,3 +1,4 @@ +use crate::buf_println; use super::*; use crate::config::Verbosity; @@ -15,7 +16,8 @@ impl StdoutEmitter { impl Emitter for StdoutEmitter { fn emit_formatted_file( &mut self, - output: &mut dyn Write, + _output: &mut dyn Write, + printer: &Printer, FormattedFile { filename, formatted_text, @@ -23,9 +25,9 @@ impl Emitter for StdoutEmitter { }: FormattedFile<'_>, ) -> Result { if self.verbosity != Verbosity::Quiet { - writeln!(output, "{filename}:\n")?; + buf_println!(printer, "{filename}:\n"); } - write!(output, "{formatted_text}")?; + buf_println!(printer, "{formatted_text}"); Ok(EmitterResult::default()) } } diff --git a/src/formatting.rs b/src/formatting.rs index 2825d8bc502..33c4df9a6b5 100644 --- a/src/formatting.rs +++ b/src/formatting.rs @@ -298,6 +298,7 @@ impl<'b, T: Write + 'b> FormatHandler for Session<'b, T> { out, &mut *self.emitter, self.config.newline_style(), + self.printer, ) { Ok(ref result) if result.has_diff => report.add_diff(), Err(e) => { diff --git a/src/source_file.rs b/src/source_file.rs index 8f66def3df3..6835f2b6ef1 100644 --- a/src/source_file.rs +++ b/src/source_file.rs @@ -15,6 +15,7 @@ use crate::create_emitter; use crate::formatting::FileRecord; use rustc_data_structures::sync::Lrc; +use crate::print::Printer; // Append a newline to the end of each file. pub(crate) fn append_newline(s: &mut String) { @@ -55,6 +56,7 @@ pub(crate) fn write_file( out: &mut T, emitter: &mut dyn Emitter, newline_style: NewlineStyle, + printer: &Printer, ) -> Result where T: Write, @@ -101,5 +103,5 @@ where formatted_text, }; - emitter.emit_formatted_file(out, formatted_file) + emitter.emit_formatted_file(out, printer, formatted_file) } From ae322eda8162b5579b32d00619cf5894cd809345 Mon Sep 17 00:00:00 2001 From: MarcusGrass Date: Sun, 25 Feb 2024 05:01:36 +0100 Subject: [PATCH 07/23] Buggy works --- Cargo.lock | 4 +- Cargo.toml | 2 +- src/bin/main.rs | 13 +++--- src/parse/session.rs | 9 +--- src/print.rs | 100 +++++++++++++++++++++++++++++++++++++++---- src/rustfmt_diff.rs | 49 ++------------------- 6 files changed, 107 insertions(+), 70 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 66e1840eb86..0aaae84b5eb 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -672,9 +672,9 @@ dependencies = [ [[package]] name = "termcolor" -version = "1.4.1" +version = "1.2.0" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "06794f8f6c5c898b3275aebefa6b8a1cb24cd2c6c79397ab15774837a0bc5755" +checksum = "be55cf8942feac5c765c2c993422806843c9a9a45d4d5c407ad6dd2ea95eb9b6" dependencies = [ "winapi-util", ] diff --git a/Cargo.toml b/Cargo.toml index 8f866de4719..bd11dd22225 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -49,7 +49,7 @@ regex = "1.7" serde = { version = "1.0.160", features = ["derive"] } serde_json = "1.0" term = "0.7" -termcolor = "1.4.1" +termcolor = "1.2.0" thiserror = "1.0.40" toml = "0.7.4" tracing = "0.1.37" diff --git a/src/bin/main.rs b/src/bin/main.rs index 3f60eff35cd..901242479c6 100644 --- a/src/bin/main.rs +++ b/src/bin/main.rs @@ -406,10 +406,10 @@ fn format( outstanding += 1; if outstanding >= num_cpus { if let Ok(thread_out) = recv.recv() { - handles.remove(&id).unwrap().join().unwrap().unwrap(); + handles.remove(&thread_out.id).unwrap().join().unwrap().unwrap(); let output = thread_out.session_result?; - let out = &mut stdout(); - out.write_all(&output).unwrap(); + stdout().write_all(&output).unwrap(); + thread_out.printer.dump()?; if thread_out.exit_code != 0 { exit_code = thread_out.exit_code; } @@ -420,11 +420,12 @@ fn format( } } drop(send); - let out = &mut stdout(); while let Ok(thread_out) = recv.recv() { - handles.remove(&id).unwrap().join().unwrap().unwrap(); + let handle_res = handles.remove(&thread_out.id).unwrap().join(); + handle_res.unwrap().unwrap(); let output = thread_out.session_result?; - out.write_all(&output).unwrap(); + stdout().write_all(&output).unwrap(); + thread_out.printer.dump()?; if thread_out.exit_code != 0 { exit_code = thread_out.exit_code; } diff --git a/src/parse/session.rs b/src/parse/session.rs index 7779c0d2574..d063ad4c567 100644 --- a/src/parse/session.rs +++ b/src/parse/session.rs @@ -124,16 +124,9 @@ fn default_dcx( ignore_path_set: Lrc, can_reset: Lrc, show_parse_errors: bool, - color: Color, + _color: Color, printer: &Printer, ) -> DiagCtxt { - let supports_color = term::stderr().map_or(false, |term| term.supports_color()); - let emit_color = if supports_color { - ColorConfig::from(color) - } else { - ColorConfig::Never - }; - let emitter = if !show_parse_errors { silent_emitter() } else { diff --git a/src/print.rs b/src/print.rs index 3aa7aaf115e..50a0db1a218 100644 --- a/src/print.rs +++ b/src/print.rs @@ -1,5 +1,6 @@ -use std::io::Write; +use std::io::{stderr, stdout, Write}; use std::sync::{Arc, Mutex}; +use termcolor::{ColorChoice, StandardStream, WriteColor as _}; use rustc_errors::{ColorSpec, WriteColor, Color as RustColor}; use crate::Color; @@ -11,8 +12,8 @@ pub struct Printer { impl Write for Printer { fn write(&mut self, buf: &[u8]) -> std::io::Result { let mut inner = self.inner.lock().unwrap(); - let col = inner.current_color; - inner.messages.push(PrintMessage::Term(TermMessage::new(buf.to_vec(), col))); + let col = inner.current_color.clone(); + inner.messages.push(PrintMessage::RustcErrTerm(RustcErrTermMessage::new(buf.to_vec(), col))); Ok(buf.len()) } @@ -30,7 +31,7 @@ impl WriteColor for Printer { #[inline] fn set_color(&mut self, spec: &ColorSpec) -> std::io::Result<()> { - self.inner.lock().unwrap().current_color = spec.fg().copied(); + self.inner.lock().unwrap().current_color = Some(spec.clone()); Ok(()) } @@ -43,7 +44,7 @@ impl WriteColor for Printer { struct PrinterInner { color_setting: Color, - current_color: Option, + current_color: Option, messages: Vec, supports_color: bool, } @@ -65,6 +66,77 @@ impl Printer { pub fn push_msg(&self, msg: PrintMessage) { self.inner.lock().unwrap().messages.push(msg); } + + pub fn dump(&self) -> Result<(), std::io::Error> { + let inner = self.inner.lock().unwrap(); + let mut use_term_stdout = term::stdout().filter(|t| inner.color_setting.use_colored_tty() && t.supports_color()); + let use_rustc_error_color = inner.color_setting.use_colored_tty() && term::stderr().map(|t| t.supports_color()).unwrap_or_default(); + let mut rustc_err_out = use_rustc_error_color.then_some(StandardStream::stderr(ColorChoice::Always)); + for msg in &inner.messages { + match msg { + PrintMessage::Stdout(out) => { + stdout().write_all(out)?; + } + PrintMessage::StdErr(err) => { + stderr().write_all(err)?; + } + PrintMessage::Term(t_msg) => { + if let Some(t) = &mut use_term_stdout { + if let Some(col) = t_msg.color { + t.fg(col).unwrap() + } + t.write_all(&t_msg.message)?; + if t_msg.color.is_some() { + t.reset().unwrap(); + } + } else { + stdout().write_all(&t_msg.message)?; + } + } + PrintMessage::RustcErrTerm(msg) => { + if let Some(t) = &mut rustc_err_out { + if let Some(col) = msg.color.as_ref().map(rustc_colorspec_compat) { + t.set_color(&col)?; + } + t.write_all(&msg.message)?; + if msg.color.is_some() { + t.reset().unwrap(); + } + } else { + stderr().write_all(&msg.message)?; + } + } + } + } + + Ok(()) + } +} + +// Rustc vendors termcolor, but not everything needed to use it, +// as far as I can tell +fn rustc_colorspec_compat(rustc: &ColorSpec) -> termcolor::ColorSpec { + let mut cs = termcolor::ColorSpec::new(); + let col = rustc.fg().and_then(rustc_color_compat); + cs.set_fg(col); + cs +} + +fn rustc_color_compat(rustc: &RustColor) -> Option { + let col = match rustc { + RustColor::Black => termcolor::Color::Black, + RustColor::Blue => termcolor::Color::Blue, + RustColor::Green => termcolor::Color::Green, + RustColor::Red => termcolor::Color::Red, + RustColor::Cyan => termcolor::Color::Cyan, + RustColor::Magenta => termcolor::Color::Magenta, + RustColor::Yellow => termcolor::Color::Yellow, + RustColor::White => termcolor::Color::White, + RustColor::Ansi256(c) => termcolor::Color::Ansi256(*c), + RustColor::Rgb(r, g, b) => termcolor::Color::Rgb(*r, *g, *b), + _ => return None, + }; + Some(col) } #[macro_export] @@ -97,16 +169,28 @@ macro_rules! buf_term_println { pub enum PrintMessage { Stdout(Vec), StdErr(Vec), - Term(TermMessage) + Term(TermMessage), + RustcErrTerm(RustcErrTermMessage) } pub struct TermMessage { message: Vec, - color: Option, + color: Option, } impl TermMessage { - pub fn new(message: Vec, color: Option) -> Self { + pub fn new(message: Vec, color: Option) -> Self { + Self { message, color } + } +} + +pub struct RustcErrTermMessage { + message: Vec, + color: Option, +} + +impl RustcErrTermMessage { + pub fn new(message: Vec, color: Option) -> Self { Self { message, color } } } diff --git a/src/rustfmt_diff.rs b/src/rustfmt_diff.rs index 9e117cdcf88..509b6d0b284 100644 --- a/src/rustfmt_diff.rs +++ b/src/rustfmt_diff.rs @@ -1,10 +1,9 @@ use std::collections::VecDeque; use std::fmt; -use std::io; use std::io::Write; use crate::buf_term_println; -use crate::config::{Color, Config, Verbosity}; +use crate::config::{Config, Verbosity}; use crate::print::Printer; #[derive(Debug, PartialEq)] @@ -141,43 +140,6 @@ impl std::str::FromStr for ModifiedLines { } } -// This struct handles writing output to stdout and abstracts away the logic -// of printing in color, if it's possible in the executing environment. -pub(crate) struct OutputWriter { - terminal: Option>>, -} - -impl OutputWriter { - // Create a new OutputWriter instance based on the caller's preference - // for colorized output and the capabilities of the terminal. - pub(crate) fn new(color: Color) -> Self { - if let Some(t) = term::stdout() { - if color.use_colored_tty() && t.supports_color() { - return OutputWriter { terminal: Some(t) }; - } - } - OutputWriter { terminal: None } - } - - // Write output in the optionally specified color. The output is written - // in the specified color if this OutputWriter instance contains a - // Terminal in its `terminal` field. - pub(crate) fn writeln(&mut self, msg: &str, color: Option) { - match &mut self.terminal { - Some(ref mut t) => { - if let Some(color) = color { - t.fg(color).unwrap(); - } - writeln!(t, "{msg}").unwrap(); - if color.is_some() { - t.reset().unwrap(); - } - } - None => println!("{msg}"), - } - } -} - // Produces a diff between the expected output and actual output of rustfmt. pub(crate) fn make_diff(expected: &str, actual: &str, context_size: usize) -> Vec { let mut line_number = 1; @@ -251,18 +213,15 @@ pub(crate) fn print_diff(diff: Vec, get_section_title: F, config: & where F: Fn(u32) -> String, { - let color = config.color(); let line_terminator = if config.verbose() == Verbosity::Verbose { "⏎" } else { "" }; - let mut writer = OutputWriter::new(color); - for mismatch in diff { let title = get_section_title(mismatch.line_number_orig); - writer.writeln(&title, None); + buf_term_println!(printer, None, "{title}"); for line in mismatch.lines { match line { @@ -270,10 +229,10 @@ where buf_term_println!(printer, None, " {str}{line_terminator}"); } DiffLine::Expected(ref str) => { - buf_term_println!(printer, Some(rustc_errors::Color::Green), "+{str}{line_terminator}"); + buf_term_println!(printer, Some(term::color::GREEN), "+{str}{line_terminator}"); }, DiffLine::Resulting(ref str) => { - buf_term_println!(printer, Some(rustc_errors::Color::Red), "-{str}{line_terminator}"); + buf_term_println!(printer, Some(term::color::RED), "-{str}{line_terminator}"); } } } From adef47a140f62abc0760f455b2d4973bf80182b6 Mon Sep 17 00:00:00 2001 From: MarcusGrass Date: Sun, 25 Feb 2024 05:34:14 +0100 Subject: [PATCH 08/23] Fixup most tests --- src/comment.rs | 18 ++++++------- src/emitter/checkstyle.rs | 2 ++ src/emitter/diff.rs | 4 +++ src/emitter/json.rs | 4 +++ src/emitter/stdout.rs | 15 +++++------ src/lib.rs | 16 ++++++----- src/print.rs | 4 +++ src/source_file.rs | 1 + src/test/configuration_snippet.rs | 4 ++- src/test/mod.rs | 44 ++++++++++++++++++++----------- src/test/mod_resolver.rs | 4 ++- src/test/parser.rs | 7 +++-- 12 files changed, 79 insertions(+), 44 deletions(-) diff --git a/src/comment.rs b/src/comment.rs index c66480727ac..d5eb8ca36d6 100644 --- a/src/comment.rs +++ b/src/comment.rs @@ -1921,19 +1921,19 @@ mod test { let comment = rewrite_comment(" //test", true, Shape::legacy(100, Indent::new(0, 100)), - &wrap_normalize_config).unwrap(); + &wrap_normalize_config, &Printer::no_color()).unwrap(); assert_eq!("/* test */", comment); let comment = rewrite_comment("// comment on a", false, Shape::legacy(10, Indent::empty()), - &wrap_normalize_config).unwrap(); + &wrap_normalize_config, &Printer::no_color()).unwrap(); assert_eq!("// comment\n// on a", comment); let comment = rewrite_comment("// A multi line comment\n // between args.", false, Shape::legacy(60, Indent::new(0, 12)), - &wrap_normalize_config).unwrap(); + &wrap_normalize_config, &Printer::no_color()).unwrap(); assert_eq!("// A multi line comment\n // between args.", comment); let input = "// comment"; @@ -1942,13 +1942,13 @@ mod test { let comment = rewrite_comment(input, true, Shape::legacy(9, Indent::new(0, 69)), - &wrap_normalize_config).unwrap(); + &wrap_normalize_config, &Printer::no_color()).unwrap(); assert_eq!(expected, comment); let comment = rewrite_comment("/* trimmed */", true, Shape::legacy(100, Indent::new(0, 100)), - &wrap_normalize_config).unwrap(); + &wrap_normalize_config, &Printer::no_color()).unwrap(); assert_eq!("/* trimmed */", comment); // Check that different comment style are properly recognised. @@ -1959,7 +1959,7 @@ mod test { */"#, false, Shape::legacy(100, Indent::new(0, 0)), - &wrap_normalize_config).unwrap(); + &wrap_normalize_config, &Printer::no_color()).unwrap(); assert_eq!("/// test1\n/// test2\n// test3", comment); // Check that the blank line marks the end of a commented paragraph. @@ -1968,7 +1968,7 @@ mod test { // test2"#, false, Shape::legacy(100, Indent::new(0, 0)), - &wrap_normalize_config).unwrap(); + &wrap_normalize_config, &Printer::no_color()).unwrap(); assert_eq!("// test1\n\n// test2", comment); // Check that the blank line marks the end of a custom-commented paragraph. @@ -1977,7 +1977,7 @@ mod test { //@ test2"#, false, Shape::legacy(100, Indent::new(0, 0)), - &wrap_normalize_config).unwrap(); + &wrap_normalize_config, &Printer::no_color()).unwrap(); assert_eq!("//@ test1\n\n//@ test2", comment); // Check that bare lines are just indented but otherwise left unchanged. @@ -1989,7 +1989,7 @@ mod test { */"#, false, Shape::legacy(100, Indent::new(0, 0)), - &wrap_config).unwrap(); + &wrap_config, &Printer::no_color()).unwrap(); assert_eq!("// test1\n/*\n a bare line!\n\n another bare line!\n*/", comment); } diff --git a/src/emitter/checkstyle.rs b/src/emitter/checkstyle.rs index d2b23811ddb..c33b81ebb5e 100644 --- a/src/emitter/checkstyle.rs +++ b/src/emitter/checkstyle.rs @@ -101,6 +101,7 @@ mod tests { let _ = emitter .emit_formatted_file( &mut writer, + &Printer::no_color(), FormattedFile { filename: &FileName::Real(PathBuf::from(bin_file)), original_text: &bin_original.join("\n"), @@ -111,6 +112,7 @@ mod tests { let _ = emitter .emit_formatted_file( &mut writer, + &Printer::no_color(), FormattedFile { filename: &FileName::Real(PathBuf::from(lib_file)), original_text: &lib_original.join("\n"), diff --git a/src/emitter/diff.rs b/src/emitter/diff.rs index f4389358586..58060f24225 100644 --- a/src/emitter/diff.rs +++ b/src/emitter/diff.rs @@ -63,6 +63,7 @@ mod tests { let result = emitter .emit_formatted_file( &mut writer, + &Printer::no_color(), FormattedFile { filename: &FileName::Real(PathBuf::from("src/lib.rs")), original_text: "fn empty() {}\n", @@ -90,6 +91,7 @@ mod tests { let _ = emitter .emit_formatted_file( &mut writer, + &Printer::no_color(), FormattedFile { filename: &FileName::Real(PathBuf::from(bin_file)), original_text: bin_original, @@ -100,6 +102,7 @@ mod tests { let _ = emitter .emit_formatted_file( &mut writer, + &Printer::no_color(), FormattedFile { filename: &FileName::Real(PathBuf::from(lib_file)), original_text: lib_original, @@ -122,6 +125,7 @@ mod tests { let _ = emitter .emit_formatted_file( &mut writer, + &Printer::no_color(), FormattedFile { filename: &FileName::Real(PathBuf::from("src/lib.rs")), original_text: "fn empty() {}\n", diff --git a/src/emitter/json.rs b/src/emitter/json.rs index 957fb411d21..5acbbbca474 100644 --- a/src/emitter/json.rs +++ b/src/emitter/json.rs @@ -198,6 +198,7 @@ mod tests { let result = emitter .emit_formatted_file( &mut writer, + &Printer::no_color(), FormattedFile { filename: &FileName::Real(PathBuf::from("src/lib.rs")), original_text: "fn empty() {}\n", @@ -245,6 +246,7 @@ mod tests { let result = emitter .emit_formatted_file( &mut writer, + &Printer::no_color(), FormattedFile { filename: &FileName::Real(PathBuf::from(file_name)), original_text: &original.join("\n"), @@ -297,6 +299,7 @@ mod tests { let _ = emitter .emit_formatted_file( &mut writer, + &Printer::no_color(), FormattedFile { filename: &FileName::Real(PathBuf::from(bin_file)), original_text: &bin_original.join("\n"), @@ -307,6 +310,7 @@ mod tests { let _ = emitter .emit_formatted_file( &mut writer, + &Printer::no_color(), FormattedFile { filename: &FileName::Real(PathBuf::from(lib_file)), original_text: &lib_original.join("\n"), diff --git a/src/emitter/stdout.rs b/src/emitter/stdout.rs index 153bd679621..6edaa62cfb8 100644 --- a/src/emitter/stdout.rs +++ b/src/emitter/stdout.rs @@ -1,23 +1,22 @@ -use crate::buf_println; use super::*; use crate::config::Verbosity; #[derive(Debug)] -pub(crate) struct StdoutEmitter { +pub(crate) struct IntoOutputEmitter { verbosity: Verbosity, } -impl StdoutEmitter { +impl IntoOutputEmitter { pub(crate) fn new(verbosity: Verbosity) -> Self { Self { verbosity } } } -impl Emitter for StdoutEmitter { +impl Emitter for IntoOutputEmitter { fn emit_formatted_file( &mut self, - _output: &mut dyn Write, - printer: &Printer, + output: &mut dyn Write, + _printer: &Printer, FormattedFile { filename, formatted_text, @@ -25,9 +24,9 @@ impl Emitter for StdoutEmitter { }: FormattedFile<'_>, ) -> Result { if self.verbosity != Verbosity::Quiet { - buf_println!(printer, "{filename}:\n"); + writeln!(output, "{filename}:\n")?; } - buf_println!(printer, "{formatted_text}"); + write!(output, "{formatted_text}")?; Ok(EmitterResult::default()) } } diff --git a/src/lib.rs b/src/lib.rs index 7d7eac64d50..86f40917154 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -527,12 +527,14 @@ pub(crate) fn create_emitter<'a>(config: &Config) -> Box { config.print_misformatted_file_names(), )), EmitMode::Stdout | EmitMode::Coverage => { - Box::new(emitter::StdoutEmitter::new(config.verbose())) + Box::new(emitter::IntoOutputEmitter::new(config.verbose())) } EmitMode::Json => Box::new(emitter::JsonEmitter::default()), EmitMode::ModifiedLines => Box::new(emitter::ModifiedLinesEmitter::default()), EmitMode::Checkstyle => Box::new(emitter::CheckstyleEmitter::default()), - EmitMode::Diff => Box::new(emitter::DiffEmitter::new(config.clone())), + EmitMode::Diff => { + Box::new(emitter::DiffEmitter::new(config.clone())) + }, } } @@ -586,15 +588,15 @@ mod unit_tests { // `format_snippet()` and `format_code_block()` should not panic // even when we cannot parse the given snippet. let snippet = "let"; - assert!(format_snippet(snippet, &Config::default(), false).is_none()); - assert!(format_code_block(snippet, &Config::default(), false).is_none()); + assert!(format_snippet(snippet, &Config::default(), false, &Printer::no_color()).is_none()); + assert!(format_code_block(snippet, &Config::default(), false, &Printer::no_color()).is_none()); } fn test_format_inner(formatter: F, input: &str, expected: &str) -> bool where - F: Fn(&str, &Config, bool) -> Option, + F: Fn(&str, &Config, bool, &Printer) -> Option, { - let output = formatter(input, &Config::default(), false); + let output = formatter(input, &Config::default(), false, &Printer::no_color()); output.is_some() && output.unwrap().snippet == expected } @@ -616,7 +618,7 @@ mod unit_tests { fn test_format_code_block_fail() { #[rustfmt::skip] let code_block = "this_line_is_100_characters_long_xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx(x, y, z);"; - assert!(format_code_block(code_block, &Config::default(), false).is_none()); + assert!(format_code_block(code_block, &Config::default(), false, &Printer::no_color()).is_none()); } #[test] diff --git a/src/print.rs b/src/print.rs index 50a0db1a218..f8d981194a8 100644 --- a/src/print.rs +++ b/src/print.rs @@ -62,6 +62,10 @@ impl Printer { } } + pub fn no_color() -> Self { + Self::new(Color::Never) + } + #[inline] pub fn push_msg(&self, msg: PrintMessage) { self.inner.lock().unwrap().messages.push(msg); diff --git a/src/source_file.rs b/src/source_file.rs index 6835f2b6ef1..589cb39a100 100644 --- a/src/source_file.rs +++ b/src/source_file.rs @@ -42,6 +42,7 @@ where out, &mut *emitter, config.newline_style(), + &Printer::no_color() )?; } emitter.emit_footer(out)?; diff --git a/src/test/configuration_snippet.rs b/src/test/configuration_snippet.rs index 857fff5cf6b..0a0e03f7332 100644 --- a/src/test/configuration_snippet.rs +++ b/src/test/configuration_snippet.rs @@ -8,6 +8,7 @@ use super::{print_mismatches, write_message, DIFF_CONTEXT_SIZE}; use crate::config::{Config, EmitMode, Verbosity}; use crate::rustfmt_diff::{make_diff, Mismatch}; use crate::{Input, Session}; +use crate::print::Printer; const CONFIGURATIONS_FILE_NAME: &str = "Configurations.md"; @@ -197,7 +198,8 @@ impl ConfigCodeBlock { let mut buf: Vec = vec![]; { - let mut session = Session::new(config, Some(&mut buf)); + let printer = Printer::no_color(); + let mut session = Session::new(config, Some(&mut buf), &printer); session.format(input).unwrap(); if self.has_parsing_errors(&session) { return false; diff --git a/src/test/mod.rs b/src/test/mod.rs index b2b3df8f4ad..d575e29dd52 100644 --- a/src/test/mod.rs +++ b/src/test/mod.rs @@ -11,11 +11,12 @@ use std::thread; use crate::config::{Color, Config, EmitMode, FileName, NewlineStyle}; use crate::formatting::{ReportedErrors, SourceFile}; -use crate::rustfmt_diff::{make_diff, print_diff, DiffLine, Mismatch, ModifiedChunk, OutputWriter}; -use crate::source_file; +use crate::rustfmt_diff::{make_diff, print_diff, DiffLine, Mismatch, ModifiedChunk}; +use crate::{buf_term_println, source_file}; use crate::{is_nightly_channel, FormatReport, FormatReportFormatterBuilder, Input, Session}; use rustfmt_config_proc_macro::nightly_only_test; +use crate::print::Printer; mod configuration_snippet; mod mod_resolver; @@ -168,8 +169,9 @@ fn verify_config_test_names() { // using only one or the other will cause the output order to differ when // `print_diff` selects the approach not used. fn write_message(msg: &str) { - let mut writer = OutputWriter::new(Color::Auto); - writer.writeln(msg, None); + let print = Printer::new(Color::Auto); + buf_term_println!(print, None, "{msg}"); + print.dump().unwrap(); } // Integration tests. The files in `tests/source` are formatted and compared @@ -237,7 +239,8 @@ fn modified_test() { .emit_mode(crate::config::EmitMode::ModifiedLines); { - let mut session = Session::new(config, Some(&mut data)); + let printer = Printer::no_color(); + let mut session = Session::new(config, Some(&mut data), &printer); session.format(Input::File(filename.into())).unwrap(); } @@ -329,7 +332,8 @@ fn assert_stdin_output( // Populate output by writing to a vec. let mut buf: Vec = vec![]; { - let mut session = Session::new(config, Some(&mut buf)); + let printer = Printer::no_color(); + let mut session = Session::new(config, Some(&mut buf), &printer); session.format(input).unwrap(); let errors = ReportedErrors { has_diff, @@ -422,7 +426,8 @@ fn format_files_find_new_files_via_cfg_if() { ]; let config = Config::default(); - let mut session = Session::::new(config, None); + let printer = Printer::no_color(); + let mut session = Session::::new(config, None, &printer); let mut write_result = HashMap::new(); for file in files { @@ -456,7 +461,8 @@ fn stdin_formatting_smoke_test() { config.set().emit_mode(EmitMode::Stdout); let mut buf: Vec = vec![]; { - let mut session = Session::new(config, Some(&mut buf)); + let printer = Printer::no_color(); + let mut session = Session::new(config, Some(&mut buf), &printer); session.format(input).unwrap(); assert!(session.has_no_errors()); } @@ -473,7 +479,8 @@ fn stdin_parser_panic_caught() { // See issue #3239. for text in ["{", "}"].iter().cloned().map(String::from) { let mut buf = vec![]; - let mut session = Session::new(Default::default(), Some(&mut buf)); + let printer = Printer::no_color(); + let mut session = Session::new(Default::default(), Some(&mut buf), &printer); let _ = session.format(Input::Text(text)); assert!(session.has_parsing_errors()); @@ -494,7 +501,8 @@ fn stdin_works_with_modified_lines() { config.set().emit_mode(EmitMode::ModifiedLines); let mut buf: Vec = vec![]; { - let mut session = Session::new(config, Some(&mut buf)); + let printer = Printer::no_color(); + let mut session = Session::new(config, Some(&mut buf), &printer); session.format(input).unwrap(); let errors = ReportedErrors { has_diff: true, @@ -563,7 +571,8 @@ fn stdin_generated_files_issue_5172() { config.set().newline_style(NewlineStyle::Unix); let mut buf: Vec = vec![]; { - let mut session = Session::new(config, Some(&mut buf)); + let printer = Printer::no_color(); + let mut session = Session::new(config, Some(&mut buf), &printer); session.format(input).unwrap(); assert!(session.has_no_errors()); } @@ -605,7 +614,8 @@ fn format_lines_errors_are_reported() { let input = Input::Text(format!("fn {long_identifier}() {{}}")); let mut config = Config::default(); config.set().error_on_line_overflow(true); - let mut session = Session::::new(config, None); + let printer = Printer::no_color(); + let mut session = Session::::new(config, None, &printer); session.format(input).unwrap(); assert!(session.has_formatting_errors()); } @@ -618,7 +628,8 @@ fn format_lines_errors_are_reported_with_tabs() { let mut config = Config::default(); config.set().error_on_line_overflow(true); config.set().hard_tabs(true); - let mut session = Session::::new(config, None); + let printer = Printer::no_color(); + let mut session = Session::::new(config, None, &printer); session.format(input).unwrap(); assert!(session.has_formatting_errors()); } @@ -667,7 +678,7 @@ fn print_mismatches_default_message(result: HashMap>) { for (file_name, diff) in result { let mismatch_msg_formatter = |line_num| format!("\nMismatch at {}:{}:", file_name.display(), line_num); - print_diff(diff, &mismatch_msg_formatter, &Default::default()); + print_diff(diff, &mismatch_msg_formatter, &Default::default(), &Printer::no_color()); } if let Some(mut t) = term::stdout() { @@ -680,7 +691,7 @@ fn print_mismatches String>( mismatch_msg_formatter: T, ) { for (_file_name, diff) in result { - print_diff(diff, &mismatch_msg_formatter, &Default::default()); + print_diff(diff, &mismatch_msg_formatter, &Default::default(), &Printer::no_color()); } if let Some(mut t) = term::stdout() { @@ -714,7 +725,8 @@ fn read_config(filename: &Path) -> Config { fn format_file>(filepath: P, config: Config) -> (bool, SourceFile, FormatReport) { let filepath = filepath.into(); let input = Input::File(filepath); - let mut session = Session::::new(config, None); + let printer = Printer::no_color(); + let mut session = Session::::new(config, None, &printer); let result = session.format(input).unwrap(); let parsing_errors = session.has_parsing_errors(); let mut source_file = SourceFile::new(); diff --git a/src/test/mod_resolver.rs b/src/test/mod_resolver.rs index aacb2acc684..27d7431158e 100644 --- a/src/test/mod_resolver.rs +++ b/src/test/mod_resolver.rs @@ -4,11 +4,13 @@ use std::path::PathBuf; use super::read_config; use crate::{FileName, Input, Session}; +use crate::print::Printer; fn verify_mod_resolution(input_file_name: &str, exp_misformatted_files: &[&str]) { let input_file = PathBuf::from(input_file_name); let config = read_config(&input_file); - let mut session = Session::::new(config, None); + let printer = Printer::no_color(); + let mut session = Session::::new(config, None, &printer); let report = session .format(Input::File(input_file_name.into())) .expect("Should not have had any execution errors"); diff --git a/src/test/parser.rs b/src/test/parser.rs index ae4a4f94d92..e2c6b23703b 100644 --- a/src/test/parser.rs +++ b/src/test/parser.rs @@ -5,6 +5,7 @@ use super::read_config; use crate::modules::{ModuleResolutionError, ModuleResolutionErrorKind}; use crate::{ErrorKind, Input, Session}; +use crate::print::Printer; #[test] fn parser_errors_in_submods_are_surfaced() { @@ -13,7 +14,8 @@ fn parser_errors_in_submods_are_surfaced() { let input_file = PathBuf::from(filename); let exp_mod_name = "invalid"; let config = read_config(&input_file); - let mut session = Session::::new(config, None); + let printer = Printer::no_color(); + let mut session = Session::::new(config, None, &printer); if let Err(ErrorKind::ModuleResolutionError(ModuleResolutionError { module, kind })) = session.format(Input::File(filename.into())) { @@ -37,7 +39,8 @@ fn parser_errors_in_submods_are_surfaced() { fn assert_parser_error(filename: &str) { let file = PathBuf::from(filename); let config = read_config(&file); - let mut session = Session::::new(config, None); + let printer = Printer::no_color(); + let mut session = Session::::new(config, None, &printer); let _ = session.format(Input::File(filename.into())).unwrap(); assert!(session.has_parsing_errors()); } From 606d89e6b55feee3dc057c9ca754fe4bc4f1379a Mon Sep 17 00:00:00 2001 From: MarcusGrass Date: Sun, 25 Feb 2024 05:35:48 +0100 Subject: [PATCH 09/23] Formatted passes tests --- config_proc_macro/src/attrs.rs | 6 +++++- src/attr.rs | 7 ++++++- src/bin/main.rs | 35 ++++++++++++++++++++++++------- src/comment.rs | 35 ++++++++++++++++++++++++------- src/emitter.rs | 2 +- src/emitter/files.rs | 2 +- src/formatting.rs | 4 ++-- src/git-rustfmt/main.rs | 2 +- src/lib.rs | 22 ++++++++++++------- src/lists.rs | 15 ++++++++++--- src/macros.rs | 21 ++++++++++--------- src/missed_spans.rs | 16 +++++++++----- src/parse/session.rs | 8 ++++--- src/print.rs | 29 +++++++++++++++++-------- src/rewrite.rs | 2 +- src/rustfmt_diff.rs | 12 +++++++---- src/source_file.rs | 4 ++-- src/test/configuration_snippet.rs | 2 +- src/test/mod.rs | 16 +++++++++++--- src/test/mod_resolver.rs | 2 +- src/test/parser.rs | 2 +- src/visitor.rs | 14 +++++++++---- tests/rustfmt/main.rs | 6 +++++- 23 files changed, 187 insertions(+), 77 deletions(-) diff --git a/config_proc_macro/src/attrs.rs b/config_proc_macro/src/attrs.rs index d8de9aae088..e7534b813d7 100644 --- a/config_proc_macro/src/attrs.rs +++ b/config_proc_macro/src/attrs.rs @@ -68,7 +68,11 @@ fn get_name_value_str_lit(attr: &syn::Attribute, name: &str) -> Option { match &attr.meta { syn::Meta::NameValue(syn::MetaNameValue { path, - value: syn::Expr::Lit(syn::ExprLit { lit: syn::Lit::Str(lit_str), .. }), + value: + syn::Expr::Lit(syn::ExprLit { + lit: syn::Lit::Str(lit_str), + .. + }), .. }) if path.is_ident(name) => Some(lit_str.value()), _ => None, diff --git a/src/attr.rs b/src/attr.rs index 3fa5d97b2ed..ebe32c97de6 100644 --- a/src/attr.rs +++ b/src/attr.rs @@ -319,7 +319,12 @@ impl Rewrite for ast::Attribute { fn rewrite(&self, context: &RewriteContext<'_>, shape: Shape) -> Option { let snippet = context.snippet(self.span); if self.is_doc_comment() { - rewrite_doc_comment(snippet, shape.comment(context.config), context.config, context.printer) + rewrite_doc_comment( + snippet, + shape.comment(context.config), + context.config, + context.printer, + ) } else { let should_skip = self .ident() diff --git a/src/bin/main.rs b/src/bin/main.rs index 901242479c6..25adf7cc9e3 100644 --- a/src/bin/main.rs +++ b/src/bin/main.rs @@ -17,8 +17,8 @@ use std::str::FromStr; use std::thread::JoinHandle; use getopts::{Matches, Options}; -use rustfmt_nightly::{buf_eprintln, buf_println}; use rustfmt_nightly::print::Printer; +use rustfmt_nightly::{buf_eprintln, buf_println}; use crate::rustfmt::{ load_config, CliOptions, Color, Config, Edition, EmitMode, FileLines, FileName, @@ -347,10 +347,18 @@ fn format( let printer = Printer::new(color); let mut session = Session::new(cfg, Some(&mut session_out), &printer); if !file.exists() { - buf_eprintln!(printer, "Error: file `{}` does not exist", file.to_str().unwrap()); + buf_eprintln!( + printer, + "Error: file `{}` does not exist", + file.to_str().unwrap() + ); session.add_operational_error(); } else if file.is_dir() { - buf_eprintln!(printer, "Error: `{}` is a directory", file.to_str().unwrap()); + buf_eprintln!( + printer, + "Error: `{}` is a directory", + file.to_str().unwrap() + ); session.add_operational_error(); } else { // Check the file directory if the config-path could not be read or not provided @@ -371,9 +379,12 @@ fn format( }; if local_config.verbose() == Verbosity::Verbose { if let Some(path) = config_path { - buf_println!(printer, "Using rustfmt config file {} for {}", + buf_println!( + printer, + "Using rustfmt config file {} for {}", path.display(), - file.display()); + file.display() + ); } } @@ -406,7 +417,12 @@ fn format( outstanding += 1; if outstanding >= num_cpus { if let Ok(thread_out) = recv.recv() { - handles.remove(&thread_out.id).unwrap().join().unwrap().unwrap(); + handles + .remove(&thread_out.id) + .unwrap() + .join() + .unwrap() + .unwrap(); let output = thread_out.session_result?; stdout().write_all(&output).unwrap(); thread_out.printer.dump()?; @@ -450,10 +466,13 @@ fn format_and_emit_report(session: &mut Session<'_, T>, input: Input) match session.format(input) { Ok(report) => { if report.has_warnings() { - buf_eprintln!(session.printer, "{}", + buf_eprintln!( + session.printer, + "{}", FormatReportFormatterBuilder::new(&report) .enable_colors(should_print_with_colors(session)) - .build()); + .build() + ); } } Err(msg) => { diff --git a/src/comment.rs b/src/comment.rs index d5eb8ca36d6..bed45cc0feb 100644 --- a/src/comment.rs +++ b/src/comment.rs @@ -8,6 +8,7 @@ use regex::Regex; use rustc_span::Span; use crate::config::Config; +use crate::print::Printer; use crate::rewrite::RewriteContext; use crate::shape::{Indent, Shape}; use crate::string::{rewrite_string, StringFormat}; @@ -16,7 +17,6 @@ use crate::utils::{ trimmed_last_line_width, unicode_str_width, }; use crate::{ErrorKind, FormattingError}; -use crate::print::Printer; lazy_static! { /// A regex matching reference doc links. @@ -249,7 +249,12 @@ pub(crate) fn combine_strs_with_missing_comments( Some(result) } -pub(crate) fn rewrite_doc_comment(orig: &str, shape: Shape, config: &Config, printer: &Printer) -> Option { +pub(crate) fn rewrite_doc_comment( + orig: &str, + shape: Shape, + config: &Config, + printer: &Printer, +) -> Option { identify_comment(orig, false, shape, config, true, printer) } @@ -781,9 +786,12 @@ impl<'a> CommentRewrite<'a> { .doc_comment_code_block_width() .min(config.max_width()); config.set().max_width(comment_max_width); - if let Some(s) = - crate::format_code_block(&self.code_block_buffer, &config, false, printer) - { + if let Some(s) = crate::format_code_block( + &self.code_block_buffer, + &config, + false, + printer, + ) { trim_custom_comment_prefix(&s.snippet) } else { trim_custom_comment_prefix(&self.code_block_buffer) @@ -948,7 +956,14 @@ fn rewrite_comment_inner( }); for (i, (line, has_leading_whitespace)) in lines.enumerate() { - if rewriter.handle_line(orig, i, line, has_leading_whitespace, is_doc_comment, printer) { + if rewriter.handle_line( + orig, + i, + line, + has_leading_whitespace, + is_doc_comment, + printer, + ) { break; } } @@ -1015,7 +1030,13 @@ pub(crate) fn rewrite_missing_comment( // check the span starts with a comment let pos = trimmed_snippet.find('/'); if !trimmed_snippet.is_empty() && pos.is_some() { - rewrite_comment(trimmed_snippet, false, shape, context.config, context.printer) + rewrite_comment( + trimmed_snippet, + false, + shape, + context.config, + context.printer, + ) } else { Some(String::new()) } diff --git a/src/emitter.rs b/src/emitter.rs index 1536328e2e7..8caeb209840 100644 --- a/src/emitter.rs +++ b/src/emitter.rs @@ -5,10 +5,10 @@ pub(crate) use self::files_with_backup::*; pub(crate) use self::json::*; pub(crate) use self::modified_lines::*; pub(crate) use self::stdout::*; +use crate::print::Printer; use crate::FileName; use std::io::{self, Write}; use std::path::Path; -use crate::print::Printer; mod checkstyle; mod diff; diff --git a/src/emitter/files.rs b/src/emitter/files.rs index 1bc6d0179d9..1c2ed146766 100644 --- a/src/emitter/files.rs +++ b/src/emitter/files.rs @@ -1,6 +1,6 @@ use super::*; -use std::fs; use crate::buf_println; +use std::fs; #[derive(Debug, Default)] pub(crate) struct FilesEmitter { diff --git a/src/formatting.rs b/src/formatting.rs index 33c4df9a6b5..675ca9f3d6e 100644 --- a/src/formatting.rs +++ b/src/formatting.rs @@ -14,10 +14,10 @@ use crate::formatting::generated::is_generated_file; use crate::modules::Module; use crate::parse::parser::{DirectoryOwnership, Parser, ParserError}; use crate::parse::session::ParseSess; +use crate::print::Printer; use crate::utils::{contains_skip, count_newlines}; use crate::visitor::FmtVisitor; -use crate::{modules, source_file, ErrorKind, FormatReport, Input, Session, buf_eprintln}; -use crate::print::Printer; +use crate::{buf_eprintln, modules, source_file, ErrorKind, FormatReport, Input, Session}; mod generated; mod newline_style; diff --git a/src/git-rustfmt/main.rs b/src/git-rustfmt/main.rs index e6f662e83ab..f328caff448 100644 --- a/src/git-rustfmt/main.rs +++ b/src/git-rustfmt/main.rs @@ -9,8 +9,8 @@ use std::str::FromStr; use getopts::{Matches, Options}; use rustfmt_nightly as rustfmt; -use tracing_subscriber::EnvFilter; use rustfmt_nightly::print::Printer; +use tracing_subscriber::EnvFilter; use crate::rustfmt::{load_config, CliOptions, FormatReportFormatterBuilder, Input, Session}; diff --git a/src/lib.rs b/src/lib.rs index 86f40917154..5021a311e1f 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -84,6 +84,7 @@ mod overflow; mod pairs; mod parse; mod patterns; +pub mod print; mod release_channel; mod reorder; mod rewrite; @@ -100,7 +101,6 @@ mod test; mod types; mod vertical; pub(crate) mod visitor; -pub mod print; /// The various errors that can occur during formatting. Note that not all of /// these can currently be propagated to clients. @@ -300,7 +300,12 @@ impl fmt::Display for FormatReport { /// Format the given snippet. The snippet is expected to be *complete* code. /// When we cannot parse the given snippet, this function returns `None`. -fn format_snippet(snippet: &str, config: &Config, is_macro_def: bool, printer: &Printer) -> Option { +fn format_snippet( + snippet: &str, + config: &Config, + is_macro_def: bool, + printer: &Printer, +) -> Option { let mut config = config.clone(); panic::catch_unwind(|| { let mut out: Vec = Vec::with_capacity(snippet.len() * 2); @@ -532,9 +537,7 @@ pub(crate) fn create_emitter<'a>(config: &Config) -> Box { EmitMode::Json => Box::new(emitter::JsonEmitter::default()), EmitMode::ModifiedLines => Box::new(emitter::ModifiedLinesEmitter::default()), EmitMode::Checkstyle => Box::new(emitter::CheckstyleEmitter::default()), - EmitMode::Diff => { - Box::new(emitter::DiffEmitter::new(config.clone())) - }, + EmitMode::Diff => Box::new(emitter::DiffEmitter::new(config.clone())), } } @@ -589,7 +592,9 @@ mod unit_tests { // even when we cannot parse the given snippet. let snippet = "let"; assert!(format_snippet(snippet, &Config::default(), false, &Printer::no_color()).is_none()); - assert!(format_code_block(snippet, &Config::default(), false, &Printer::no_color()).is_none()); + assert!( + format_code_block(snippet, &Config::default(), false, &Printer::no_color()).is_none() + ); } fn test_format_inner(formatter: F, input: &str, expected: &str) -> bool @@ -618,7 +623,10 @@ mod unit_tests { fn test_format_code_block_fail() { #[rustfmt::skip] let code_block = "this_line_is_100_characters_long_xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx(x, y, z);"; - assert!(format_code_block(code_block, &Config::default(), false, &Printer::no_color()).is_none()); + assert!( + format_code_block(code_block, &Config::default(), false, &Printer::no_color()) + .is_none() + ); } #[test] diff --git a/src/lists.rs b/src/lists.rs index 9afff476fa7..a5d40b4cac9 100644 --- a/src/lists.rs +++ b/src/lists.rs @@ -258,7 +258,11 @@ where } // Format a list of commented items into a string. -pub(crate) fn write_list(items: I, formatting: &ListFormatting<'_>, printer: &Printer) -> Option +pub(crate) fn write_list( + items: I, + formatting: &ListFormatting<'_>, + printer: &Printer, +) -> Option where I: IntoIterator + Clone, T: AsRef, @@ -363,8 +367,13 @@ where // Block style in non-vertical mode. let block_mode = tactic == DefinitiveListTactic::Horizontal; // Width restriction is only relevant in vertical mode. - let comment = - rewrite_comment(comment, block_mode, formatting.shape, formatting.config, printer)?; + let comment = rewrite_comment( + comment, + block_mode, + formatting.shape, + formatting.config, + printer, + )?; result.push_str(&comment); if !inner_item.is_empty() { diff --git a/src/macros.rs b/src/macros.rs index a4ca34890db..290778ce037 100644 --- a/src/macros.rs +++ b/src/macros.rs @@ -1289,17 +1289,18 @@ impl MacroBranch { config.set().max_width(new_width); // First try to format as items, then as statements. - let new_body_snippet = match crate::format_snippet(&body_str, &config, true, context.printer) { - Some(new_body) => new_body, - None => { - let new_width = new_width + config.tab_spaces(); - config.set().max_width(new_width); - match crate::format_code_block(&body_str, &config, true, context.printer) { - Some(new_body) => new_body, - None => return None, + let new_body_snippet = + match crate::format_snippet(&body_str, &config, true, context.printer) { + Some(new_body) => new_body, + None => { + let new_width = new_width + config.tab_spaces(); + config.set().max_width(new_width); + match crate::format_code_block(&body_str, &config, true, context.printer) { + Some(new_body) => new_body, + None => return None, + } } - } - }; + }; if !filtered_str_fits(&new_body_snippet.snippet, config.max_width(), shape) { return None; diff --git a/src/missed_spans.rs b/src/missed_spans.rs index 05eb04ea670..c47bb18b16b 100644 --- a/src/missed_spans.rs +++ b/src/missed_spans.rs @@ -280,15 +280,21 @@ impl<'a> FmtVisitor<'a> { self.push_str(&comment_indent.to_string_with_newline(self.config)); let other_lines = &subslice[offset + 1..]; - let comment_str = - rewrite_comment(other_lines, false, comment_shape, self.config, self.printer) - .unwrap_or_else(|| String::from(other_lines)); + let comment_str = rewrite_comment( + other_lines, + false, + comment_shape, + self.config, + self.printer, + ) + .unwrap_or_else(|| String::from(other_lines)); self.push_str(&comment_str); } } } else { - let comment_str = rewrite_comment(subslice, false, comment_shape, self.config, self.printer) - .unwrap_or_else(|| String::from(subslice)); + let comment_str = + rewrite_comment(subslice, false, comment_shape, self.config, self.printer) + .unwrap_or_else(|| String::from(subslice)); self.push_str(&comment_str); } diff --git a/src/parse/session.rs b/src/parse/session.rs index d063ad4c567..e570d9a66e7 100644 --- a/src/parse/session.rs +++ b/src/parse/session.rs @@ -15,11 +15,11 @@ use crate::config::file_lines::LineRange; use crate::config::options::Color; use crate::ignore_path::IgnorePathSet; use crate::parse::parser::{ModError, ModulePathSuccess}; +use crate::print::Printer; use crate::source_map::LineRangeUtils; use crate::utils::starts_with_newline; use crate::visitor::SnippetProvider; use crate::{Config, ErrorKind, FileName}; -use crate::print::Printer; /// ParseSess holds structs necessary for constructing a parser. pub(crate) struct ParseSess { @@ -134,8 +134,10 @@ fn default_dcx( rustc_driver::DEFAULT_LOCALE_RESOURCES.to_vec(), false, ); - Box::new(EmitterWriter::new(Box::new(printer.clone()), fallback_bundle)) - //Box::new(EmitterWriter::stderr(emit_color, fallback_bundle).sm(Some(source_map.clone()))) + Box::new(EmitterWriter::new( + Box::new(printer.clone()), + fallback_bundle, + )) }; DiagCtxt::with_emitter(Box::new(SilentOnIgnoredFilesEmitter { has_non_ignorable_parser_errors: false, diff --git a/src/print.rs b/src/print.rs index f8d981194a8..bbbd002dcb6 100644 --- a/src/print.rs +++ b/src/print.rs @@ -1,8 +1,8 @@ +use crate::Color; +use rustc_errors::{Color as RustColor, ColorSpec, WriteColor}; use std::io::{stderr, stdout, Write}; use std::sync::{Arc, Mutex}; use termcolor::{ColorChoice, StandardStream, WriteColor as _}; -use rustc_errors::{ColorSpec, WriteColor, Color as RustColor}; -use crate::Color; #[derive(Clone)] pub struct Printer { @@ -13,7 +13,12 @@ impl Write for Printer { fn write(&mut self, buf: &[u8]) -> std::io::Result { let mut inner = self.inner.lock().unwrap(); let col = inner.current_color.clone(); - inner.messages.push(PrintMessage::RustcErrTerm(RustcErrTermMessage::new(buf.to_vec(), col))); + inner + .messages + .push(PrintMessage::RustcErrTerm(RustcErrTermMessage::new( + buf.to_vec(), + col, + ))); Ok(buf.len()) } @@ -50,7 +55,6 @@ struct PrinterInner { } impl Printer { - pub fn new(term_output_color: Color) -> Self { Self { inner: Arc::new(Mutex::new(PrinterInner { @@ -73,9 +77,14 @@ impl Printer { pub fn dump(&self) -> Result<(), std::io::Error> { let inner = self.inner.lock().unwrap(); - let mut use_term_stdout = term::stdout().filter(|t| inner.color_setting.use_colored_tty() && t.supports_color()); - let use_rustc_error_color = inner.color_setting.use_colored_tty() && term::stderr().map(|t| t.supports_color()).unwrap_or_default(); - let mut rustc_err_out = use_rustc_error_color.then_some(StandardStream::stderr(ColorChoice::Always)); + let mut use_term_stdout = + term::stdout().filter(|t| inner.color_setting.use_colored_tty() && t.supports_color()); + let use_rustc_error_color = inner.color_setting.use_colored_tty() + && term::stderr() + .map(|t| t.supports_color()) + .unwrap_or_default(); + let mut rustc_err_out = + use_rustc_error_color.then_some(StandardStream::stderr(ColorChoice::Always)); for msg in &inner.messages { match msg { PrintMessage::Stdout(out) => { @@ -166,7 +175,9 @@ macro_rules! buf_term_println { ($pb: expr, $col:expr, $($arg:tt)*) => {{ let mut msg_buf = Vec::new(); let _ = writeln!(&mut msg_buf, $($arg)*); - $pb.push_msg($crate::print::PrintMessage::Term($crate::print::TermMessage::new(msg_buf, $col))); + $pb.push_msg( + $crate::print::PrintMessage::Term($crate::print::TermMessage::new(msg_buf, $col)) + ); }}; } @@ -174,7 +185,7 @@ pub enum PrintMessage { Stdout(Vec), StdErr(Vec), Term(TermMessage), - RustcErrTerm(RustcErrTermMessage) + RustcErrTerm(RustcErrTermMessage), } pub struct TermMessage { diff --git a/src/rewrite.rs b/src/rewrite.rs index fbaf6ccd5e3..2a25c556c80 100644 --- a/src/rewrite.rs +++ b/src/rewrite.rs @@ -8,11 +8,11 @@ use rustc_span::Span; use crate::config::{Config, IndentStyle}; use crate::parse::session::ParseSess; +use crate::print::Printer; use crate::shape::Shape; use crate::skip::SkipContext; use crate::visitor::SnippetProvider; use crate::FormatReport; -use crate::print::Printer; pub(crate) trait Rewrite { /// Rewrite self into shape. diff --git a/src/rustfmt_diff.rs b/src/rustfmt_diff.rs index 509b6d0b284..1515febdf3a 100644 --- a/src/rustfmt_diff.rs +++ b/src/rustfmt_diff.rs @@ -1,7 +1,7 @@ +use crate::buf_term_println; use std::collections::VecDeque; use std::fmt; use std::io::Write; -use crate::buf_term_println; use crate::config::{Config, Verbosity}; use crate::print::Printer; @@ -209,8 +209,12 @@ pub(crate) fn make_diff(expected: &str, actual: &str, context_size: usize) -> Ve results } -pub(crate) fn print_diff(diff: Vec, get_section_title: F, config: &Config, printer: &Printer) -where +pub(crate) fn print_diff( + diff: Vec, + get_section_title: F, + config: &Config, + printer: &Printer, +) where F: Fn(u32) -> String, { let line_terminator = if config.verbose() == Verbosity::Verbose { @@ -230,7 +234,7 @@ where } DiffLine::Expected(ref str) => { buf_term_println!(printer, Some(term::color::GREEN), "+{str}{line_terminator}"); - }, + } DiffLine::Resulting(ref str) => { buf_term_println!(printer, Some(term::color::RED), "-{str}{line_terminator}"); } diff --git a/src/source_file.rs b/src/source_file.rs index 589cb39a100..0e2db62d2e8 100644 --- a/src/source_file.rs +++ b/src/source_file.rs @@ -14,8 +14,8 @@ use crate::create_emitter; #[cfg(test)] use crate::formatting::FileRecord; -use rustc_data_structures::sync::Lrc; use crate::print::Printer; +use rustc_data_structures::sync::Lrc; // Append a newline to the end of each file. pub(crate) fn append_newline(s: &mut String) { @@ -42,7 +42,7 @@ where out, &mut *emitter, config.newline_style(), - &Printer::no_color() + &Printer::no_color(), )?; } emitter.emit_footer(out)?; diff --git a/src/test/configuration_snippet.rs b/src/test/configuration_snippet.rs index 0a0e03f7332..d8b170bf338 100644 --- a/src/test/configuration_snippet.rs +++ b/src/test/configuration_snippet.rs @@ -6,9 +6,9 @@ use std::path::{Path, PathBuf}; use super::{print_mismatches, write_message, DIFF_CONTEXT_SIZE}; use crate::config::{Config, EmitMode, Verbosity}; +use crate::print::Printer; use crate::rustfmt_diff::{make_diff, Mismatch}; use crate::{Input, Session}; -use crate::print::Printer; const CONFIGURATIONS_FILE_NAME: &str = "Configurations.md"; diff --git a/src/test/mod.rs b/src/test/mod.rs index d575e29dd52..b564b3e78c2 100644 --- a/src/test/mod.rs +++ b/src/test/mod.rs @@ -15,8 +15,8 @@ use crate::rustfmt_diff::{make_diff, print_diff, DiffLine, Mismatch, ModifiedChu use crate::{buf_term_println, source_file}; use crate::{is_nightly_channel, FormatReport, FormatReportFormatterBuilder, Input, Session}; -use rustfmt_config_proc_macro::nightly_only_test; use crate::print::Printer; +use rustfmt_config_proc_macro::nightly_only_test; mod configuration_snippet; mod mod_resolver; @@ -678,7 +678,12 @@ fn print_mismatches_default_message(result: HashMap>) { for (file_name, diff) in result { let mismatch_msg_formatter = |line_num| format!("\nMismatch at {}:{}:", file_name.display(), line_num); - print_diff(diff, &mismatch_msg_formatter, &Default::default(), &Printer::no_color()); + print_diff( + diff, + &mismatch_msg_formatter, + &Default::default(), + &Printer::no_color(), + ); } if let Some(mut t) = term::stdout() { @@ -691,7 +696,12 @@ fn print_mismatches String>( mismatch_msg_formatter: T, ) { for (_file_name, diff) in result { - print_diff(diff, &mismatch_msg_formatter, &Default::default(), &Printer::no_color()); + print_diff( + diff, + &mismatch_msg_formatter, + &Default::default(), + &Printer::no_color(), + ); } if let Some(mut t) = term::stdout() { diff --git a/src/test/mod_resolver.rs b/src/test/mod_resolver.rs index 27d7431158e..1e0d831012a 100644 --- a/src/test/mod_resolver.rs +++ b/src/test/mod_resolver.rs @@ -3,8 +3,8 @@ use std::path::PathBuf; use super::read_config; -use crate::{FileName, Input, Session}; use crate::print::Printer; +use crate::{FileName, Input, Session}; fn verify_mod_resolution(input_file_name: &str, exp_misformatted_files: &[&str]) { let input_file = PathBuf::from(input_file_name); diff --git a/src/test/parser.rs b/src/test/parser.rs index e2c6b23703b..167c27f6df3 100644 --- a/src/test/parser.rs +++ b/src/test/parser.rs @@ -4,8 +4,8 @@ use std::path::PathBuf; use super::read_config; use crate::modules::{ModuleResolutionError, ModuleResolutionErrorKind}; -use crate::{ErrorKind, Input, Session}; use crate::print::Printer; +use crate::{ErrorKind, Input, Session}; #[test] fn parser_errors_in_submods_are_surfaced() { diff --git a/src/visitor.rs b/src/visitor.rs index 8ccbd7eaae3..c1731b000c2 100644 --- a/src/visitor.rs +++ b/src/visitor.rs @@ -17,6 +17,7 @@ use crate::items::{ use crate::macros::{macro_style, rewrite_macro, rewrite_macro_def, MacroPosition}; use crate::modules::Module; use crate::parse::session::ParseSess; +use crate::print::Printer; use crate::rewrite::{Rewrite, RewriteContext}; use crate::shape::{Indent, Shape}; use crate::skip::{is_skip_attr, SkipContext}; @@ -28,7 +29,6 @@ use crate::utils::{ last_line_width, mk_sp, ptr_vec_to_ref_vec, rewrite_ident, starts_with_newline, stmt_expr, }; use crate::{ErrorKind, FormatReport, FormattingError}; -use crate::print::Printer; /// Creates a string slice corresponding to the specified span. pub(crate) struct SnippetProvider { @@ -316,8 +316,13 @@ impl<'b, 'a: 'b> FmtVisitor<'a> { // put the other lines below it, shaping it as needed let other_lines = &sub_slice[offset + 1..]; - let comment_str = - rewrite_comment(other_lines, false, comment_shape, config, self.printer); + let comment_str = rewrite_comment( + other_lines, + false, + comment_shape, + config, + self.printer, + ); match comment_str { Some(ref s) => self.push_str(s), None => self.push_str(other_lines), @@ -347,7 +352,8 @@ impl<'b, 'a: 'b> FmtVisitor<'a> { self.push_str(&self.block_indent.to_string_with_newline(config)); } - let comment_str = rewrite_comment(&sub_slice, false, comment_shape, config, self.printer); + let comment_str = + rewrite_comment(&sub_slice, false, comment_shape, config, self.printer); match comment_str { Some(ref s) => self.push_str(s), None => self.push_str(&sub_slice), diff --git a/tests/rustfmt/main.rs b/tests/rustfmt/main.rs index 11fb4786e82..76214b8196e 100644 --- a/tests/rustfmt/main.rs +++ b/tests/rustfmt/main.rs @@ -176,7 +176,11 @@ fn rustfmt_emits_error_on_line_overflow_true() { #[test] #[allow(non_snake_case)] fn dont_emit_ICE() { - let files = ["tests/target/issue_5728.rs", "tests/target/issue_5729.rs", "tests/target/issue_6069.rs"]; + let files = [ + "tests/target/issue_5728.rs", + "tests/target/issue_5729.rs", + "tests/target/issue_6069.rs", + ]; for file in files { let args = [file]; From 1c3f95564f69b49ead084a639acdd2a22690ef81 Mon Sep 17 00:00:00 2001 From: MarcusGrass Date: Sun, 25 Feb 2024 23:27:36 +0100 Subject: [PATCH 10/23] Pre dump --- Progress.md | 28 ++++++++++++++++++++++++++++ src/bin/main.rs | 9 +++++++-- src/formatting.rs | 26 +++++++++++--------------- src/print.rs | 46 ++++++++++++++++++++++++++++++++++++++++++++-- 4 files changed, 90 insertions(+), 19 deletions(-) create mode 100644 Progress.md diff --git a/Progress.md b/Progress.md new file mode 100644 index 00000000000..639ba19be9e --- /dev/null +++ b/Progress.md @@ -0,0 +1,28 @@ +# Multithreaded rewrite progress + +Pretty difficult, high risk of messy code. + +There is only one real hurdle, getting stdout and stderr printing to be synchronized and backwards compatible. + +This is non-trivial when internals can run threaded, eagerly printing cannot happen, messages have +to be passed back through buffers. Even that would be non-trivial if there wasn't color-term writing happening +making things complicated. + +## Rustc termcolor vendoring + +A big problem is the way that `rustc` vendors termcolor. It vendors some types but no printing facilities, +and the vendored code isn't interoperable with the published `termcolor` library, which means that +either the printing facilities need to be reinvented (no), or some compatibility-code needs to be introduced (yes). + +## Changes + +In practice there are four printing facilities used: + +1. Regular `stdout`, pretty easy, replace `println` with printing into a buffer. +2. Regular `stderr`, same as above in all ways that matter. +3. Term stdout, this happens in the `diff`-printing part of the code. +4. Term stderr, this is done by `rustc_error` and the most complex to integrate. + +Additionally, these four facilities can't be separated, since they have to preserve order between each other. + + diff --git a/src/bin/main.rs b/src/bin/main.rs index 25adf7cc9e3..33c06c2ad97 100644 --- a/src/bin/main.rs +++ b/src/bin/main.rs @@ -295,6 +295,7 @@ fn format_string(input: String, options: GetOptsOptions) -> Result { let printer = Printer::new(config.color()); let mut session = Session::new(config, Some(out), &printer); format_and_emit_report(&mut session, Input::Text(input)); + printer.dump()?; let exit_code = if session.has_operational_errors() || session.has_parsing_errors() { 1 @@ -424,7 +425,9 @@ fn format( .unwrap() .unwrap(); let output = thread_out.session_result?; - stdout().write_all(&output).unwrap(); + if !output.is_empty() { + stdout().write_all(&output).unwrap(); + } thread_out.printer.dump()?; if thread_out.exit_code != 0 { exit_code = thread_out.exit_code; @@ -440,7 +443,9 @@ fn format( let handle_res = handles.remove(&thread_out.id).unwrap().join(); handle_res.unwrap().unwrap(); let output = thread_out.session_result?; - stdout().write_all(&output).unwrap(); + if !output.is_empty() { + stdout().write_all(&output).unwrap(); + } thread_out.printer.dump()?; if thread_out.exit_code != 0 { exit_code = thread_out.exit_code; diff --git a/src/formatting.rs b/src/formatting.rs index 675ca9f3d6e..e6a00416626 100644 --- a/src/formatting.rs +++ b/src/formatting.rs @@ -17,7 +17,7 @@ use crate::parse::session::ParseSess; use crate::print::Printer; use crate::utils::{contains_skip, count_newlines}; use crate::visitor::FmtVisitor; -use crate::{buf_eprintln, modules, source_file, ErrorKind, FormatReport, Input, Session}; +use crate::{buf_eprintln, modules, source_file, ErrorKind, FormatReport, Input, Session, buf_println}; mod generated; mod newline_style; @@ -40,7 +40,10 @@ impl<'b, T: Write + 'b> Session<'b, T> { if self.config.disable_all_formatting() { // When the input is from stdin, echo back the input. return match input { - Input::Text(ref buf) => echo_back_stdin(buf), + Input::Text(ref buf) => { + buf_println!(self.printer, "{buf}"); + Ok(FormatReport::new()) + }, _ => Ok(FormatReport::new()), }; } @@ -91,13 +94,6 @@ fn should_skip_module( false } -fn echo_back_stdin(input: &str) -> Result { - if let Err(e) = io::stdout().write_all(input.as_bytes()) { - return Err(From::from(e)); - } - Ok(FormatReport::new()) -} - // Format an entire crate (or subset of the module tree). fn format_project( input: Input, @@ -153,20 +149,20 @@ fn format_project( for (path, module) in files { if input_is_stdin && contains_skip(module.attrs()) { - return echo_back_stdin( - context + buf_println!(printer, "{}", context .parse_session .snippet_provider(module.span) - .entire_snippet(), - ); + .entire_snippet()); + return Ok(FormatReport::new()); } - should_emit_verbose(input_is_stdin, config, || println!("Formatting {}", path)); + should_emit_verbose(input_is_stdin, config, || buf_println!(printer, "Formatting {}", path)); context.format_file(path, &module, is_macro_def)?; } timer = timer.done_formatting(); should_emit_verbose(input_is_stdin, config, || { - println!( + buf_println!( + printer, "Spent {0:.3} secs in the parsing phase, and {1:.3} secs in the formatting phase", timer.get_parse_time(), timer.get_format_time(), diff --git a/src/print.rs b/src/print.rs index bbbd002dcb6..b6a8e9f1c51 100644 --- a/src/print.rs +++ b/src/print.rs @@ -1,3 +1,4 @@ +use std::fmt::Formatter; use crate::Color; use rustc_errors::{Color as RustColor, ColorSpec, WriteColor}; use std::io::{stderr, stdout, Write}; @@ -22,10 +23,18 @@ impl Write for Printer { Ok(buf.len()) } + #[inline] + fn write_all(&mut self, buf: &[u8]) -> std::io::Result<()> { + self.write(buf)?; + Ok(()) + } + #[inline] fn flush(&mut self) -> std::io::Result<()> { + //println!("Flush"); Ok(()) } + } impl WriteColor for Printer { @@ -45,6 +54,7 @@ impl WriteColor for Printer { self.inner.lock().unwrap().current_color.take(); Ok(()) } + } struct PrinterInner { @@ -77,6 +87,10 @@ impl Printer { pub fn dump(&self) -> Result<(), std::io::Error> { let inner = self.inner.lock().unwrap(); + // Pretty common case, early exit + if inner.messages.is_empty() { + return Ok(()); + } let mut use_term_stdout = term::stdout().filter(|t| inner.color_setting.use_colored_tty() && t.supports_color()); let use_rustc_error_color = inner.color_setting.use_colored_tty() @@ -121,6 +135,8 @@ impl Printer { } } } + stdout().flush()?; + stderr().flush()?; Ok(()) } @@ -130,8 +146,15 @@ impl Printer { // as far as I can tell fn rustc_colorspec_compat(rustc: &ColorSpec) -> termcolor::ColorSpec { let mut cs = termcolor::ColorSpec::new(); - let col = rustc.fg().and_then(rustc_color_compat); - cs.set_fg(col); + let fg = rustc.fg().and_then(rustc_color_compat); + cs.set_fg(fg); + let bg = rustc.bg().and_then(rustc_color_compat); + cs.set_bg(bg); + cs.set_bold(rustc.bold()); + cs.set_strikethrough(rustc.strikethrough()); + cs.set_underline(rustc.underline()); + cs.set_intense(rustc.intense()); + cs.set_italic(rustc.italic()); cs } @@ -188,6 +211,25 @@ pub enum PrintMessage { RustcErrTerm(RustcErrTermMessage), } +impl std::fmt::Debug for PrintMessage { + fn fmt(&self, f: &mut Formatter<'_>) -> std::fmt::Result { + match self { + PrintMessage::Stdout(buf) => { + f.write_fmt(format_args!("PrintMessage::Stdout({:?})", core::str::from_utf8(buf))) + } + PrintMessage::StdErr(buf) => { + f.write_fmt(format_args!("PrintMessage::Stderr({:?})", core::str::from_utf8(buf))) + } + PrintMessage::Term(msg) => { + f.write_fmt(format_args!("PrintMessage::Term({:?}, {:?})", core::str::from_utf8(&msg.message), msg.color)) + } + PrintMessage::RustcErrTerm(msg) => { + f.write_fmt(format_args!("PrintMessage::RustcErrTerm({:?}, {:?})", core::str::from_utf8(&msg.message), msg.color)) + } + } + } +} + pub struct TermMessage { message: Vec, color: Option, From ef3016880ea541aa2f9605557649debe3da0ad1f Mon Sep 17 00:00:00 2001 From: MarcusGrass Date: Mon, 26 Feb 2024 06:37:41 +0100 Subject: [PATCH 11/23] Fixup `rustc_error` print output --- src/parse/session.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/parse/session.rs b/src/parse/session.rs index e570d9a66e7..191c55410ac 100644 --- a/src/parse/session.rs +++ b/src/parse/session.rs @@ -137,7 +137,7 @@ fn default_dcx( Box::new(EmitterWriter::new( Box::new(printer.clone()), fallback_bundle, - )) + ).sm(Some(source_map.clone()))) }; DiagCtxt::with_emitter(Box::new(SilentOnIgnoredFilesEmitter { has_non_ignorable_parser_errors: false, From 70cc3b5250a8d81675e35b35923d8f5e934ad394 Mon Sep 17 00:00:00 2001 From: MarcusGrass Date: Mon, 26 Feb 2024 06:43:00 +0100 Subject: [PATCH 12/23] Fixup stdin echo, no newline add --- src/formatting.rs | 8 +++++--- src/print.rs | 9 +++++++++ 2 files changed, 14 insertions(+), 3 deletions(-) diff --git a/src/formatting.rs b/src/formatting.rs index e6a00416626..e2dab304deb 100644 --- a/src/formatting.rs +++ b/src/formatting.rs @@ -17,7 +17,7 @@ use crate::parse::session::ParseSess; use crate::print::Printer; use crate::utils::{contains_skip, count_newlines}; use crate::visitor::FmtVisitor; -use crate::{buf_eprintln, modules, source_file, ErrorKind, FormatReport, Input, Session, buf_println}; +use crate::{buf_eprintln, modules, source_file, ErrorKind, FormatReport, Input, Session, buf_println, buf_print}; mod generated; mod newline_style; @@ -41,7 +41,8 @@ impl<'b, T: Write + 'b> Session<'b, T> { // When the input is from stdin, echo back the input. return match input { Input::Text(ref buf) => { - buf_println!(self.printer, "{buf}"); + // Echo back stdin + buf_print!(self.printer, "{buf}"); Ok(FormatReport::new()) }, _ => Ok(FormatReport::new()), @@ -149,7 +150,8 @@ fn format_project( for (path, module) in files { if input_is_stdin && contains_skip(module.attrs()) { - buf_println!(printer, "{}", context + // Echo back stdin + buf_print!(printer, "{}", context .parse_session .snippet_provider(module.span) .entire_snippet()); diff --git a/src/print.rs b/src/print.rs index b6a8e9f1c51..a855f8a1549 100644 --- a/src/print.rs +++ b/src/print.rs @@ -175,6 +175,15 @@ fn rustc_color_compat(rustc: &RustColor) -> Option { Some(col) } +#[macro_export] +macro_rules! buf_print { + ($pb: expr, $($arg:tt)*) => {{ + let mut msg_buf = Vec::new(); + let _ = write!(&mut msg_buf, $($arg)*); + $pb.push_msg($crate::print::PrintMessage::Stdout(msg_buf)); + }}; +} + #[macro_export] macro_rules! buf_println { ($pb: expr, $($arg:tt)*) => {{ From b1e1cd477abf6c8aa346fd62cba44a19e69eac7c Mon Sep 17 00:00:00 2001 From: MarcusGrass Date: Mon, 26 Feb 2024 06:44:24 +0100 Subject: [PATCH 13/23] Fmt --- src/formatting.rs | 19 ++++++++++++++----- src/parse/session.rs | 8 ++++---- src/print.rs | 34 +++++++++++++++++++--------------- 3 files changed, 37 insertions(+), 24 deletions(-) diff --git a/src/formatting.rs b/src/formatting.rs index e2dab304deb..3c5380706a2 100644 --- a/src/formatting.rs +++ b/src/formatting.rs @@ -17,7 +17,10 @@ use crate::parse::session::ParseSess; use crate::print::Printer; use crate::utils::{contains_skip, count_newlines}; use crate::visitor::FmtVisitor; -use crate::{buf_eprintln, modules, source_file, ErrorKind, FormatReport, Input, Session, buf_println, buf_print}; +use crate::{ + buf_eprintln, buf_print, buf_println, modules, source_file, ErrorKind, FormatReport, Input, + Session, +}; mod generated; mod newline_style; @@ -44,7 +47,7 @@ impl<'b, T: Write + 'b> Session<'b, T> { // Echo back stdin buf_print!(self.printer, "{buf}"); Ok(FormatReport::new()) - }, + } _ => Ok(FormatReport::new()), }; } @@ -151,13 +154,19 @@ fn format_project( for (path, module) in files { if input_is_stdin && contains_skip(module.attrs()) { // Echo back stdin - buf_print!(printer, "{}", context + buf_print!( + printer, + "{}", + context .parse_session .snippet_provider(module.span) - .entire_snippet()); + .entire_snippet() + ); return Ok(FormatReport::new()); } - should_emit_verbose(input_is_stdin, config, || buf_println!(printer, "Formatting {}", path)); + should_emit_verbose(input_is_stdin, config, || { + buf_println!(printer, "Formatting {}", path) + }); context.format_file(path, &module, is_macro_def)?; } timer = timer.done_formatting(); diff --git a/src/parse/session.rs b/src/parse/session.rs index 191c55410ac..bfd4551656f 100644 --- a/src/parse/session.rs +++ b/src/parse/session.rs @@ -134,10 +134,10 @@ fn default_dcx( rustc_driver::DEFAULT_LOCALE_RESOURCES.to_vec(), false, ); - Box::new(EmitterWriter::new( - Box::new(printer.clone()), - fallback_bundle, - ).sm(Some(source_map.clone()))) + Box::new( + EmitterWriter::new(Box::new(printer.clone()), fallback_bundle) + .sm(Some(source_map.clone())), + ) }; DiagCtxt::with_emitter(Box::new(SilentOnIgnoredFilesEmitter { has_non_ignorable_parser_errors: false, diff --git a/src/print.rs b/src/print.rs index a855f8a1549..ec19c42bd83 100644 --- a/src/print.rs +++ b/src/print.rs @@ -1,6 +1,6 @@ -use std::fmt::Formatter; use crate::Color; use rustc_errors::{Color as RustColor, ColorSpec, WriteColor}; +use std::fmt::Formatter; use std::io::{stderr, stdout, Write}; use std::sync::{Arc, Mutex}; use termcolor::{ColorChoice, StandardStream, WriteColor as _}; @@ -34,7 +34,6 @@ impl Write for Printer { //println!("Flush"); Ok(()) } - } impl WriteColor for Printer { @@ -54,7 +53,6 @@ impl WriteColor for Printer { self.inner.lock().unwrap().current_color.take(); Ok(()) } - } struct PrinterInner { @@ -223,18 +221,24 @@ pub enum PrintMessage { impl std::fmt::Debug for PrintMessage { fn fmt(&self, f: &mut Formatter<'_>) -> std::fmt::Result { match self { - PrintMessage::Stdout(buf) => { - f.write_fmt(format_args!("PrintMessage::Stdout({:?})", core::str::from_utf8(buf))) - } - PrintMessage::StdErr(buf) => { - f.write_fmt(format_args!("PrintMessage::Stderr({:?})", core::str::from_utf8(buf))) - } - PrintMessage::Term(msg) => { - f.write_fmt(format_args!("PrintMessage::Term({:?}, {:?})", core::str::from_utf8(&msg.message), msg.color)) - } - PrintMessage::RustcErrTerm(msg) => { - f.write_fmt(format_args!("PrintMessage::RustcErrTerm({:?}, {:?})", core::str::from_utf8(&msg.message), msg.color)) - } + PrintMessage::Stdout(buf) => f.write_fmt(format_args!( + "PrintMessage::Stdout({:?})", + core::str::from_utf8(buf) + )), + PrintMessage::StdErr(buf) => f.write_fmt(format_args!( + "PrintMessage::Stderr({:?})", + core::str::from_utf8(buf) + )), + PrintMessage::Term(msg) => f.write_fmt(format_args!( + "PrintMessage::Term({:?}, {:?})", + core::str::from_utf8(&msg.message), + msg.color + )), + PrintMessage::RustcErrTerm(msg) => f.write_fmt(format_args!( + "PrintMessage::RustcErrTerm({:?}, {:?})", + core::str::from_utf8(&msg.message), + msg.color + )), } } } From b749673e1bbd39efbd62e725990eaef335d7af38 Mon Sep 17 00:00:00 2001 From: MarcusGrass Date: Mon, 26 Feb 2024 06:56:41 +0100 Subject: [PATCH 14/23] Use available parallelism --- src/bin/main.rs | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/src/bin/main.rs b/src/bin/main.rs index 33c06c2ad97..2cf2af658f6 100644 --- a/src/bin/main.rs +++ b/src/bin/main.rs @@ -12,6 +12,7 @@ use std::collections::HashMap; use std::env; use std::fs::File; use std::io::{self, stdout, Read, Write}; +use std::num::NonZeroUsize; use std::path::{Path, PathBuf}; use std::str::FromStr; use std::thread::JoinHandle; @@ -327,7 +328,7 @@ fn format( } } - let num_cpus = 32; + let parallelism = std::thread::available_parallelism().unwrap_or(NonZeroUsize::MIN); let (send, recv) = std::sync::mpsc::channel(); let mut exit_code = 0; @@ -416,7 +417,7 @@ fn format( handles.insert(id, handle); id += 1; outstanding += 1; - if outstanding >= num_cpus { + if outstanding >= parallelism.get() { if let Ok(thread_out) = recv.recv() { handles .remove(&thread_out.id) From 7297078bdb14758812726f714c530ce296f5df2f Mon Sep 17 00:00:00 2001 From: MarcusGrass Date: Mon, 26 Feb 2024 08:50:14 +0100 Subject: [PATCH 15/23] break out join handling --- src/bin/main.rs | 74 ++++++++++++++++++++++++++++++------------------- 1 file changed, 46 insertions(+), 28 deletions(-) diff --git a/src/bin/main.rs b/src/bin/main.rs index 2cf2af658f6..11c7b3f4885 100644 --- a/src/bin/main.rs +++ b/src/bin/main.rs @@ -9,13 +9,13 @@ use rustfmt_nightly as rustfmt; use tracing_subscriber::EnvFilter; use std::collections::HashMap; -use std::env; use std::fs::File; use std::io::{self, stdout, Read, Write}; use std::num::NonZeroUsize; use std::path::{Path, PathBuf}; use std::str::FromStr; use std::thread::JoinHandle; +use std::{env, panic}; use getopts::{Matches, Options}; use rustfmt_nightly::print::Printer; @@ -372,11 +372,11 @@ fn format( drop(session); let _ = s.send(ThreadedFileOutput { session_result: Err(e), - id, + id: my_id, exit_code, printer, }); - return Ok::<_, std::io::Error>(my_id); + return Ok::<_, std::io::Error>(()); } }; if local_config.verbose() == Verbosity::Verbose { @@ -408,30 +408,20 @@ fn format( drop(session); let _ = s.send(ThreadedFileOutput { session_result: Ok(session_out), - id, + id: my_id, exit_code, printer, }); - Ok(my_id) + Ok(()) }); handles.insert(id, handle); id += 1; outstanding += 1; if outstanding >= parallelism.get() { if let Ok(thread_out) = recv.recv() { - handles - .remove(&thread_out.id) - .unwrap() - .join() - .unwrap() - .unwrap(); - let output = thread_out.session_result?; - if !output.is_empty() { - stdout().write_all(&output).unwrap(); - } - thread_out.printer.dump()?; - if thread_out.exit_code != 0 { - exit_code = thread_out.exit_code; + let exit = join_thread_reporting_back(&mut handles, thread_out)?; + if exit != 0 { + exit_code = exit; } outstanding -= 1; } else { @@ -439,22 +429,26 @@ fn format( } } } + // Drop sender, or this will deadlock drop(send); while let Ok(thread_out) = recv.recv() { - let handle_res = handles.remove(&thread_out.id).unwrap().join(); - handle_res.unwrap().unwrap(); - let output = thread_out.session_result?; - if !output.is_empty() { - stdout().write_all(&output).unwrap(); - } - thread_out.printer.dump()?; - if thread_out.exit_code != 0 { - exit_code = thread_out.exit_code; + let exit = join_thread_reporting_back(&mut handles, thread_out)?; + if exit != 0 { + exit_code = exit; } } // These have errors for (_id, jh) in handles { - jh.join().unwrap().unwrap(); + match jh.join() { + Ok(res) => { + res?; + } + Err(panicked) => { + // Propagate the thread's panic, not much to do here + // if the error should be preserved (which it should) + panic::resume_unwind(panicked) + } + } } // If we were given a path via dump-minimal-config, output any options @@ -468,6 +462,30 @@ fn format( Ok(exit_code) } +fn join_thread_reporting_back( + handles: &mut HashMap>>, + threaded_file_output: ThreadedFileOutput, +) -> Result { + let handle = handles + .remove(&threaded_file_output.id) + .expect("Join thread not found by id"); + match handle.join() { + Ok(result) => { + result?; + } + Err(panicked) => { + // Should never end up here logically, since the thread reported back + panic::resume_unwind(panicked) + } + } + let output = threaded_file_output.session_result?; + if !output.is_empty() { + stdout().write_all(&output).unwrap(); + } + threaded_file_output.printer.dump()?; + Ok(threaded_file_output.exit_code) +} + fn format_and_emit_report(session: &mut Session<'_, T>, input: Input) { match session.format(input) { Ok(report) => { From 2d8c7aa1225cd3aa2e028b1fe451e702dbd95236 Mon Sep 17 00:00:00 2001 From: MarcusGrass Date: Mon, 26 Feb 2024 09:01:30 +0100 Subject: [PATCH 16/23] Add some comments --- Progress.md | 4 ++-- src/bin/main.rs | 14 ++++++++++---- 2 files changed, 12 insertions(+), 6 deletions(-) diff --git a/Progress.md b/Progress.md index 639ba19be9e..98e88547371 100644 --- a/Progress.md +++ b/Progress.md @@ -16,6 +16,8 @@ either the printing facilities need to be reinvented (no), or some compatibility ## Changes +### Output facilities + In practice there are four printing facilities used: 1. Regular `stdout`, pretty easy, replace `println` with printing into a buffer. @@ -24,5 +26,3 @@ In practice there are four printing facilities used: 4. Term stderr, this is done by `rustc_error` and the most complex to integrate. Additionally, these four facilities can't be separated, since they have to preserve order between each other. - - diff --git a/src/bin/main.rs b/src/bin/main.rs index 11c7b3f4885..858d2474546 100644 --- a/src/bin/main.rs +++ b/src/bin/main.rs @@ -329,14 +329,17 @@ fn format( } let parallelism = std::thread::available_parallelism().unwrap_or(NonZeroUsize::MIN); + // Use a channel + map to get 'next-completed' thread, rather than + // waiting on the chronologically first handle to join, if there are more files + // than available parallelism. let (send, recv) = std::sync::mpsc::channel(); + let mut handles: HashMap> = HashMap::new(); let mut exit_code = 0; let mut outstanding = 0; // If the thread panics, the channel will just be dropped, - // so keep track of the spinning threads + // so keep track of the spinning threads to get a stacktrace from it later let mut id = 0; - let mut handles: HashMap> = HashMap::new(); let check = options.check; let color = config.color(); for file in files { @@ -373,7 +376,7 @@ fn format( let _ = s.send(ThreadedFileOutput { session_result: Err(e), id: my_id, - exit_code, + exit_code: 1, printer, }); return Ok::<_, std::io::Error>(()); @@ -431,13 +434,16 @@ fn format( } // Drop sender, or this will deadlock drop(send); + + // Drain running threads while let Ok(thread_out) = recv.recv() { let exit = join_thread_reporting_back(&mut handles, thread_out)?; if exit != 0 { exit_code = exit; } } - // These have errors + + // All successful threads have been removed from `handles` only errors are left for (_id, jh) in handles { match jh.join() { Ok(res) => { From e10204d5f818f12c19f90560075951302aae9bae Mon Sep 17 00:00:00 2001 From: MarcusGrass Date: Mon, 26 Feb 2024 09:09:40 +0100 Subject: [PATCH 17/23] Remove printer from emitter trait --- src/emitter.rs | 1 - src/emitter/checkstyle.rs | 3 --- src/emitter/diff.rs | 27 +++++++++++++-------------- src/emitter/files.rs | 15 +++++++-------- src/emitter/files_with_backup.rs | 1 - src/emitter/json.rs | 5 ----- src/emitter/modified_lines.rs | 1 - src/emitter/stdout.rs | 1 - src/formatting.rs | 1 - src/lib.rs | 8 ++++---- src/source_file.rs | 8 +++----- 11 files changed, 27 insertions(+), 44 deletions(-) diff --git a/src/emitter.rs b/src/emitter.rs index 8caeb209840..720c3c64323 100644 --- a/src/emitter.rs +++ b/src/emitter.rs @@ -33,7 +33,6 @@ pub(crate) trait Emitter { fn emit_formatted_file( &mut self, output: &mut dyn Write, - printer: &Printer, formatted_file: FormattedFile<'_>, ) -> Result; diff --git a/src/emitter/checkstyle.rs b/src/emitter/checkstyle.rs index c33b81ebb5e..9385ae59a06 100644 --- a/src/emitter/checkstyle.rs +++ b/src/emitter/checkstyle.rs @@ -21,7 +21,6 @@ impl Emitter for CheckstyleEmitter { fn emit_formatted_file( &mut self, output: &mut dyn Write, - _printer: &Printer, FormattedFile { filename, original_text, @@ -101,7 +100,6 @@ mod tests { let _ = emitter .emit_formatted_file( &mut writer, - &Printer::no_color(), FormattedFile { filename: &FileName::Real(PathBuf::from(bin_file)), original_text: &bin_original.join("\n"), @@ -112,7 +110,6 @@ mod tests { let _ = emitter .emit_formatted_file( &mut writer, - &Printer::no_color(), FormattedFile { filename: &FileName::Real(PathBuf::from(lib_file)), original_text: &lib_original.join("\n"), diff --git a/src/emitter/diff.rs b/src/emitter/diff.rs index 58060f24225..64917317ced 100644 --- a/src/emitter/diff.rs +++ b/src/emitter/diff.rs @@ -2,21 +2,21 @@ use super::*; use crate::config::Config; use crate::rustfmt_diff::{make_diff, print_diff}; -pub(crate) struct DiffEmitter { +pub(crate) struct DiffEmitter<'a> { config: Config, + printer: &'a Printer, } -impl DiffEmitter { - pub(crate) fn new(config: Config) -> Self { - Self { config } +impl<'a> DiffEmitter<'a> { + pub(crate) fn new(config: Config, printer: &'a Printer) -> Self { + Self { config, printer } } } -impl Emitter for DiffEmitter { +impl<'a> Emitter for DiffEmitter<'a> { fn emit_formatted_file( &mut self, output: &mut dyn Write, - printer: &Printer, FormattedFile { filename, original_text, @@ -35,7 +35,7 @@ impl Emitter for DiffEmitter { mismatch, |line_num| format!("Diff in {}:{}:", filename, line_num), &self.config, - printer, + self.printer, ); } } else if original_text != formatted_text { @@ -59,11 +59,11 @@ mod tests { fn does_not_print_when_no_files_reformatted() { let mut writer = Vec::new(); let config = Config::default(); - let mut emitter = DiffEmitter::new(config); + let printer = Printer::no_color(); + let mut emitter = DiffEmitter::new(config, &printer); let result = emitter .emit_formatted_file( &mut writer, - &Printer::no_color(), FormattedFile { filename: &FileName::Real(PathBuf::from("src/lib.rs")), original_text: "fn empty() {}\n", @@ -87,11 +87,11 @@ mod tests { let mut writer = Vec::new(); let mut config = Config::default(); config.set().print_misformatted_file_names(true); - let mut emitter = DiffEmitter::new(config); + let printer = Printer::no_color(); + let mut emitter = DiffEmitter::new(config, &printer); let _ = emitter .emit_formatted_file( &mut writer, - &Printer::no_color(), FormattedFile { filename: &FileName::Real(PathBuf::from(bin_file)), original_text: bin_original, @@ -102,7 +102,6 @@ mod tests { let _ = emitter .emit_formatted_file( &mut writer, - &Printer::no_color(), FormattedFile { filename: &FileName::Real(PathBuf::from(lib_file)), original_text: lib_original, @@ -121,11 +120,11 @@ mod tests { fn prints_newline_message_with_only_newline_style_diff() { let mut writer = Vec::new(); let config = Config::default(); - let mut emitter = DiffEmitter::new(config); + let printer = Printer::no_color(); + let mut emitter = DiffEmitter::new(config, &printer); let _ = emitter .emit_formatted_file( &mut writer, - &Printer::no_color(), FormattedFile { filename: &FileName::Real(PathBuf::from("src/lib.rs")), original_text: "fn empty() {}\n", diff --git a/src/emitter/files.rs b/src/emitter/files.rs index 1c2ed146766..376c1ba1915 100644 --- a/src/emitter/files.rs +++ b/src/emitter/files.rs @@ -2,24 +2,23 @@ use super::*; use crate::buf_println; use std::fs; -#[derive(Debug, Default)] -pub(crate) struct FilesEmitter { - print_misformatted_file_names: bool, +#[derive(Default)] +pub(crate) struct FilesEmitter<'a> { + print_misformatted_file_names: Option<&'a Printer>, } -impl FilesEmitter { - pub(crate) fn new(print_misformatted_file_names: bool) -> Self { +impl<'a> FilesEmitter<'a> { + pub(crate) fn new(print_misformatted_file_names: Option<&'a Printer>) -> Self { Self { print_misformatted_file_names, } } } -impl Emitter for FilesEmitter { +impl<'a> Emitter for FilesEmitter<'a> { fn emit_formatted_file( &mut self, _output: &mut dyn Write, - printer: &Printer, FormattedFile { filename, original_text, @@ -30,7 +29,7 @@ impl Emitter for FilesEmitter { let filename = ensure_real_path(filename); if original_text != formatted_text { fs::write(filename, formatted_text)?; - if self.print_misformatted_file_names { + if let Some(printer) = self.print_misformatted_file_names { buf_println!(printer, "{}", filename.display()); } } diff --git a/src/emitter/files_with_backup.rs b/src/emitter/files_with_backup.rs index 4b239c339cd..4c15f6fa5ec 100644 --- a/src/emitter/files_with_backup.rs +++ b/src/emitter/files_with_backup.rs @@ -8,7 +8,6 @@ impl Emitter for FilesWithBackupEmitter { fn emit_formatted_file( &mut self, _output: &mut dyn Write, - _printer: &Printer, FormattedFile { filename, original_text, diff --git a/src/emitter/json.rs b/src/emitter/json.rs index 5acbbbca474..084f565804c 100644 --- a/src/emitter/json.rs +++ b/src/emitter/json.rs @@ -32,7 +32,6 @@ impl Emitter for JsonEmitter { fn emit_formatted_file( &mut self, _output: &mut dyn Write, - _printer: &Printer, FormattedFile { filename, original_text, @@ -198,7 +197,6 @@ mod tests { let result = emitter .emit_formatted_file( &mut writer, - &Printer::no_color(), FormattedFile { filename: &FileName::Real(PathBuf::from("src/lib.rs")), original_text: "fn empty() {}\n", @@ -246,7 +244,6 @@ mod tests { let result = emitter .emit_formatted_file( &mut writer, - &Printer::no_color(), FormattedFile { filename: &FileName::Real(PathBuf::from(file_name)), original_text: &original.join("\n"), @@ -299,7 +296,6 @@ mod tests { let _ = emitter .emit_formatted_file( &mut writer, - &Printer::no_color(), FormattedFile { filename: &FileName::Real(PathBuf::from(bin_file)), original_text: &bin_original.join("\n"), @@ -310,7 +306,6 @@ mod tests { let _ = emitter .emit_formatted_file( &mut writer, - &Printer::no_color(), FormattedFile { filename: &FileName::Real(PathBuf::from(lib_file)), original_text: &lib_original.join("\n"), diff --git a/src/emitter/modified_lines.rs b/src/emitter/modified_lines.rs index f9e300eb087..81f0a31b974 100644 --- a/src/emitter/modified_lines.rs +++ b/src/emitter/modified_lines.rs @@ -8,7 +8,6 @@ impl Emitter for ModifiedLinesEmitter { fn emit_formatted_file( &mut self, output: &mut dyn Write, - _printer: &Printer, FormattedFile { original_text, formatted_text, diff --git a/src/emitter/stdout.rs b/src/emitter/stdout.rs index 6edaa62cfb8..6c467e7f897 100644 --- a/src/emitter/stdout.rs +++ b/src/emitter/stdout.rs @@ -16,7 +16,6 @@ impl Emitter for IntoOutputEmitter { fn emit_formatted_file( &mut self, output: &mut dyn Write, - _printer: &Printer, FormattedFile { filename, formatted_text, diff --git a/src/formatting.rs b/src/formatting.rs index 3c5380706a2..44777c289bc 100644 --- a/src/formatting.rs +++ b/src/formatting.rs @@ -305,7 +305,6 @@ impl<'b, T: Write + 'b> FormatHandler for Session<'b, T> { out, &mut *self.emitter, self.config.newline_style(), - self.printer, ) { Ok(ref result) if result.has_diff => report.add_diff(), Err(e) => { diff --git a/src/lib.rs b/src/lib.rs index 5021a311e1f..642675563a9 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -452,7 +452,7 @@ pub struct Session<'b, T: Write> { impl<'b, T: Write + 'b> Session<'b, T> { pub fn new(config: Config, mut out: Option<&'b mut T>, printer: &'b Printer) -> Session<'b, T> { - let emitter = create_emitter(&config); + let emitter = create_emitter(&config, printer); if let Some(ref mut out) = out { let _ = emitter.emit_header(out); @@ -523,13 +523,13 @@ impl<'b, T: Write + 'b> Session<'b, T> { } } -pub(crate) fn create_emitter<'a>(config: &Config) -> Box { +pub(crate) fn create_emitter<'a>(config: &Config, printer: &'a Printer) -> Box { match config.emit_mode() { EmitMode::Files if config.make_backup() => { Box::new(emitter::FilesWithBackupEmitter::default()) } EmitMode::Files => Box::new(emitter::FilesEmitter::new( - config.print_misformatted_file_names(), + config.print_misformatted_file_names().then_some(printer), )), EmitMode::Stdout | EmitMode::Coverage => { Box::new(emitter::IntoOutputEmitter::new(config.verbose())) @@ -537,7 +537,7 @@ pub(crate) fn create_emitter<'a>(config: &Config) -> Box { EmitMode::Json => Box::new(emitter::JsonEmitter::default()), EmitMode::ModifiedLines => Box::new(emitter::ModifiedLinesEmitter::default()), EmitMode::Checkstyle => Box::new(emitter::CheckstyleEmitter::default()), - EmitMode::Diff => Box::new(emitter::DiffEmitter::new(config.clone())), + EmitMode::Diff => Box::new(emitter::DiffEmitter::new(config.clone(), printer)), } } diff --git a/src/source_file.rs b/src/source_file.rs index 0e2db62d2e8..f0da963752d 100644 --- a/src/source_file.rs +++ b/src/source_file.rs @@ -14,7 +14,6 @@ use crate::create_emitter; #[cfg(test)] use crate::formatting::FileRecord; -use crate::print::Printer; use rustc_data_structures::sync::Lrc; // Append a newline to the end of each file. @@ -31,7 +30,8 @@ pub(crate) fn write_all_files( where T: Write, { - let mut emitter = create_emitter(config); + let printer = crate::print::Printer::no_color(); + let mut emitter = create_emitter(config, &printer); emitter.emit_header(out)?; for (filename, text) in source_file { @@ -42,7 +42,6 @@ where out, &mut *emitter, config.newline_style(), - &Printer::no_color(), )?; } emitter.emit_footer(out)?; @@ -57,7 +56,6 @@ pub(crate) fn write_file( out: &mut T, emitter: &mut dyn Emitter, newline_style: NewlineStyle, - printer: &Printer, ) -> Result where T: Write, @@ -104,5 +102,5 @@ where formatted_text, }; - emitter.emit_formatted_file(out, printer, formatted_file) + emitter.emit_formatted_file(out, formatted_file) } From 81fa030baf209a8b02c31027c06d88cdee09f0ec Mon Sep 17 00:00:00 2001 From: MarcusGrass Date: Mon, 26 Feb 2024 09:18:07 +0100 Subject: [PATCH 18/23] Comments, reorder, rename --- src/bin/main.rs | 4 +- src/print.rs | 133 +++++++++++++++++++++--------------------------- src/test/mod.rs | 2 +- 3 files changed, 62 insertions(+), 77 deletions(-) diff --git a/src/bin/main.rs b/src/bin/main.rs index 858d2474546..ea6c2958785 100644 --- a/src/bin/main.rs +++ b/src/bin/main.rs @@ -296,7 +296,7 @@ fn format_string(input: String, options: GetOptsOptions) -> Result { let printer = Printer::new(config.color()); let mut session = Session::new(config, Some(out), &printer); format_and_emit_report(&mut session, Input::Text(input)); - printer.dump()?; + printer.write_to_outputs()?; let exit_code = if session.has_operational_errors() || session.has_parsing_errors() { 1 @@ -488,7 +488,7 @@ fn join_thread_reporting_back( if !output.is_empty() { stdout().write_all(&output).unwrap(); } - threaded_file_output.printer.dump()?; + threaded_file_output.printer.write_to_outputs()?; Ok(threaded_file_output.exit_code) } diff --git a/src/print.rs b/src/print.rs index ec19c42bd83..f506634d2d9 100644 --- a/src/print.rs +++ b/src/print.rs @@ -1,60 +1,17 @@ use crate::Color; use rustc_errors::{Color as RustColor, ColorSpec, WriteColor}; -use std::fmt::Formatter; use std::io::{stderr, stdout, Write}; use std::sync::{Arc, Mutex}; use termcolor::{ColorChoice, StandardStream, WriteColor as _}; #[derive(Clone)] pub struct Printer { + // Needs `Mutex` to be UnwindSafe, although, this should be + // safe to `Unwind` without it. + // Needs `Arc` to pass the boundary over to `rustc_error`. inner: Arc>, } -impl Write for Printer { - fn write(&mut self, buf: &[u8]) -> std::io::Result { - let mut inner = self.inner.lock().unwrap(); - let col = inner.current_color.clone(); - inner - .messages - .push(PrintMessage::RustcErrTerm(RustcErrTermMessage::new( - buf.to_vec(), - col, - ))); - Ok(buf.len()) - } - - #[inline] - fn write_all(&mut self, buf: &[u8]) -> std::io::Result<()> { - self.write(buf)?; - Ok(()) - } - - #[inline] - fn flush(&mut self) -> std::io::Result<()> { - //println!("Flush"); - Ok(()) - } -} - -impl WriteColor for Printer { - #[inline] - fn supports_color(&self) -> bool { - self.inner.lock().unwrap().supports_color - } - - #[inline] - fn set_color(&mut self, spec: &ColorSpec) -> std::io::Result<()> { - self.inner.lock().unwrap().current_color = Some(spec.clone()); - Ok(()) - } - - #[inline] - fn reset(&mut self) -> std::io::Result<()> { - self.inner.lock().unwrap().current_color.take(); - Ok(()) - } -} - struct PrinterInner { color_setting: Color, current_color: Option, @@ -83,14 +40,19 @@ impl Printer { self.inner.lock().unwrap().messages.push(msg); } - pub fn dump(&self) -> Result<(), std::io::Error> { + /// Writes stored messages to respective outputs in order. + pub fn write_to_outputs(&self) -> Result<(), std::io::Error> { let inner = self.inner.lock().unwrap(); // Pretty common case, early exit if inner.messages.is_empty() { return Ok(()); } + + // Stdout term, from diffs let mut use_term_stdout = term::stdout().filter(|t| inner.color_setting.use_colored_tty() && t.supports_color()); + + // `rustc_error` diagnostics, compilation errors. let use_rustc_error_color = inner.color_setting.use_colored_tty() && term::stderr() .map(|t| t.supports_color()) @@ -140,6 +102,52 @@ impl Printer { } } +/// Trait evoked by `rustc_error`s `EmitterWriter` (`HumanEmitter` on main) to print +/// compilation errors and diffs. +impl Write for Printer { + fn write(&mut self, buf: &[u8]) -> std::io::Result { + let mut inner = self.inner.lock().unwrap(); + let col = inner.current_color.clone(); + inner + .messages + .push(PrintMessage::RustcErrTerm(RustcErrTermMessage::new( + buf.to_vec(), + col, + ))); + Ok(buf.len()) + } + + #[inline] + fn flush(&mut self) -> std::io::Result<()> { + Ok(()) + } + + #[inline] + fn write_all(&mut self, buf: &[u8]) -> std::io::Result<()> { + self.write(buf)?; + Ok(()) + } +} + +impl WriteColor for Printer { + #[inline] + fn supports_color(&self) -> bool { + self.inner.lock().unwrap().supports_color + } + + #[inline] + fn set_color(&mut self, spec: &ColorSpec) -> std::io::Result<()> { + self.inner.lock().unwrap().current_color = Some(spec.clone()); + Ok(()) + } + + #[inline] + fn reset(&mut self) -> std::io::Result<()> { + self.inner.lock().unwrap().current_color.take(); + Ok(()) + } +} + // Rustc vendors termcolor, but not everything needed to use it, // as far as I can tell fn rustc_colorspec_compat(rustc: &ColorSpec) -> termcolor::ColorSpec { @@ -149,10 +157,12 @@ fn rustc_colorspec_compat(rustc: &ColorSpec) -> termcolor::ColorSpec { let bg = rustc.bg().and_then(rustc_color_compat); cs.set_bg(bg); cs.set_bold(rustc.bold()); - cs.set_strikethrough(rustc.strikethrough()); - cs.set_underline(rustc.underline()); cs.set_intense(rustc.intense()); + cs.set_underline(rustc.underline()); + cs.set_dimmed(rustc.dimmed()); cs.set_italic(rustc.italic()); + cs.set_reset(rustc.reset()); + cs.set_strikethrough(rustc.strikethrough()); cs } @@ -218,31 +228,6 @@ pub enum PrintMessage { RustcErrTerm(RustcErrTermMessage), } -impl std::fmt::Debug for PrintMessage { - fn fmt(&self, f: &mut Formatter<'_>) -> std::fmt::Result { - match self { - PrintMessage::Stdout(buf) => f.write_fmt(format_args!( - "PrintMessage::Stdout({:?})", - core::str::from_utf8(buf) - )), - PrintMessage::StdErr(buf) => f.write_fmt(format_args!( - "PrintMessage::Stderr({:?})", - core::str::from_utf8(buf) - )), - PrintMessage::Term(msg) => f.write_fmt(format_args!( - "PrintMessage::Term({:?}, {:?})", - core::str::from_utf8(&msg.message), - msg.color - )), - PrintMessage::RustcErrTerm(msg) => f.write_fmt(format_args!( - "PrintMessage::RustcErrTerm({:?}, {:?})", - core::str::from_utf8(&msg.message), - msg.color - )), - } - } -} - pub struct TermMessage { message: Vec, color: Option, diff --git a/src/test/mod.rs b/src/test/mod.rs index b564b3e78c2..c6252168270 100644 --- a/src/test/mod.rs +++ b/src/test/mod.rs @@ -171,7 +171,7 @@ fn verify_config_test_names() { fn write_message(msg: &str) { let print = Printer::new(Color::Auto); buf_term_println!(print, None, "{msg}"); - print.dump().unwrap(); + print.write_to_outputs().unwrap(); } // Integration tests. The files in `tests/source` are formatted and compared From eb78a340a0c377715d89e37bb140c947167f09a3 Mon Sep 17 00:00:00 2001 From: MarcusGrass Date: Mon, 26 Feb 2024 09:24:58 +0100 Subject: [PATCH 19/23] Clean up a bit more --- src/emitter/files.rs | 17 ++++++++--------- src/lib.rs | 2 +- src/parse/session.rs | 2 -- 3 files changed, 9 insertions(+), 12 deletions(-) diff --git a/src/emitter/files.rs b/src/emitter/files.rs index 376c1ba1915..aaea82c86e0 100644 --- a/src/emitter/files.rs +++ b/src/emitter/files.rs @@ -1,24 +1,23 @@ use super::*; -use crate::buf_println; use std::fs; #[derive(Default)] -pub(crate) struct FilesEmitter<'a> { - print_misformatted_file_names: Option<&'a Printer>, +pub(crate) struct FilesEmitter { + print_misformatted_file_names: bool, } -impl<'a> FilesEmitter<'a> { - pub(crate) fn new(print_misformatted_file_names: Option<&'a Printer>) -> Self { +impl FilesEmitter { + pub(crate) fn new(print_misformatted_file_names: bool) -> Self { Self { print_misformatted_file_names, } } } -impl<'a> Emitter for FilesEmitter<'a> { +impl Emitter for FilesEmitter { fn emit_formatted_file( &mut self, - _output: &mut dyn Write, + output: &mut dyn Write, FormattedFile { filename, original_text, @@ -29,8 +28,8 @@ impl<'a> Emitter for FilesEmitter<'a> { let filename = ensure_real_path(filename); if original_text != formatted_text { fs::write(filename, formatted_text)?; - if let Some(printer) = self.print_misformatted_file_names { - buf_println!(printer, "{}", filename.display()); + if self.print_misformatted_file_names { + writeln!(output, "{}", filename.display())?; } } Ok(EmitterResult::default()) diff --git a/src/lib.rs b/src/lib.rs index 642675563a9..123b6e6642f 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -529,7 +529,7 @@ pub(crate) fn create_emitter<'a>(config: &Config, printer: &'a Printer) -> Box Box::new(emitter::FilesEmitter::new( - config.print_misformatted_file_names().then_some(printer), + config.print_misformatted_file_names(), )), EmitMode::Stdout | EmitMode::Coverage => { Box::new(emitter::IntoOutputEmitter::new(config.verbose())) diff --git a/src/parse/session.rs b/src/parse/session.rs index bfd4551656f..4e1a41a14a1 100644 --- a/src/parse/session.rs +++ b/src/parse/session.rs @@ -124,7 +124,6 @@ fn default_dcx( ignore_path_set: Lrc, can_reset: Lrc, show_parse_errors: bool, - _color: Color, printer: &Printer, ) -> DiagCtxt { let emitter = if !show_parse_errors { @@ -162,7 +161,6 @@ impl ParseSess { Lrc::clone(&ignore_path_set), Lrc::clone(&can_reset_errors), config.show_parse_errors(), - config.color(), printer, ); let parse_sess = RawParseSess::with_dcx(dcx, source_map); From bdf8291d157cf19aa53f6a23f6bceb3771b47a73 Mon Sep 17 00:00:00 2001 From: MarcusGrass Date: Mon, 26 Feb 2024 09:44:07 +0100 Subject: [PATCH 20/23] Write more on progress --- Progress.md | 48 +++++++++++++++++++++++++++++++++++++++++++----- 1 file changed, 43 insertions(+), 5 deletions(-) diff --git a/Progress.md b/Progress.md index 98e88547371..0302ac7a510 100644 --- a/Progress.md +++ b/Progress.md @@ -8,11 +8,10 @@ This is non-trivial when internals can run threaded, eagerly printing cannot hap to be passed back through buffers. Even that would be non-trivial if there wasn't color-term writing happening making things complicated. -## Rustc termcolor vendoring - -A big problem is the way that `rustc` vendors termcolor. It vendors some types but no printing facilities, -and the vendored code isn't interoperable with the published `termcolor` library, which means that -either the printing facilities need to be reinvented (no), or some compatibility-code needs to be introduced (yes). +The change became pretty big, even though the only substantive change +is in `print.rs` and `bin/main.rs`, because the printer has to be propagated +deep into the library, about half of the changes are adding a function argument, and reformatting +caused by those lines becoming too long. ## Changes @@ -26,3 +25,42 @@ In practice there are four printing facilities used: 4. Term stderr, this is done by `rustc_error` and the most complex to integrate. Additionally, these four facilities can't be separated, since they have to preserve order between each other. + +### Rename StdoutEmitter + +Confusing naming, it doesn't output into `stdout`, but into the +provided buffer. + +## Pros + +This change brings a substantial speedup, especially for large projects, the speedup +becoming less and less but not none or negative for smaller projects. + +If instant measures as the average reaction time of 250ms, this brings +down tokio from the not-instant 271ms, to the instant 86ms. + +Giving space for projects almost 3x tokio's size to have an "instant" formatting experience. + +## Drawbacks + +There are some drawbacks to consider + +### Late printing memory increase + +All messages that would have gone into `stdout` or `stderr` now go +into an in-mem buffer. This causes a memory-overhead which scales with project size +(number of files). For a large project, this is a difference of +`117M` on the master version, and `170M` on this version. + +### Rustc termcolor vendoring + +A problem is the way that `rustc` vendors termcolor. It vendors some types but no printing facilities, +and the vendored code isn't interoperable with the published `termcolor` library, which means that +either the printing facilities need to be reinvented (no), or some compatibility-code needs to be introduced (yes). +Hopefully there's some better solution to this, but it's liveable at the moment. + +### Accidentally messing with outputs will cause an output jumble + +This change would make any direct interaction with `stdout` and `stderr` in the +formatting codepath a bug. Generally, printing deep inside a library is considered bad for +imo, but now that would be upgraded to a bug. From 9505f435a4f07427576dc8a1475f4f62ae2d31a5 Mon Sep 17 00:00:00 2001 From: MarcusGrass Date: Mon, 26 Feb 2024 11:46:44 +0100 Subject: [PATCH 21/23] Leave attrs.rs, move merge md into PR --- Progress.md | 66 ---------------------------------- config_proc_macro/src/attrs.rs | 6 +--- 2 files changed, 1 insertion(+), 71 deletions(-) delete mode 100644 Progress.md diff --git a/Progress.md b/Progress.md deleted file mode 100644 index 0302ac7a510..00000000000 --- a/Progress.md +++ /dev/null @@ -1,66 +0,0 @@ -# Multithreaded rewrite progress - -Pretty difficult, high risk of messy code. - -There is only one real hurdle, getting stdout and stderr printing to be synchronized and backwards compatible. - -This is non-trivial when internals can run threaded, eagerly printing cannot happen, messages have -to be passed back through buffers. Even that would be non-trivial if there wasn't color-term writing happening -making things complicated. - -The change became pretty big, even though the only substantive change -is in `print.rs` and `bin/main.rs`, because the printer has to be propagated -deep into the library, about half of the changes are adding a function argument, and reformatting -caused by those lines becoming too long. - -## Changes - -### Output facilities - -In practice there are four printing facilities used: - -1. Regular `stdout`, pretty easy, replace `println` with printing into a buffer. -2. Regular `stderr`, same as above in all ways that matter. -3. Term stdout, this happens in the `diff`-printing part of the code. -4. Term stderr, this is done by `rustc_error` and the most complex to integrate. - -Additionally, these four facilities can't be separated, since they have to preserve order between each other. - -### Rename StdoutEmitter - -Confusing naming, it doesn't output into `stdout`, but into the -provided buffer. - -## Pros - -This change brings a substantial speedup, especially for large projects, the speedup -becoming less and less but not none or negative for smaller projects. - -If instant measures as the average reaction time of 250ms, this brings -down tokio from the not-instant 271ms, to the instant 86ms. - -Giving space for projects almost 3x tokio's size to have an "instant" formatting experience. - -## Drawbacks - -There are some drawbacks to consider - -### Late printing memory increase - -All messages that would have gone into `stdout` or `stderr` now go -into an in-mem buffer. This causes a memory-overhead which scales with project size -(number of files). For a large project, this is a difference of -`117M` on the master version, and `170M` on this version. - -### Rustc termcolor vendoring - -A problem is the way that `rustc` vendors termcolor. It vendors some types but no printing facilities, -and the vendored code isn't interoperable with the published `termcolor` library, which means that -either the printing facilities need to be reinvented (no), or some compatibility-code needs to be introduced (yes). -Hopefully there's some better solution to this, but it's liveable at the moment. - -### Accidentally messing with outputs will cause an output jumble - -This change would make any direct interaction with `stdout` and `stderr` in the -formatting codepath a bug. Generally, printing deep inside a library is considered bad for -imo, but now that would be upgraded to a bug. diff --git a/config_proc_macro/src/attrs.rs b/config_proc_macro/src/attrs.rs index e7534b813d7..d8de9aae088 100644 --- a/config_proc_macro/src/attrs.rs +++ b/config_proc_macro/src/attrs.rs @@ -68,11 +68,7 @@ fn get_name_value_str_lit(attr: &syn::Attribute, name: &str) -> Option { match &attr.meta { syn::Meta::NameValue(syn::MetaNameValue { path, - value: - syn::Expr::Lit(syn::ExprLit { - lit: syn::Lit::Str(lit_str), - .. - }), + value: syn::Expr::Lit(syn::ExprLit { lit: syn::Lit::Str(lit_str), .. }), .. }) if path.is_ident(name) => Some(lit_str.value()), _ => None, From 1928408f96611f244400ae7ad23cb37527056f75 Mon Sep 17 00:00:00 2001 From: MarcusGrass Date: Mon, 26 Feb 2024 13:16:37 +0100 Subject: [PATCH 22/23] Clean up imports --- src/bin/main.rs | 9 +++------ 1 file changed, 3 insertions(+), 6 deletions(-) diff --git a/src/bin/main.rs b/src/bin/main.rs index ea6c2958785..b285152d668 100644 --- a/src/bin/main.rs +++ b/src/bin/main.rs @@ -17,14 +17,11 @@ use std::str::FromStr; use std::thread::JoinHandle; use std::{env, panic}; -use getopts::{Matches, Options}; -use rustfmt_nightly::print::Printer; -use rustfmt_nightly::{buf_eprintln, buf_println}; - use crate::rustfmt::{ - load_config, CliOptions, Color, Config, Edition, EmitMode, FileLines, FileName, - FormatReportFormatterBuilder, Input, Session, Verbosity, + buf_eprintln, buf_println, load_config, print::Printer, CliOptions, Color, Config, Edition, + EmitMode, FileLines, FileName, FormatReportFormatterBuilder, Input, Session, Verbosity, }; +use getopts::{Matches, Options}; const BUG_REPORT_URL: &str = "https://github.com/rust-lang/rustfmt/issues/new?labels=bug"; From de08ccd20e6b84cfcdad3915343ae689573611b9 Mon Sep 17 00:00:00 2001 From: MarcusGrass Date: Mon, 26 Feb 2024 13:52:51 +0100 Subject: [PATCH 23/23] Comment, import, and minor code cleanup --- src/bin/main.rs | 2 +- src/emitter/files.rs | 2 +- src/git-rustfmt/main.rs | 5 +++-- src/print.rs | 31 ++++++++++++++++--------------- 4 files changed, 21 insertions(+), 19 deletions(-) diff --git a/src/bin/main.rs b/src/bin/main.rs index b285152d668..cce29a7840f 100644 --- a/src/bin/main.rs +++ b/src/bin/main.rs @@ -327,7 +327,7 @@ fn format( let parallelism = std::thread::available_parallelism().unwrap_or(NonZeroUsize::MIN); // Use a channel + map to get 'next-completed' thread, rather than - // waiting on the chronologically first handle to join, if there are more files + // waiting on the chronologically first handle to join, impactful if there are more files // than available parallelism. let (send, recv) = std::sync::mpsc::channel(); let mut handles: HashMap> = HashMap::new(); diff --git a/src/emitter/files.rs b/src/emitter/files.rs index aaea82c86e0..6360b73ee61 100644 --- a/src/emitter/files.rs +++ b/src/emitter/files.rs @@ -1,7 +1,7 @@ use super::*; use std::fs; -#[derive(Default)] +#[derive(Debug, Default)] pub(crate) struct FilesEmitter { print_misformatted_file_names: bool, } diff --git a/src/git-rustfmt/main.rs b/src/git-rustfmt/main.rs index f328caff448..ff29d6b41ea 100644 --- a/src/git-rustfmt/main.rs +++ b/src/git-rustfmt/main.rs @@ -9,10 +9,11 @@ use std::str::FromStr; use getopts::{Matches, Options}; use rustfmt_nightly as rustfmt; -use rustfmt_nightly::print::Printer; use tracing_subscriber::EnvFilter; -use crate::rustfmt::{load_config, CliOptions, FormatReportFormatterBuilder, Input, Session}; +use crate::rustfmt::{ + load_config, print::Printer, CliOptions, FormatReportFormatterBuilder, Input, Session, +}; fn prune_files(files: Vec<&str>) -> Vec<&str> { let prefixes: Vec<_> = files diff --git a/src/print.rs b/src/print.rs index f506634d2d9..e857f51f64c 100644 --- a/src/print.rs +++ b/src/print.rs @@ -47,18 +47,19 @@ impl Printer { if inner.messages.is_empty() { return Ok(()); } - - // Stdout term, from diffs - let mut use_term_stdout = - term::stdout().filter(|t| inner.color_setting.use_colored_tty() && t.supports_color()); - - // `rustc_error` diagnostics, compilation errors. - let use_rustc_error_color = inner.color_setting.use_colored_tty() - && term::stderr() - .map(|t| t.supports_color()) - .unwrap_or_default(); - let mut rustc_err_out = - use_rustc_error_color.then_some(StandardStream::stderr(ColorChoice::Always)); + let (mut diff_term_stdout, mut rustc_term_stderr) = inner + .color_setting + .use_colored_tty() + .then(|| { + ( + term::stdout().filter(|t| t.supports_color()), + term::stderr().and_then(|t| { + t.supports_color() + .then_some(StandardStream::stderr(ColorChoice::Always)) + }), + ) + }) + .unwrap_or_default(); for msg in &inner.messages { match msg { PrintMessage::Stdout(out) => { @@ -68,7 +69,7 @@ impl Printer { stderr().write_all(err)?; } PrintMessage::Term(t_msg) => { - if let Some(t) = &mut use_term_stdout { + if let Some(t) = &mut diff_term_stdout { if let Some(col) = t_msg.color { t.fg(col).unwrap() } @@ -81,12 +82,12 @@ impl Printer { } } PrintMessage::RustcErrTerm(msg) => { - if let Some(t) = &mut rustc_err_out { + if let Some(t) = &mut rustc_term_stderr { if let Some(col) = msg.color.as_ref().map(rustc_colorspec_compat) { t.set_color(&col)?; } t.write_all(&msg.message)?; - if msg.color.is_some() { + if msg.color.as_ref().map(|cs| cs.reset()).unwrap_or_default() { t.reset().unwrap(); } } else {