From 0d79ca617d7847420983de5b40fd81a503050800 Mon Sep 17 00:00:00 2001 From: Devon Govett Date: Sun, 4 Apr 2021 04:24:49 -0400 Subject: [PATCH] fix(es/transforms/optimization): Fix inlining nested block statements in branch simplifier (#1536) swc_ecma_transforms_optimization: - Fix branch remover. --- .../optimization/src/simplify/branch/mod.rs | 39 ++++++++++++++++++- .../optimization/src/simplify/branch/tests.rs | 18 +++++++++ .../pass/issue-1328/case1/output/entry.js | 6 ++- 3 files changed, 59 insertions(+), 4 deletions(-) diff --git a/ecmascript/transforms/optimization/src/simplify/branch/mod.rs b/ecmascript/transforms/optimization/src/simplify/branch/mod.rs index caf1cb9f4812..87e2d51594b0 100644 --- a/ecmascript/transforms/optimization/src/simplify/branch/mod.rs +++ b/ecmascript/transforms/optimization/src/simplify/branch/mod.rs @@ -428,13 +428,15 @@ impl Fold for Remover { Stmt::Block(BlockStmt { span, stmts }) => { if stmts.is_empty() { Stmt::Empty(EmptyStmt { span }) - } else if stmts.len() == 1 && !is_block_scoped_stuff(&stmts[0]) { + } else if stmts.len() == 1 + && !is_block_scoped_stuff(&stmts[0]) + && stmt_depth(&stmts[0]) <= 1 + { stmts.into_iter().next().unwrap().fold_with(self) } else { Stmt::Block(BlockStmt { span, stmts }) } } - Stmt::Try(TryStmt { span, block, @@ -1532,3 +1534,36 @@ fn check_for_stopper(s: &[Stmt], only_conditional: bool) -> bool { v.visit_stmts(s, &Invalid { span: DUMMY_SP } as _); v.found } + +/// Finds the depth of statements without hitting a block +fn stmt_depth(s: &Stmt) -> u32 { + let mut depth = 0; + + match s { + // Stop when hitting a statement we know can't increase statement depth. + Stmt::Block(_) | Stmt::Labeled(_) | Stmt::Switch(_) | Stmt::Decl(_) | Stmt::Expr(_) => {} + // Take the max depth of if statements + Stmt::If(i) => { + depth += 1; + if let Some(alt) = &i.alt { + depth += std::cmp::max(stmt_depth(&i.cons), stmt_depth(&alt)); + } else { + depth += stmt_depth(&i.cons); + } + } + // These statements can have bodies without a wrapping block + Stmt::With(WithStmt { body, .. }) + | Stmt::While(WhileStmt { body, .. }) + | Stmt::DoWhile(DoWhileStmt { body, .. }) + | Stmt::For(ForStmt { body, .. }) + | Stmt::ForIn(ForInStmt { body, .. }) + | Stmt::ForOf(ForOfStmt { body, .. }) => { + depth += 1; + depth += stmt_depth(&body); + } + // All other statements increase the depth by 1 + _ => depth += 1, + } + + depth +} diff --git a/ecmascript/transforms/optimization/src/simplify/branch/tests.rs b/ecmascript/transforms/optimization/src/simplify/branch/tests.rs index 01d498eb6634..ce5272096dd8 100644 --- a/ecmascript/transforms/optimization/src/simplify/branch/tests.rs +++ b/ecmascript/transforms/optimization/src/simplify/branch/tests.rs @@ -1722,3 +1722,21 @@ c = 3; console.log(c);", ); } + +#[test] +fn nested_block_stmt() { + test( + "if (Date.now() < 0) { + for(let i = 0; i < 10; i++){ + if (Date.now() < 0) { + console.log(1); + } + } + } else { + console.log(2); + }", + "if (Date.now() < 0) { + for(let i = 0; i < 10; i++)if (Date.now() < 0) console.log(1); + } else console.log(2);", + ); +} diff --git a/spack/tests/pass/issue-1328/case1/output/entry.js b/spack/tests/pass/issue-1328/case1/output/entry.js index 4c9f5ec9ca7d..96bbf226d778 100644 --- a/spack/tests/pass/issue-1328/case1/output/entry.js +++ b/spack/tests/pass/issue-1328/case1/output/entry.js @@ -474,8 +474,10 @@ var load = __spack_require__.bind(void 0, function(module, exports) { this.method = "next"; this.arg = undefined; this.tryEntries.forEach(resetTryEntry); - if (!skipTempReset) for(var name in this)// Not sure about the optimal order of these conditions: - if (name.charAt(0) === "t" && hasOwn.call(this, name) && !isNaN(+name.slice(1))) this[name] = undefined; + if (!skipTempReset) { + for(var name in this)// Not sure about the optimal order of these conditions: + if (name.charAt(0) === "t" && hasOwn.call(this, name) && !isNaN(+name.slice(1))) this[name] = undefined; + } }, stop: function() { this.done = true;