From 04cc821e4a3de60808cf5f7eaa8277f1a0c06a2b Mon Sep 17 00:00:00 2001 From: topecongiro Date: Sun, 24 Mar 2019 18:43:35 +0900 Subject: [PATCH 1/4] Add a test for #3465 --- tests/source/issue-3465.rs | 42 ++++++++++++++++++++++++++++++++++++++ tests/target/issue-3465.rs | 42 ++++++++++++++++++++++++++++++++++++++ 2 files changed, 84 insertions(+) create mode 100644 tests/source/issue-3465.rs create mode 100644 tests/target/issue-3465.rs diff --git a/tests/source/issue-3465.rs b/tests/source/issue-3465.rs new file mode 100644 index 00000000000..0bc95ad4616 --- /dev/null +++ b/tests/source/issue-3465.rs @@ -0,0 +1,42 @@ +fn main() { + ((((((((((((((((((((((((((((((((((((((((((0) + 1) + 1) + + 1) + + 1) + + 1) + + 1) + + 1) + + 1) + + 1) + + 1) + + 1) + + 1) + + 1) + + 1) + + 1) + + 1) + + 1) + + 1) + + 1) + + 1) + + 1) + + 1) + + 1) + + 1) + + 1) + + 1) + + 1) + + 1) + + 1) + + 1) + + 1) + + 1) + + 1) + + 1) + + 1) + + 1) + + 1) + + 1) + + 1) + + 1) + + 1); +} diff --git a/tests/target/issue-3465.rs b/tests/target/issue-3465.rs new file mode 100644 index 00000000000..9e2680f0fee --- /dev/null +++ b/tests/target/issue-3465.rs @@ -0,0 +1,42 @@ +fn main() { + ((((((((((((((((((((((((((((((((((((((((((0) + 1) + 1) + + 1) + + 1) + + 1) + + 1) + + 1) + + 1) + + 1) + + 1) + + 1) + + 1) + + 1) + + 1) + + 1) + + 1) + + 1) + + 1) + + 1) + + 1) + + 1) + + 1) + + 1) + + 1) + + 1) + + 1) + + 1) + + 1) + + 1) + + 1) + + 1) + + 1) + + 1) + + 1) + + 1) + + 1) + + 1) + + 1) + + 1) + + 1) + + 1); +} From abfb381189846923157f5edd5948f2c0f00002eb Mon Sep 17 00:00:00 2001 From: topecongiro Date: Sun, 24 Mar 2019 18:55:11 +0900 Subject: [PATCH 2/4] Avoid rewriting pairs mutiple times --- src/pairs.rs | 93 ++++++++++++++++++++++++++++------------------------ 1 file changed, 51 insertions(+), 42 deletions(-) diff --git a/src/pairs.rs b/src/pairs.rs index 421167ad6d7..49a77c102f6 100644 --- a/src/pairs.rs +++ b/src/pairs.rs @@ -34,19 +34,11 @@ pub(crate) fn rewrite_all_pairs( shape: Shape, context: &RewriteContext<'_>, ) -> Option { - // First we try formatting on one line. - if let Some(list) = expr.flatten(false) { - if let Some(r) = rewrite_pairs_one_line(&list, shape, context) { - return Some(r); - } - } - - // We can't format on line, so try many. When we flatten here we make sure - // to only flatten pairs with the same operator, that way we don't - // necessarily need one line per sub-expression, but we don't do anything - // too funny wrt precedence. - expr.flatten(true) - .and_then(|list| rewrite_pairs_multiline(&list, shape, context)) + expr.flatten(context, shape).and_then(|list| { + // First we try formatting on one line. + rewrite_pairs_one_line(&list, shape, context) + .or_else(|| rewrite_pairs_multiline(&list, shape, context)) + }) } // This may return a multi-line result since we allow the last expression to go @@ -61,22 +53,23 @@ fn rewrite_pairs_one_line( let mut result = String::new(); let base_shape = shape.block(); - for (e, s) in list.list.iter().zip(list.separators.iter()) { - let cur_shape = base_shape.offset_left(last_line_width(&result))?; - let rewrite = e.rewrite(context, cur_shape)?; + for ((_, rewrite), s) in list.list.iter().zip(list.separators.iter()) { + if let Some(rewrite) = rewrite { + if !is_single_line(&rewrite) || result.len() > shape.width { + return None; + } - if !is_single_line(&rewrite) || result.len() > shape.width { + result.push_str(&rewrite); + result.push(' '); + result.push_str(s); + result.push(' '); + } else { return None; } - - result.push_str(&rewrite); - result.push(' '); - result.push_str(s); - result.push(' '); } let prefix_len = result.len(); - let last = list.list.last().unwrap(); + let last = list.list.last()?.0; let cur_shape = base_shape.offset_left(last_line_width(&result))?; let last_rewrite = last.rewrite(context, cur_shape)?; result.push_str(&last_rewrite); @@ -112,10 +105,9 @@ fn rewrite_pairs_multiline( let indent_str = nested_shape.indent.to_string_with_newline(context.config); let mut result = String::new(); - let rewrite = list.list[0].rewrite(context, shape)?; - result.push_str(&rewrite); + result.push_str(&list.list[0].1.as_ref()?); - for (e, s) in list.list[1..].iter().zip(list.separators.iter()) { + for ((e, default_rw), s) in list.list[1..].iter().zip(list.separators.iter()) { // The following test checks if we should keep two subexprs on the same // line. We do this if not doing so would create an orphan and there is // enough space to do so. @@ -139,24 +131,20 @@ fn rewrite_pairs_multiline( } } - let nested_overhead = s.len() + 1; - let line_shape = match context.config.binop_separator() { + match context.config.binop_separator() { SeparatorPlace::Back => { result.push(' '); result.push_str(s); result.push_str(&indent_str); - nested_shape.sub_width(nested_overhead)? } SeparatorPlace::Front => { result.push_str(&indent_str); result.push_str(s); result.push(' '); - nested_shape.offset_left(nested_overhead)? } - }; + } - let rewrite = e.rewrite(context, line_shape)?; - result.push_str(&rewrite); + result.push_str(&default_rw.as_ref()?); } Some(result) } @@ -250,27 +238,46 @@ pub(crate) fn rewrite_pair( // A pair which forms a tree and can be flattened (e.g., binops). trait FlattenPair: Rewrite + Sized { - // If `_same_op` is `true`, then we only combine binops with the same - // operator into the list. E.g,, if the source is `a * b + c`, if `_same_op` - // is true, we make `[(a * b), c]` if `_same_op` is false, we make - // `[a, b, c]` - fn flatten(&self, _same_op: bool) -> Option> { + fn flatten(&self, _: &RewriteContext<'_>, _: Shape) -> Option> { None } } struct PairList<'a, 'b, T: Rewrite> { - list: Vec<&'b T>, + list: Vec<(&'b T, Option)>, separators: Vec<&'a str>, } impl FlattenPair for ast::Expr { - fn flatten(&self, same_op: bool) -> Option> { + fn flatten( + &self, + context: &RewriteContext<'_>, + shape: Shape, + ) -> Option> { let top_op = match self.node { ast::ExprKind::Binary(op, _, _) => op.node, _ => return None, }; + let default_rewrite = |node: &ast::Expr, sep: usize, is_first: bool| { + if is_first { + return node.rewrite(context, shape); + } + let nested_overhead = sep + 1; + let rhs_offset = shape.rhs_overhead(&context.config); + let nested_shape = (match context.config.indent_style() { + IndentStyle::Visual => shape.visual_indent(0), + IndentStyle::Block => shape.block_indent(context.config.tab_spaces()), + }) + .with_max_width(&context.config) + .sub_width(rhs_offset)?; + let default_shape = match context.config.binop_separator() { + SeparatorPlace::Back => nested_shape.sub_width(nested_overhead)?, + SeparatorPlace::Front => nested_shape.offset_left(nested_overhead)?, + }; + node.rewrite(context, default_shape) + }; + // Turn a tree of binop expressions into a list using a depth-first, // in-order traversal. let mut stack = vec![]; @@ -279,12 +286,14 @@ fn flatten(&self, same_op: bool) -> Option> { let mut node = self; loop { match node.node { - ast::ExprKind::Binary(op, ref lhs, _) if !same_op || op.node == top_op => { + ast::ExprKind::Binary(op, ref lhs, _) if op.node == top_op => { stack.push(node); node = lhs; } _ => { - list.push(node); + let op_len = separators.last().map_or(0, |s: &&str| s.len()); + let rw = default_rewrite(node, op_len, list.is_empty()); + list.push((node, rw)); if let Some(pop) = stack.pop() { match pop.node { ast::ExprKind::Binary(op, _, ref rhs) => { From 3357712dfae35dbf9a83f051348b59f9a21d8987 Mon Sep 17 00:00:00 2001 From: topecongiro Date: Sun, 24 Mar 2019 18:56:48 +0900 Subject: [PATCH 3/4] Increase stack size on ci This is required for tests/target/issue-3465.rs. --- .travis.yml | 2 +- appveyor.yml | 1 + 2 files changed, 2 insertions(+), 1 deletion(-) diff --git a/.travis.yml b/.travis.yml index 741103b93a4..d498415bdd6 100644 --- a/.travis.yml +++ b/.travis.yml @@ -45,7 +45,7 @@ script: - | if [ -z ${INTEGRATION} ]; then cargo build - cargo test + RUST_MIN_STACK=8388608 cargo test else ./ci/integration.sh fi diff --git a/appveyor.yml b/appveyor.yml index 87c1773ce8e..84043e93fb9 100644 --- a/appveyor.yml +++ b/appveyor.yml @@ -2,6 +2,7 @@ # and modified (mainly removal of deployment) to suit rustfmt. environment: + RUST_MIN_STACK: 8388608 global: PROJECT_NAME: rustfmt matrix: From a93139778722491d75dd55d53e786781de8ffdcd Mon Sep 17 00:00:00 2001 From: topecongiro Date: Sun, 24 Mar 2019 19:00:19 +0900 Subject: [PATCH 4/4] Move rust-clippy to allow_failures --- .travis.yml | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/.travis.yml b/.travis.yml index d498415bdd6..61a854dc216 100644 --- a/.travis.yml +++ b/.travis.yml @@ -28,12 +28,13 @@ matrix: - env: INTEGRATION=mdbook - env: INTEGRATION=packed_simd - env: INTEGRATION=rand - - env: INTEGRATION=rust-clippy - env: INTEGRATION=rust-semverver - env: INTEGRATION=stdsimd TARGET=x86_64-unknown-linux-gnu - env: INTEGRATION=tempdir - env: INTEGRATION=futures-rs allow_failures: + # Doesn't build - keep this in allow_failures as it's fragile to breaking changes of rustc. + - env: INTEGRATION=rust-clippy # Doesn't build - seems to be because of an option - env: INTEGRATION=packed_simd # Doesn't build - a temporal build failure due to breaking changes in the nightly compilre