Skip to content

Commit a37d439

Browse files
August-ZAugust-Z
authored
refactor(es/minifier): simplify constant-false loop guards (#12293)
**Description:** The constant-false `do...while` guard has an `allow_break_continue` parameter whose value is `false` at every reachable call. Remove the unused parameter and simplify the recursive checks while preserving the guard's existing acceptance and rejection behavior. Add regression coverage for control-flow targets, hoisted bindings, condition side effects and exceptions, directives, parent rewrites, and compression option boundaries. These fixtures protect the existing conservative behavior when later transformations revisit the loop or its ancestors. The new opt-in Script checks execute the original source and each of two successive minifications against independent expectations, using the same compression and mangling options. They check stdout or typed completion values, with dedicated cases validating distinctions such as `undefined` versus a string and positive versus negative zero. **Validation:** Local validation on macOS aarch64 with Node v26.3.0: - `cargo fmt --all` - `cargo test -p swc_ecma_minifier`: 7,977 passed, 48 ignored. - `cargo test -p swc_ecma_minifier --test compress`: independently rerun; 3,074 passed, 27 ignored. - `cargo test -p swc_ecma_minifier --features concurrent,debug --test compress do_while_false`: 58 passed. - `(cd crates/swc_ecma_minifier && ./scripts/exec.sh)`: 487 exec tests and 2,128 terser_exec tests passed. - `cargo clippy -p swc_ecma_minifier --all-targets -- -D warnings` The code and tests are identical to those used for these checks. The local `cargo clippy --all --all-targets -- -D warnings` run failed on macOS because of the unchanged `noexec_mount_in` dead-code warning in `crates/swc_native_addon/src/platform/unix.rs:117`. Its production caller is Linux-only. GitHub Actions has independently passed the [CI run](https://github.com/swc-project/swc/actions/runs/34212998202) for the current PR head, including the minifier crate tests on the PR merge commit and the configured Linux, macOS, Windows, and Wasm checks. The [CodSpeed report for the current PR head](#12293 (comment)) reports no significant performance changes across 200 compared benchmarks in CPU Simulation. It uses fallback main commit `d602f58`; the 61 skipped entries are historical benchmark identifiers already unregistered at that baseline. The current SWC and minifier benchmark targets completed their measurements and uploaded their data. **Related issue (if exists):** Follow-up to #12160.
1 parent 4b4c135 commit a37d439

134 files changed

Lines changed: 1861 additions & 42 deletions

File tree

Some content is hidden

Large Commits have some content hidden by default. Use the searchbox below for content that may be hidden.

‎.changeset/simplify-constant-false-loop-guards.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_minifier: patch
4+
---
5+
6+
refactor(es/minifier): Simplify constant-false loop guards

‎crates/swc_ecma_minifier/src/compress/pure/loops.rs‎

Lines changed: 10 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -265,8 +265,10 @@ impl Pure<'_> {
265265
let val = stmt.test.as_pure_bool(self.expr_ctx);
266266
if let Value::Known(false) = val {
267267
// A direct break or continue targets this loop and cannot survive
268-
// unwrapping.
269-
if should_not_inline_loop_body(&stmt.body, false) {
268+
// unwrapping. Even removing a terminal jump along with the loop
269+
// can expose its ancestors to unsafe purity or directive
270+
// simplifications, including during a later minification.
271+
if should_not_inline_loop_body(&stmt.body) {
270272
return;
271273
}
272274

@@ -308,33 +310,23 @@ fn optimize_loop_body(loop_body: &mut Stmt) {
308310
}
309311
}
310312

311-
fn should_not_inline_loop_body(s: &Stmt, allow_break_continue: bool) -> bool {
313+
fn should_not_inline_loop_body(s: &Stmt) -> bool {
312314
match s {
313-
Stmt::Block(s) => s
314-
.stmts
315-
.iter()
316-
.any(|s| should_not_inline_loop_body(s, allow_break_continue)),
315+
Stmt::Block(s) => s.stmts.iter().any(should_not_inline_loop_body),
317316

318317
Stmt::If(s) => {
319-
should_not_inline_loop_body(&s.cons, false)
318+
should_not_inline_loop_body(&s.cons)
320319
|| s.alt
321320
.as_deref()
322-
.map(|s| should_not_inline_loop_body(s, false))
321+
.map(should_not_inline_loop_body)
323322
.unwrap_or_default()
324323
}
325324
Stmt::Switch(s) => s
326325
.cases
327326
.iter()
328-
.any(|c| c.cons.iter().any(|s| should_not_inline_loop_body(s, false))),
329-
330-
Stmt::Continue(ContinueStmt {
331-
label: Some(..), ..
332-
})
333-
| Stmt::Break(BreakStmt {
334-
label: Some(..), ..
335-
}) => true,
327+
.any(|c| c.cons.iter().any(should_not_inline_loop_body)),
336328

337-
Stmt::Break(..) | Stmt::Continue(..) => !allow_break_continue,
329+
Stmt::Break(..) | Stmt::Continue(..) => true,
338330

339331
Stmt::Return(..)
340332
| Stmt::Throw(..)

‎crates/swc_ecma_minifier/tests/compress.rs‎

Lines changed: 145 additions & 24 deletions
Original file line numberDiff line numberDiff line change
@@ -21,7 +21,7 @@ use swc_common::{
2121
input::SourceFileInput,
2222
sync::Lrc,
2323
util::take::Take,
24-
EqIgnoreSpan, FileName, Mark, SourceMap,
24+
EqIgnoreSpan, FileName, Mark, SourceFile, SourceMap,
2525
};
2626
use swc_ecma_ast::*;
2727
use swc_ecma_codegen::{
@@ -144,14 +144,26 @@ fn run(
144144
comments: Option<&dyn Comments>,
145145
mangle: Option<TestMangleOptions>,
146146
skip_hygiene: bool,
147+
) -> Option<Program> {
148+
let fm = cm.load_file(input).expect("failed to load input.js");
149+
run_with_source(cm, handler, fm, config, comments, mangle, skip_hygiene)
150+
}
151+
152+
/// Use the same parser and optimizer for fixture files and re-minified output.
153+
fn run_with_source(
154+
cm: Lrc<SourceMap>,
155+
handler: &Handler,
156+
fm: Lrc<SourceFile>,
157+
config: &str,
158+
comments: Option<&dyn Comments>,
159+
mangle: Option<TestMangleOptions>,
160+
skip_hygiene: bool,
147161
) -> Option<Program> {
148162
HANDLER.set(handler, || {
149163
let disable_hygiene = mangle.is_some() || skip_hygiene;
150164

151165
let (module, mut config) = parse_compressor_config(cm.clone(), config);
152166

153-
let fm = cm.load_file(input).expect("failed to load input.js");
154-
155167
eprintln!("---- {} -----\n{}", Color::Green.paint("Input"), fm.src);
156168

157169
if env::var("SWC_RUN").unwrap_or_default() == "1" {
@@ -250,11 +262,7 @@ fn run(
250262
},
251263
);
252264
let end = Instant::now();
253-
tracing::info!(
254-
"optimize({}) took {:?}",
255-
input.display(),
256-
end - optimization_start
257-
);
265+
tracing::info!("optimize({}) took {:?}", fm.name, end - optimization_start);
258266

259267
if !disable_hygiene {
260268
output.visit_mut_with(&mut hygiene())
@@ -263,11 +271,7 @@ fn run(
263271
let output = output.apply(&mut fixer(None));
264272

265273
let end = Instant::now();
266-
tracing::info!(
267-
"process({}) took {:?}",
268-
input.display(),
269-
end - minification_start
270-
);
274+
tracing::info!("process({}) took {:?}", fm.name, end - minification_start);
271275

272276
Some(output)
273277
})
@@ -306,6 +310,19 @@ fn find_config(dir: &Path) -> String {
306310
panic!("failed to find config file for {}", dir.display())
307311
}
308312

313+
fn read_mangle_config(dir: &Path) -> Option<TestMangleOptions> {
314+
let mangle = read_to_string(dir.join("mangle.json")).ok();
315+
if let Some(mangle) = &mangle {
316+
eprintln!(
317+
"---- {} -----\n{}",
318+
Color::Green.paint("Mangle config"),
319+
mangle
320+
);
321+
}
322+
323+
mangle.map(|s| serde_json::from_str(&s).expect("failed to deserialize mangle.json"))
324+
}
325+
309326
#[testing::fixture("tests/fixture/**/input.js")]
310327
#[testing::fixture("tests/pass-1/**/input.js")]
311328
#[testing::fixture("tests/pass-default/**/input.js")]
@@ -317,17 +334,7 @@ fn custom_fixture(input: PathBuf) {
317334
testing::run_test2(false, |cm, handler| {
318335
let comments = SingleThreadedComments::default();
319336

320-
let mangle = dir.join("mangle.json");
321-
let mangle = read_to_string(mangle).ok();
322-
if let Some(mangle) = &mangle {
323-
eprintln!(
324-
"---- {} -----\n{}",
325-
Color::Green.paint("Mangle config"),
326-
mangle
327-
);
328-
}
329-
let mangle: Option<TestMangleOptions> =
330-
mangle.map(|s| serde_json::from_str(&s).expect("failed to deserialize mangle.json"));
337+
let mangle = read_mangle_config(dir);
331338

332339
let output = run(
333340
cm.clone(),
@@ -368,6 +375,120 @@ fn custom_fixture(input: PathBuf) {
368375
.unwrap()
369376
}
370377

378+
/// Check Script completion values without wrapping the fixture in a function.
379+
/// Expected values include the primitive type and distinguish NaN and negative
380+
/// zero. Check the original and two minifications independently: a retained
381+
/// block can protect one invocation while exposing a regression in the next
382+
/// one.
383+
#[testing::fixture("tests/fixture/**/expected.completion")]
384+
fn script_completion(expected: PathBuf) {
385+
check_script_fixture(expected, ScriptExpectation::Completion);
386+
}
387+
388+
/// Opt in to independent execution checks before and after two minifications.
389+
/// The regular fixture test continues to check the first output snapshot.
390+
#[testing::fixture("tests/fixture/**/expected.repeat-stdout")]
391+
fn script_repeated_stdout(expected: PathBuf) {
392+
check_script_fixture(expected, ScriptExpectation::Stdout);
393+
}
394+
395+
#[derive(Clone, Copy)]
396+
enum ScriptExpectation {
397+
Completion,
398+
Stdout,
399+
}
400+
401+
fn check_script_fixture(expected: PathBuf, expectation: ScriptExpectation) {
402+
let dir = expected.parent().unwrap();
403+
let input = dir.join("input.js");
404+
let config = find_config(dir);
405+
let mangle = read_mangle_config(dir);
406+
let expected = read_to_string(expected).expect("failed to read expected Script result");
407+
let mut source = read_to_string(&input).expect("failed to read input.js");
408+
409+
testing::run_test2(false, |cm, handler| {
410+
for round in 0..=2 {
411+
if round != 0 {
412+
let comments = SingleThreadedComments::default();
413+
let fm = cm.new_source_file(
414+
FileName::Custom(format!("{} (round {round})", input.display())).into(),
415+
source,
416+
);
417+
let output = run_with_source(
418+
cm.clone(),
419+
&handler,
420+
fm,
421+
&config,
422+
Some(&comments),
423+
mangle.clone(),
424+
false,
425+
)
426+
.expect("failed to optimize Script fixture");
427+
assert!(output.is_script(), "Script fixtures must parse as Scripts");
428+
source = print(cm.clone(), &[output], Some(&comments), true, true);
429+
}
430+
431+
eprintln!("---- Script round {round} -----\n{source}");
432+
let actual = match expectation {
433+
ScriptExpectation::Completion => script_completion_of(&source),
434+
ScriptExpectation::Stdout => exec_node_js(
435+
&source,
436+
JsExecOptions {
437+
cache: false,
438+
..Default::default()
439+
},
440+
),
441+
}
442+
.expect("failed to execute Script fixture");
443+
assert_eq!(
444+
DebugUsingDisplay(&actual),
445+
DebugUsingDisplay(&expected),
446+
"Script round {round}: {}",
447+
input.display()
448+
);
449+
}
450+
451+
Ok(())
452+
})
453+
.unwrap()
454+
}
455+
456+
fn script_completion_of(source: &str) -> Result<String, Error> {
457+
let source = serde_json::to_string(source).expect("failed to serialize Script source");
458+
exec_node_js(
459+
&format!(
460+
r#"
461+
const value = require('node:vm').runInNewContext({source});
462+
const type = value === null ? 'null' : typeof value;
463+
let result;
464+
switch (type) {{
465+
case 'undefined':
466+
case 'null':
467+
result = {{ type }};
468+
break;
469+
case 'number':
470+
result = {{ type, value: Object.is(value, -0) ? '-0' : String(value) }};
471+
break;
472+
case 'bigint':
473+
result = {{ type, value: String(value) }};
474+
break;
475+
case 'boolean':
476+
case 'string':
477+
result = {{ type, value }};
478+
break;
479+
default:
480+
throw new Error('Unsupported Script completion type: ' + type);
481+
}}
482+
console.log(JSON.stringify(result));
483+
"#
484+
),
485+
JsExecOptions {
486+
cache: false,
487+
..Default::default()
488+
},
489+
)
490+
}
491+
371492
#[derive(Default)]
372493
struct NonFiniteLiteralValidator {
373494
nan_count: usize,

‎crates/swc_ecma_minifier/tests/fixture/issues/do-while-false-hoisted-bindings/config.json‎

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,6 @@
1+
{
2+
"dead_code": true,
3+
"evaluate": true,
4+
"loops": true,
5+
"passes": 2
6+
}

‎crates/swc_ecma_minifier/tests/fixture/issues/do-while-false-hoisted-bindings/expected.stdout‎

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,4 @@
1+
true
2+
true
3+
true
4+
outer

‎crates/swc_ecma_minifier/tests/fixture/issues/do-while-false-hoisted-bindings/input.js‎

Lines changed: 33 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,33 @@
1+
var shared = "outer";
2+
3+
function directReadAfterBreak() {
4+
do {
5+
break;
6+
var local = "unreachable";
7+
} while (false);
8+
return local;
9+
}
10+
11+
function shadowAfterContinue() {
12+
do {
13+
continue;
14+
var shared = "unreachable";
15+
} while (false);
16+
return shared;
17+
}
18+
19+
function capturedBinding() {
20+
var read = function () {
21+
return shared;
22+
};
23+
do {
24+
break;
25+
var shared = "unreachable";
26+
} while (false);
27+
return read;
28+
}
29+
30+
console.log(directReadAfterBreak() === undefined);
31+
console.log(shadowAfterContinue() === undefined);
32+
console.log(capturedBinding()() === undefined);
33+
console.log(shared);

‎crates/swc_ecma_minifier/tests/fixture/issues/do-while-false-hoisted-bindings/output.js‎

Lines changed: 29 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,29 @@
1+
var shared = "outer";
2+
function directReadAfterBreak() {
3+
do {
4+
var local;
5+
break;
6+
}while (false)
7+
return local;
8+
}
9+
function shadowAfterContinue() {
10+
do {
11+
var shared;
12+
continue;
13+
}while (false)
14+
return shared;
15+
}
16+
function capturedBinding() {
17+
var read = function() {
18+
return shared;
19+
};
20+
do {
21+
var shared;
22+
break;
23+
}while (false)
24+
return read;
25+
}
26+
console.log(void 0 === directReadAfterBreak());
27+
console.log(void 0 === shadowAfterContinue());
28+
console.log(void 0 === capturedBinding()());
29+
console.log(shared);

‎crates/swc_ecma_minifier/tests/fixture/issues/do-while-false-terminal-jump/completion/assignment-break/expected.completion‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1 @@
1+
{"type":"number","value":"2"}

‎crates/swc_ecma_minifier/tests/fixture/issues/do-while-false-terminal-jump/completion/assignment-break/input.js‎

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,6 @@
1+
var value;
2+
value = 1;
3+
do {
4+
value = 2;
5+
break;
6+
} while (false);

‎crates/swc_ecma_minifier/tests/fixture/issues/do-while-false-terminal-jump/completion/assignment-break/output.js‎

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,6 @@
1+
var value;
2+
value = 1;
3+
do {
4+
value = 2;
5+
break;
6+
}while (false)

0 commit comments

Comments
 (0)