From 094e687e42a19b747203fa1967d192c5384ef459 Mon Sep 17 00:00:00 2001 From: Shotaro Yamada Date: Tue, 9 Oct 2018 21:28:40 +0900 Subject: [PATCH 1/7] Remove an unsafe code --- src/visitor.rs | 10 +++------- 1 file changed, 3 insertions(+), 7 deletions(-) diff --git a/src/visitor.rs b/src/visitor.rs index 30752826a85..7b952c20f52 100644 --- a/src/visitor.rs +++ b/src/visitor.rs @@ -792,13 +792,9 @@ impl<'b, 'a: 'b> FmtVisitor<'a> { where F: Fn(&RewriteContext) -> Option, { - let result; - let macro_rewrite_failure = { - let context = self.get_context(); - result = f(&context); - unsafe { *context.macro_rewrite_failure.as_ptr() } - }; - self.macro_rewrite_failure |= macro_rewrite_failure; + let context = self.get_context(); + let result = f(&context); + self.macro_rewrite_failure |= *context.macro_rewrite_failure.borrow(); result } From d55729987ff798b49838e5b5707cfca807b5b6b7 Mon Sep 17 00:00:00 2001 From: Shotaro Yamada Date: Tue, 9 Oct 2018 21:47:00 +0900 Subject: [PATCH 2/7] Use UnOp::to_string --- src/expr.rs | 7 +------ 1 file changed, 1 insertion(+), 6 deletions(-) diff --git a/src/expr.rs b/src/expr.rs index 752fb792899..48a493c8a74 100644 --- a/src/expr.rs +++ b/src/expr.rs @@ -1815,12 +1815,7 @@ fn rewrite_unary_op( shape: Shape, ) -> Option { // For some reason, an UnOp is not spanned like BinOp! - let operator_str = match op { - ast::UnOp::Deref => "*", - ast::UnOp::Not => "!", - ast::UnOp::Neg => "-", - }; - rewrite_unary_prefix(context, operator_str, expr, shape) + rewrite_unary_prefix(context, ast::UnOp::to_string(op), expr, shape) } fn rewrite_assignment( From 751bcf5fa08ee5c5ba156e43c7f8327f7968b02f Mon Sep 17 00:00:00 2001 From: Shotaro Yamada Date: Mon, 15 Oct 2018 20:36:39 +0900 Subject: [PATCH 3/7] Clippy --- src/format-diff/main.rs | 2 +- src/items.rs | 19 +++++++------------ src/lists.rs | 2 +- src/macros.rs | 4 ++-- src/overflow.rs | 2 +- src/source_map.rs | 12 +++++++----- src/utils.rs | 2 +- src/visitor.rs | 33 ++++++++++++++++----------------- 8 files changed, 36 insertions(+), 40 deletions(-) diff --git a/src/format-diff/main.rs b/src/format-diff/main.rs index d509a4429b4..aacfa12d437 100644 --- a/src/format-diff/main.rs +++ b/src/format-diff/main.rs @@ -83,7 +83,7 @@ fn main() { ); if let Err(e) = run(&opts) { - println!("{}", opts.usage(&format!("{}", e))); + println!("{}", opts.usage(&e.to_string())); process::exit(1); } } diff --git a/src/items.rs b/src/items.rs index 1673bffbce9..b35e8ea4587 100644 --- a/src/items.rs +++ b/src/items.rs @@ -1543,7 +1543,7 @@ pub fn rewrite_struct_field_prefix( rewrite_ident(context, name), type_annotation_spacing.0 ), - None => format!("{}", vis), + None => vis.to_string(), }) } @@ -2004,18 +2004,13 @@ fn rewrite_fn_base( one_line_budget, multi_line_budget, arg_indent ); + result.push('('); // Check if vertical layout was forced. - if one_line_budget == 0 { - if snuggle_angle_bracket { - result.push('('); - } else { - result.push_str("("); - if context.config.indent_style() == IndentStyle::Visual { - result.push_str(&arg_indent.to_string_with_newline(context.config)); - } - } - } else { - result.push('('); + if one_line_budget == 0 + && !snuggle_angle_bracket + && context.config.indent_style() == IndentStyle::Visual + { + result.push_str(&arg_indent.to_string_with_newline(context.config)); } // Skip `pub(crate)`. diff --git a/src/lists.rs b/src/lists.rs index 57f778896d3..92990fe9a17 100644 --- a/src/lists.rs +++ b/src/lists.rs @@ -50,7 +50,7 @@ impl<'a> ListFormatting<'a> { ends_with_newline: true, preserve_newline: false, nested: false, - config: config, + config, } } diff --git a/src/macros.rs b/src/macros.rs index dab27d8a073..1e668cf1d87 100644 --- a/src/macros.rs +++ b/src/macros.rs @@ -1073,7 +1073,7 @@ fn next_space(tok: &Token) -> SpaceState { /// when the macro is not an instance of try! (or parsing the inner expression /// failed). pub fn convert_try_mac(mac: &ast::Mac, context: &RewriteContext) -> Option { - if &format!("{}", mac.node.path) == "try" { + if &mac.node.path.to_string() == "try" { let ts: TokenStream = mac.node.tts.clone().into(); let mut parser = new_parser_from_tts(context.parse_session, ts.trees().collect()); @@ -1491,5 +1491,5 @@ fn rewrite_macro_with_items( result.push_str(&shape.indent.to_string_with_newline(context.config)); result.push_str(closer); result.push_str(trailing_semicolon); - return Some(result); + Some(result) } diff --git a/src/overflow.rs b/src/overflow.rs index fc1803b2ab6..053ce1b9212 100644 --- a/src/overflow.rs +++ b/src/overflow.rs @@ -425,7 +425,7 @@ impl<'a> Context<'a> { _ => expr.rewrite(self.context, shape), } } - item @ _ => item.rewrite(self.context, shape), + item => item.rewrite(self.context, shape), }; if let Some(rewrite) = rewrite { diff --git a/src/source_map.rs b/src/source_map.rs index dee5e4f7601..ed081adcd86 100644 --- a/src/source_map.rs +++ b/src/source_map.rs @@ -51,11 +51,13 @@ impl<'a> SpanUtils for SnippetProvider<'a> { } fn span_before(&self, original: Span, needle: &str) -> BytePos { - self.opt_span_before(original, needle).expect(&format!( - "bad span: {}: {}", - needle, - self.span_to_snippet(original).unwrap() - )) + self.opt_span_before(original, needle).unwrap_or_else(|| { + panic!( + "bad span: {}: {}", + needle, + self.span_to_snippet(original).unwrap() + ) + }) } fn opt_span_after(&self, original: Span, needle: &str) -> Option { diff --git a/src/utils.rs b/src/utils.rs index 7ef9254a482..2ff3b7e525a 100644 --- a/src/utils.rs +++ b/src/utils.rs @@ -45,7 +45,7 @@ pub fn is_same_visibility(a: &Visibility, b: &Visibility) -> bool { ( VisibilityKind::Restricted { path: p, .. }, VisibilityKind::Restricted { path: q, .. }, - ) => format!("{}", p) == format!("{}", q), + ) => p.to_string() == q.to_string(), (VisibilityKind::Public, VisibilityKind::Public) | (VisibilityKind::Inherited, VisibilityKind::Inherited) | ( diff --git a/src/visitor.rs b/src/visitor.rs index 7b952c20f52..63e2c6bdcac 100644 --- a/src/visitor.rs +++ b/src/visitor.rs @@ -112,7 +112,7 @@ impl<'b, 'a: 'b> FmtVisitor<'a> { if contains_skip(get_attrs_from_stmt(stmt)) { self.push_skipped_with_span(stmt.span()); } else { - let shape = self.shape().clone(); + let shape = self.shape(); let rewrite = self.with_context(|ctx| stmt.rewrite(&ctx, shape)); self.push_rewrite(stmt.span(), rewrite) } @@ -367,13 +367,13 @@ impl<'b, 'a: 'b> FmtVisitor<'a> { let where_span_end = snippet .find_uncommented("{") .map(|x| BytePos(x as u32) + source!(self, item.span).lo()); - let block_indent = self.block_indent.clone(); + let block_indent = self.block_indent; let rw = self.with_context(|ctx| format_impl(&ctx, item, block_indent, where_span_end)); self.push_rewrite(item.span, rw); } ast::ItemKind::Trait(..) => { - let block_indent = self.block_indent.clone(); + let block_indent = self.block_indent; let rw = self.with_context(|ctx| format_trait(&ctx, item, block_indent)); self.push_rewrite(item.span, rw); } @@ -652,20 +652,19 @@ impl<'b, 'a: 'b> FmtVisitor<'a> { ErrorKind::DeprecatedAttr, )], ); - } else if attr.path.segments[0].ident.to_string() == "rustfmt" { - if attr.path.segments.len() == 1 - || attr.path.segments[1].ident.to_string() != "skip" - { - let file_name = self.source_map.span_to_filename(attr.span).into(); - self.report.append( - file_name, - vec![FormattingError::from_span( - attr.span, - &self.source_map, - ErrorKind::BadAttr, - )], - ); - } + } else if attr.path.segments[0].ident.to_string() == "rustfmt" + && (attr.path.segments.len() == 1 + || attr.path.segments[1].ident.to_string() != "skip") + { + let file_name = self.source_map.span_to_filename(attr.span).into(); + self.report.append( + file_name, + vec![FormattingError::from_span( + attr.span, + &self.source_map, + ErrorKind::BadAttr, + )], + ); } } if contains_skip(attrs) { From 6380f885278e33ff058e01555538ced58c44dacf Mon Sep 17 00:00:00 2001 From: Shotaro Yamada Date: Wed, 10 Oct 2018 10:50:25 +0900 Subject: [PATCH 4/7] Cleanup --- src/missed_spans.rs | 18 ++---------------- src/string.rs | 4 ++-- 2 files changed, 4 insertions(+), 18 deletions(-) diff --git a/src/missed_spans.rs b/src/missed_spans.rs index f2f2295a872..b7d73bbf0ba 100644 --- a/src/missed_spans.rs +++ b/src/missed_spans.rs @@ -120,7 +120,7 @@ impl<'a> FmtVisitor<'a> { } fn push_vertical_spaces(&mut self, mut newline_count: usize) { - let offset = self.count_trailing_newlines(); + let offset = self.buffer.chars().rev().take_while(|c| *c == '\n').count(); let newline_upper_bound = self.config.blank_lines_upper_bound() + 1; let newline_lower_bound = self.config.blank_lines_lower_bound() + 1; @@ -142,16 +142,6 @@ impl<'a> FmtVisitor<'a> { self.push_str(&blank_lines); } - fn count_trailing_newlines(&self) -> usize { - let mut buf = &*self.buffer; - let mut result = 0; - while buf.ends_with('\n') { - buf = &buf[..buf.len() - 1]; - result += 1; - } - result - } - fn write_snippet(&mut self, span: Span, process_last_snippet: F) where F: Fn(&mut FmtVisitor, &str, &str), @@ -271,11 +261,7 @@ impl<'a> FmtVisitor<'a> { if let Some('/') = subslice.chars().nth(1) { // check that there are no contained block comments - if !subslice - .split('\n') - .map(|s| s.trim_left()) - .any(|s| s.len() >= 2 && &s[0..2] == "/*") - { + if !subslice.lines().any(|s| s.trim_left().starts_with("/*")) { // Add a newline after line comments self.push_str("\n"); } diff --git a/src/string.rs b/src/string.rs index ca063123e39..b70f9f0f12b 100644 --- a/src/string.rs +++ b/src/string.rs @@ -242,9 +242,9 @@ fn break_string(max_chars: usize, trim_end: bool, line_end: &str, input: &[&str] for (i, grapheme) in input[0..=index].iter().enumerate() { if is_line_feed(grapheme) { if i <= index_minus_ws { - let mut line = input[0..i].join(""); + let mut line = &input[0..i].join("")[..]; if trim_end { - line = line.trim_right().to_string(); + line = line.trim_right(); } return SnippetState::EndWithLineFeed(format!("{}\n", line), i + 1); } From 6fb188bd43840f4a99c6a4b4cdbdb21ccf3304e7 Mon Sep 17 00:00:00 2001 From: Shotaro Yamada Date: Mon, 15 Oct 2018 21:10:34 +0900 Subject: [PATCH 5/7] Use concat() instead of join("") --- src/string.rs | 28 ++++++++-------------------- 1 file changed, 8 insertions(+), 20 deletions(-) diff --git a/src/string.rs b/src/string.rs index b70f9f0f12b..d45f15a1c9b 100644 --- a/src/string.rs +++ b/src/string.rs @@ -172,7 +172,7 @@ fn detect_url(s: &[&str], index: usize) -> Option { if s.len() < start + 8 { return None; } - let prefix = s[start..start + 8].join(""); + let prefix = s[start..start + 8].concat(); if prefix.starts_with("https://") || prefix.starts_with("http://") || prefix.starts_with("ftp://") @@ -242,7 +242,7 @@ fn break_string(max_chars: usize, trim_end: bool, line_end: &str, input: &[&str] for (i, grapheme) in input[0..=index].iter().enumerate() { if is_line_feed(grapheme) { if i <= index_minus_ws { - let mut line = &input[0..i].join("")[..]; + let mut line = &input[0..i].concat()[..]; if trim_end { line = line.trim_right(); } @@ -256,7 +256,7 @@ fn break_string(max_chars: usize, trim_end: bool, line_end: &str, input: &[&str] for (i, grapheme) in input[index + 1..].iter().enumerate() { if !trim_end && is_line_feed(grapheme) { return SnippetState::EndWithLineFeed( - input[0..=index + 1 + i].join("").to_string(), + input[0..=index + 1 + i].concat(), index + 2 + i, ); } else if not_whitespace_except_line_feed(grapheme) { @@ -266,15 +266,9 @@ fn break_string(max_chars: usize, trim_end: bool, line_end: &str, input: &[&str] } if trim_end { - SnippetState::LineEnd( - input[0..=index_minus_ws].join("").to_string(), - index_plus_ws + 1, - ) + SnippetState::LineEnd(input[0..=index_minus_ws].concat(), index_plus_ws + 1) } else { - SnippetState::LineEnd( - input[0..=index_plus_ws].join("").to_string(), - index_plus_ws + 1, - ) + SnippetState::LineEnd(input[0..=index_plus_ws].concat(), index_plus_ws + 1) } }; @@ -297,15 +291,9 @@ fn break_string(max_chars: usize, trim_end: bool, line_end: &str, input: &[&str] .position(|grapheme| not_whitespace_except_line_feed(grapheme)) .unwrap_or(0); return if trim_end { - SnippetState::LineEnd( - input[..=url_index_end].join("").to_string(), - index_plus_ws + 1, - ) + SnippetState::LineEnd(input[..=url_index_end].concat(), index_plus_ws + 1) } else { - return SnippetState::LineEnd( - input[..=index_plus_ws].join("").to_string(), - index_plus_ws + 1, - ); + return SnippetState::LineEnd(input[..=index_plus_ws].concat(), index_plus_ws + 1); }; } match input[0..max_chars] @@ -330,7 +318,7 @@ fn break_string(max_chars: usize, trim_end: bool, line_end: &str, input: &[&str] // A boundary was found after the line limit Some(index) => break_at(max_chars + index), // No boundary to the right, the input cannot be broken - None => SnippetState::EndOfInput(input.join("").to_string()), + None => SnippetState::EndOfInput(input.concat()), }, }, } From 6c964fd0303041066381204328d4368b5c29d3f7 Mon Sep 17 00:00:00 2001 From: Shotaro Yamada Date: Mon, 15 Oct 2018 21:20:03 +0900 Subject: [PATCH 6/7] Reduce allocations --- src/comment.rs | 6 +++--- src/config/config_type.rs | 2 +- src/items.rs | 4 ++-- 3 files changed, 6 insertions(+), 6 deletions(-) diff --git a/src/comment.rs b/src/comment.rs index 0f2409be1c6..561989a1a11 100644 --- a/src/comment.rs +++ b/src/comment.rs @@ -764,12 +764,12 @@ fn trim_custom_comment_prefix(s: &str) -> String { // due to comment wrapping, a line that was originaly behind `#` is split over // multiple lines, which needs then to be prefixed with a `#` if !orig.trim_left().starts_with("# ") { - format!("# {}", orig) + Cow::from(format!("# {}", orig)) } else { - orig.to_string() + Cow::from(orig) } } else { - line.to_string() + Cow::from(line) } }) .collect::>() diff --git a/src/config/config_type.rs b/src/config/config_type.rs index edd6ae5634f..291a59e1055 100644 --- a/src/config/config_type.rs +++ b/src/config/config_type.rs @@ -112,7 +112,7 @@ macro_rules! create_config { cloned.width_heuristics = None; ::toml::to_string(&cloned) - .map_err(|e| format!("Could not output config: {}", e.to_string())) + .map_err(|e| format!("Could not output config: {}", e)) } } diff --git a/src/items.rs b/src/items.rs index b35e8ea4587..19caafbfb82 100644 --- a/src/items.rs +++ b/src/items.rs @@ -779,7 +779,7 @@ pub fn format_impl( let outer_indent_str = offset.block_only().to_string_with_newline(context.config); result.push_str(&inner_indent_str); - result.push_str(visitor.buffer.to_string().trim()); + result.push_str(visitor.buffer.trim()); result.push_str(&outer_indent_str); } else if need_newline || !context.config.empty_item_single_line() { result.push_str(&sep); @@ -1137,7 +1137,7 @@ pub fn format_trait(context: &RewriteContext, item: &ast::Item, offset: Indent) let inner_indent_str = visitor.block_indent.to_string_with_newline(context.config); result.push_str(&inner_indent_str); - result.push_str(visitor.buffer.to_string().trim()); + result.push_str(visitor.buffer.trim()); result.push_str(&outer_indent_str); } else if result.contains('\n') { result.push_str(&outer_indent_str); From e64a6d371b498ad2187ec4937c144b1bf83d0659 Mon Sep 17 00:00:00 2001 From: Shotaro Yamada Date: Mon, 15 Oct 2018 23:50:01 +0900 Subject: [PATCH 7/7] Cargo.lock --- Cargo.lock | 9 +++++++++ 1 file changed, 9 insertions(+) diff --git a/Cargo.lock b/Cargo.lock index be908dac466..53890b57fe1 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -67,6 +67,9 @@ source = "registry+https://github.com/rust-lang/crates.io-index" name = "bytecount" version = "0.3.2" source = "registry+https://github.com/rust-lang/crates.io-index" +dependencies = [ + "simd 0.2.3 (registry+https://github.com/rust-lang/crates.io-index)", +] [[package]] name = "byteorder" @@ -674,6 +677,11 @@ dependencies = [ "serde 1.0.79 (registry+https://github.com/rust-lang/crates.io-index)", ] +[[package]] +name = "simd" +version = "0.2.3" +source = "registry+https://github.com/rust-lang/crates.io-index" + [[package]] name = "smallvec" version = "0.6.5" @@ -918,6 +926,7 @@ dependencies = [ "checksum serde 1.0.79 (registry+https://github.com/rust-lang/crates.io-index)" = "84257ccd054dc351472528c8587b4de2dbf0dc0fe2e634030c1a90bfdacebaa9" "checksum serde_derive 1.0.79 (registry+https://github.com/rust-lang/crates.io-index)" = "31569d901045afbff7a9479f793177fe9259819aff10ab4f89ef69bbc5f567fe" "checksum serde_json 1.0.32 (registry+https://github.com/rust-lang/crates.io-index)" = "43344e7ce05d0d8280c5940cabb4964bea626aa58b1ec0e8c73fa2a8512a38ce" +"checksum simd 0.2.3 (registry+https://github.com/rust-lang/crates.io-index)" = "0048b17eb9577ac545c61d85c3559b41dfb4cbea41c9bd9ca6a4f73ff05fda84" "checksum smallvec 0.6.5 (registry+https://github.com/rust-lang/crates.io-index)" = "153ffa32fd170e9944f7e0838edf824a754ec4c1fc64746fcc9fe1f8fa602e5d" "checksum stable_deref_trait 1.1.1 (registry+https://github.com/rust-lang/crates.io-index)" = "dba1a27d3efae4351c8051072d619e3ade2820635c3958d826bfea39d59b54c8" "checksum syn 0.14.9 (registry+https://github.com/rust-lang/crates.io-index)" = "261ae9ecaa397c42b960649561949d69311f08eeaea86a65696e6e46517cf741"