mirror of
https://github.com/oxc-project/oxc.git
synced 2026-11-04 10:29:58 +00:00
fix(minifier): preserve guaranteed throws from class heritage evaluation (#24349)
Evaluating a class declaration can throw before any of its members matter. `class C extends C {}` reads its own name while it is still in the TDZ and always throws a ReferenceError. A heritage that is a `var`, or a lexical binding declared later, can be undefined or uninitialized at evaluation time and throws a TypeError. The minifier only checked for a literal arrow function as heritage, so removing an unused class could silently delete one of these guaranteed throws.
The removal check now unwraps parens and sequence wrappers before looking at the heritage, and keeps any class whose heritage identifier resolves to a class, `let`/`const`, or `var` binding. Declaration order cannot be proven in the middle of minification (transforms copy and move spans), so the rule is conservative on purpose. Function declarations, imports, and globals are initialized before any class can evaluate, so classes extending them can still be removed.
## Example
```js
var f = () => {};
class C extends f {}
```
Running this throws `TypeError: Class extends value ... is not a constructor`.
Before, the unused class was removed and the whole program minified to nothing, so the TypeError was gone. After, the output is unchanged and the throw is preserved:
```js
var f = () => {};
class C extends f {}
```
This fixes six test262 runtime failures that exist on main today (17279 -> 17285 positive passes): `in-extends-expression-grouped`, `in-extends-expression-assigned`, `superclass-arrow-function`, `prototype-getter`, `prototype-setter`, and `constructable-but-no-prototype`. The runtime suite executes the minified tests in node; the results were verified through monitor-oxc. Minified sizes are unchanged on every fixture.
Implemented with AI assistance (Claude Code); design, review, and verification driven by the author per the repo AI-usage policy.
This commit is contained in:
1 parent
b6d2a29e47
commit
7d3336365d
4 files changed
+86
-27
No files matched your search
@@ -12,7 +12,7 @@ use oxc_ecmascript::{
|
||||
side_effects::{MayHaveSideEffects, MayHaveSideEffectsContext},
|
||||
};
|
||||
use oxc_span::GetSpan;
|
||||
use oxc_syntax::symbol::SymbolId;
|
||||
use oxc_syntax::symbol::{SymbolFlags, SymbolId};
|
||||
|
||||
use super::PeepholeOptimizations;
|
||||
use super::fold_constants::is_cjs_module_exports_hint;
|
||||
@@ -1005,12 +1005,24 @@ impl<'a> PeepholeOptimizations {
|
||||
c: &mut Class<'a>,
|
||||
ctx: &mut TraverseCtx<'a>,
|
||||
) -> Option<ArenaVec<'a, Expression<'a>>> {
|
||||
// TypeError `class C extends (() => {}) {}`
|
||||
if c.super_class
|
||||
.as_ref()
|
||||
.is_some_and(|e| matches!(e, Expression::ArrowFunctionExpression(_)))
|
||||
{
|
||||
return None;
|
||||
if let Some(super_class) = &c.super_class {
|
||||
// Unwrap parens and sequence tails — `(0, x)` — so the check sees
|
||||
// the value the heritage actually evaluates to.
|
||||
let mut e = super_class.get_inner_expression();
|
||||
while let Expression::SequenceExpression(seq) = e {
|
||||
let Some(last) = seq.expressions.last() else { break };
|
||||
e = last.get_inner_expression();
|
||||
}
|
||||
match e {
|
||||
// TypeError `class C extends (() => {}) {}`
|
||||
Expression::ArrowFunctionExpression(_) => return None,
|
||||
Expression::Identifier(ident)
|
||||
if Self::heritage_may_be_uninitialized(ident, ctx) =>
|
||||
{
|
||||
return None;
|
||||
}
|
||||
_ => {}
|
||||
}
|
||||
}
|
||||
// Don't remove classes with decorators - they may have side effects
|
||||
if !c.decorators.is_empty() {
|
||||
@@ -1079,4 +1091,30 @@ impl<'a> PeepholeOptimizations {
|
||||
ctx.notice_change();
|
||||
Some(exprs)
|
||||
}
|
||||
|
||||
/// A heritage identifier can throw at class-evaluation time regardless
|
||||
/// of expression purity: the class's own name and any lexical binding
|
||||
/// declared later are in their TDZ (ReferenceError — test262
|
||||
/// class/name-binding/in-extends-expression.js), and a
|
||||
/// hoisted-but-unassigned `var` or missing parameter evaluates to
|
||||
/// `undefined` (TypeError: not a constructor). Reference order cannot be
|
||||
/// proven mid-minification (transforms copy and move spans), so any
|
||||
/// heritage resolving to a class, lexical, or `var` binding is
|
||||
/// conservatively unremovable. Hoisted plain function declarations and
|
||||
/// import bindings are initialized before evaluation and stay removable;
|
||||
/// unresolved identifiers keep flowing through the global side-effect
|
||||
/// machinery. The deeper fix — modeling potentially-uninitialized
|
||||
/// identifier reads — belongs in `oxc_ecmascript`'s side-effect layer,
|
||||
/// which currently treats every resolved identifier read as pure.
|
||||
fn heritage_may_be_uninitialized(
|
||||
ident: &IdentifierReference<'_>,
|
||||
ctx: &TraverseCtx<'a>,
|
||||
) -> bool {
|
||||
const UNINIT_RISK: SymbolFlags = SymbolFlags::Variable.union(SymbolFlags::Class);
|
||||
ident
|
||||
.reference_id
|
||||
.get()
|
||||
.and_then(|reference_id| ctx.scoping().get_reference(reference_id).symbol_id())
|
||||
.is_some_and(|symbol_id| ctx.scoping().symbol_flags(symbol_id).intersects(UNINIT_RISK))
|
||||
}
|
||||
}
|
||||
@@ -356,6 +356,45 @@ fn keep_in_script_mode() {
|
||||
test_options_source_type("class C {}", "class C {}", source_type, &options);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn keep_class_cycle_with_wrapped_arrow_heritage() {
|
||||
// The removal check must see the arrow heritage through a pure
|
||||
// sequence/paren wrapper, so the decision cannot change when a fold
|
||||
// surfaces the literal arrow between passes.
|
||||
test_smallest(
|
||||
"class A extends (0, () => {}) { m() { new B() } } class B { m() { new A() } } console.log(1);",
|
||||
"class A extends (() => {}) {\n\tm() {\n\t\tnew B();\n\t}\n}\nclass B {\n\tm() {\n\t\tnew A();\n\t}\n}\nconsole.log(1);",
|
||||
);
|
||||
// The single, fully-unused class is kept for the same reason a literal
|
||||
// arrow heritage is kept: evaluating it is a guaranteed TypeError.
|
||||
test_smallest("class C extends (0, () => {}) {}", "class C extends (() => {}) {}");
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn keep_class_with_tdz_or_undefined_heritage() {
|
||||
// test262 language/statements/class/name-binding/in-extends-expression.js:
|
||||
// the class's own name is in its TDZ while the heritage evaluates, so the
|
||||
// declaration is a guaranteed ReferenceError that must survive.
|
||||
test_same_smallest("class C extends C {}");
|
||||
// The test262 shape: the class lives in a callback whose call is live.
|
||||
// (A NAMED function wrapper additionally hits a pre-existing hole in the
|
||||
// pure-function model — `may_have_side_effects` does not model heritage
|
||||
// TDZ throws, so `f` reads as pure and the call is dropped on `main`
|
||||
// too; that is a separate `oxc_ecmascript` issue, not covered here.)
|
||||
test_same_smallest("g(function() {\n\tclass C extends C {}\n});");
|
||||
// The wrapped variant classifies through the same heritage unwrap.
|
||||
test_smallest("class C extends (0, C) {}", "class C extends C {}");
|
||||
// A forward lexical heritage also evaluates in its TDZ; reference order
|
||||
// cannot be proven mid-minification (transforms copy and move spans), so
|
||||
// any class/lexical/`var` heritage keeps the class.
|
||||
test_same_smallest(
|
||||
"class A extends B {\n\tm() {\n\t\tnew A();\n\t}\n}\nclass B {\n\tm() {\n\t\tnew A();\n\t}\n}",
|
||||
);
|
||||
// `var` heritage: a hoisted-but-unassigned binding is `undefined`, and
|
||||
// `extends undefined` is a TypeError.
|
||||
test_same_smallest("var B = class {};\nclass A extends B {\n\tm() {\n\t\tnew A();\n\t}\n}");
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn remove_unused_import_specifiers() {
|
||||
let options = CompressOptions::smallest();
|
||||
|
||||
@@ -2,7 +2,7 @@ commit: de8e621c
|
||||
|
||||
runtime Summary:
|
||||
AST Parsed : 18277/18277 (100.00%)
|
||||
Positive Passed: 17279/18277 (94.54%)
|
||||
Positive Passed: 17285/18277 (94.57%)
|
||||
minify Error: tasks/coverage/test262/test/language/computed-property-names/class/static/generator-prototype.js
|
||||
Test262Error: Expected a TypeError to be thrown but no exception was thrown at all
|
||||
|
||||
@@ -1629,21 +1629,12 @@ Test262Error: reject reason Expected SameValue(«TypeError: The iterator does no
|
||||
transform Error: tasks/coverage/test262/test/language/statements/class/async-gen-method-static/yield-star-sync-throw.js
|
||||
throw-arg-1
|
||||
|
||||
minify Error: tasks/coverage/test262/test/language/statements/class/definition/constructable-but-no-prototype.js
|
||||
Test262Error: Expected a TypeError to be thrown but no exception was thrown at all
|
||||
|
||||
minify Error: tasks/coverage/test262/test/language/statements/class/definition/getters-non-configurable-err.js
|
||||
Test262Error: Expected a TypeError to be thrown but no exception was thrown at all
|
||||
|
||||
minify Error: tasks/coverage/test262/test/language/statements/class/definition/invalid-extends.js
|
||||
Test262Error: Expected a TypeError to be thrown but no exception was thrown at all
|
||||
|
||||
minify Error: tasks/coverage/test262/test/language/statements/class/definition/prototype-getter.js
|
||||
Test262Error: Expected a TypeError to be thrown but no exception was thrown at all
|
||||
|
||||
minify Error: tasks/coverage/test262/test/language/statements/class/definition/prototype-setter.js
|
||||
Test262Error: Expected a TypeError to be thrown but no exception was thrown at all
|
||||
|
||||
minify Error: tasks/coverage/test262/test/language/statements/class/definition/setters-non-configurable-err.js
|
||||
Test262Error: Expected a TypeError to be thrown but no exception was thrown at all
|
||||
|
||||
@@ -2328,12 +2319,6 @@ Test262Error: Expected SameValue(«"_"», «"#field"») to be true
|
||||
transform Error: tasks/coverage/test262/test/language/statements/class/elements/static-field-init-with-this.js
|
||||
Test262Error: Expected SameValue(«"undefinedtest"», «"test262test"») to be true
|
||||
|
||||
minify Error: tasks/coverage/test262/test/language/statements/class/name-binding/in-extends-expression-assigned.js
|
||||
Test262Error: Expected a ReferenceError to be thrown but no exception was thrown at all
|
||||
|
||||
minify Error: tasks/coverage/test262/test/language/statements/class/name-binding/in-extends-expression-grouped.js
|
||||
Test262Error: Expected a ReferenceError to be thrown but no exception was thrown at all
|
||||
|
||||
minify Error: tasks/coverage/test262/test/language/statements/class/static-method-gen-non-configurable-err.js
|
||||
Test262Error: Expected a TypeError to be thrown but no exception was thrown at all
|
||||
|
||||
@@ -2346,9 +2331,6 @@ Test262Error: Expected a TypeError to be thrown but no exception was thrown at a
|
||||
codegen Error: tasks/coverage/test262/test/language/statements/class/subclass/private-class-field-on-nonextensible-return-override.js
|
||||
Test262Error: Expected a TypeError to be thrown but no exception was thrown at all
|
||||
|
||||
minify Error: tasks/coverage/test262/test/language/statements/class/subclass/superclass-arrow-function.js
|
||||
Test262Error: Expected a TypeError to be thrown but no exception was thrown at all
|
||||
|
||||
transform Error: tasks/coverage/test262/test/language/statements/class/subclass/superclass-async-function.js
|
||||
TypeError: Cannot redefine property: prototype
|
||||
|
||||
|
||||
@@ -12,5 +12,5 @@ antd.js | 6.69 MB || 1044 | 56 |
|
||||
|
||||
binder.ts | 193.08 kB || 33 | 1 || 7076 | 824
|
||||
|
||||
kitchen-sink.tsx | 732.90 kB || 2298 | 174 || 87997 | 15654
|
||||
kitchen-sink.tsx | 732.90 kB || 2309 | 175 || 88129 | 15654
|
||||
|
||||
Reference in new issue
Block a user