Skip to content

Commit 87bc698

Browse files
authored
fix(es/minifier): Fix termination detection (#10741)
**Description:** `terminates()` of `swc_ecma_utils` was wrong. **Related issue:** - Closes #10720
1 parent 2ff78cb commit 87bc698

File tree

6 files changed

+99
-23
lines changed

6 files changed

+99
-23
lines changed

.changeset/warm-lamps-prove.md

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,6 @@
1+
---
2+
swc_core: patch
3+
swc_ecma_utils: patch
4+
---
5+
6+
fix(es/minifier): Fix termination detection
Lines changed: 19 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,19 @@
1+
export function someFn({
2+
someVal,
3+
shouldBreak
4+
}) {
5+
switch (someVal) {
6+
case 'one':
7+
// if there is a certain condition, we exit
8+
if (shouldBreak === 'break') {
9+
break
10+
}
11+
12+
return 1
13+
default:
14+
return 0
15+
}
16+
17+
// this code gets hit if we break
18+
return 11
19+
}
Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,12 @@
1+
export function someFn({ someVal, shouldBreak }) {
2+
switch(someVal){
3+
case 'one':
4+
// if there is a certain condition, we exit
5+
if ('break' === shouldBreak) break;
6+
return 1;
7+
default:
8+
return 0;
9+
}
10+
// this code gets hit if we break
11+
return 11;
12+
}

crates/swc_ecma_minifier/tests/fixture/next/wrap-contracts/output.js

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -25989,8 +25989,7 @@ class Zip {
2598925989
throw Error(`Support for ${contractDefinition.srcWasmLang} not implemented yet.`);
2599025990
}
2599125991
return this.logger.info(`WASM ${contractDefinition.srcWasmLang} handler created in ${benchmark.elapsed()}`), new WasmHandlerApi_1.WasmHandlerApi(swGlobal, contractDefinition, jsExports || wasmInstance.exports);
25992-
}
25993-
{
25992+
} else {
2599425993
this.logger.info('Creating handler for js contract', contractDefinition.txId);
2599525994
const normalizedSource = (0, normalize_source_1.normalizeContractSource)(contractDefinition.src, evaluationOptions.useVM2);
2599625995
if (!evaluationOptions.allowUnsafeClient && normalizedSource.includes('SmartWeave.unsafeClient')) throw Error('Using unsafeClient is not allowed by default. Use EvaluationOptions.allowUnsafeClient flag.');

crates/swc_ecma_minifier/tests/libs-size.snapshot.md

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,6 @@
99
| react.js | 70.45 KiB | 22.44 KiB | 8.04 KiB |
1010
| terser.js | 1.08 MiB | 446.68 KiB | 120.49 KiB |
1111
| three.js | 1.19 MiB | 630.83 KiB | 154.80 KiB |
12-
| typescript.js | 10.45 MiB | 3.17 MiB | 840.67 KiB |
12+
| typescript.js | 10.45 MiB | 3.17 MiB | 840.66 KiB |
1313
| victory.js | 2.30 MiB | 694.16 KiB | 154.24 KiB |
1414
| vue.js | 334.13 KiB | 113.72 KiB | 41.82 KiB |

crates/swc_ecma_utils/src/lib.rs

Lines changed: 60 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -467,24 +467,64 @@ pub trait StmtExt {
467467

468468
/// stmts contain top level return/break/continue/throw
469469
fn terminates(&self) -> bool {
470-
fn terminates_many(stmts: &[Stmt], allow_break: bool, allow_throw: bool) -> bool {
470+
fn terminates_many(
471+
stmts: &[Stmt],
472+
in_switch: bool,
473+
allow_break: bool,
474+
allow_throw: bool,
475+
) -> Result<bool, ()> {
471476
stmts
472477
.iter()
473478
.rev()
474-
.any(|s| terminates(s, allow_break, allow_throw))
475-
}
476-
477-
fn terminates(stmt: &Stmt, allow_break: bool, allow_throw: bool) -> bool {
478-
match stmt {
479-
Stmt::Break(_) => allow_break,
479+
.map(|s| terminates(s, in_switch, allow_break, allow_throw))
480+
.try_fold(false, |acc, x| x.map(|v| acc || v))
481+
}
482+
483+
fn terminates(
484+
stmt: &Stmt,
485+
in_switch: bool,
486+
allow_break: bool,
487+
allow_throw: bool,
488+
) -> Result<bool, ()> {
489+
Ok(match stmt {
490+
Stmt::Break(_) => {
491+
if in_switch {
492+
// In case of `break` in switch, we should stop the analysis because the
493+
// statements after `if (foo) break;` may not execute.
494+
//
495+
// So the `return 1` in
496+
//
497+
// ```js
498+
// switch (foo) {
499+
// case 1:
500+
// if (bar) break;
501+
// return 1;
502+
// default:
503+
// return 0;
504+
// }
505+
// ```
506+
//
507+
// may not execute and we should return `false`.
508+
return Err(());
509+
} else {
510+
allow_break
511+
}
512+
}
480513
Stmt::Throw(_) => allow_throw,
481514
Stmt::Continue(_) | Stmt::Return(_) => true,
482-
Stmt::Block(block) => terminates_many(&block.stmts, allow_break, allow_throw),
483-
Stmt::If(IfStmt {
484-
cons,
485-
alt: Some(alt),
486-
..
487-
}) => cons.terminates() && alt.terminates(),
515+
Stmt::Block(block) => {
516+
terminates_many(&block.stmts, in_switch, allow_break, allow_throw)?
517+
}
518+
Stmt::If(IfStmt { cons, alt, .. }) => {
519+
if let Some(alt) = alt {
520+
terminates(cons, in_switch, allow_break, allow_throw)?
521+
&& terminates(alt, in_switch, allow_break, allow_throw)?
522+
} else {
523+
terminates(cons, in_switch, allow_break, allow_throw)?;
524+
525+
false
526+
}
527+
}
488528
Stmt::Switch(s) => {
489529
let mut has_default = false;
490530
let mut has_non_empty_terminates = false;
@@ -495,12 +535,12 @@ pub trait StmtExt {
495535
}
496536

497537
if !case.cons.is_empty() {
498-
let t = terminates_many(&case.cons, false, allow_throw);
538+
let t = terminates_many(&case.cons, true, false, allow_throw)?;
499539

500540
if t {
501541
has_non_empty_terminates = true
502542
} else {
503-
return false;
543+
return Ok(false);
504544
}
505545
}
506546
}
@@ -509,17 +549,17 @@ pub trait StmtExt {
509549
}
510550
Stmt::Try(t) => {
511551
if let Some(h) = &t.handler {
512-
terminates_many(&t.block.stmts, allow_break, false)
513-
&& terminates_many(&h.body.stmts, allow_break, allow_throw)
552+
terminates_many(&t.block.stmts, in_switch, allow_break, false)?
553+
&& terminates_many(&h.body.stmts, in_switch, allow_break, allow_throw)?
514554
} else {
515-
terminates_many(&t.block.stmts, allow_break, allow_throw)
555+
terminates_many(&t.block.stmts, in_switch, allow_break, allow_throw)?
516556
}
517557
}
518558
_ => false,
519-
}
559+
})
520560
}
521561

522-
terminates(self.as_stmt(), true, true)
562+
terminates(self.as_stmt(), false, true, true) == Ok(true)
523563
}
524564

525565
fn may_have_side_effects(&self, ctx: ExprCtx) -> bool {

0 commit comments

Comments
 (0)