Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat(transformer/class-properties): exit faster from super replacement visitor #8028

Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
74 changes: 38 additions & 36 deletions crates/oxc_transformer/src/es2022/class_properties/constructor.rs
Original file line number Diff line number Diff line change
Expand Up @@ -682,30 +682,43 @@ impl<'a, 'c> ConstructorBodySuperReplacer<'a, 'c> {
mut self,
constructor: &mut Function<'a>,
) -> (ScopeId, InstanceInitsInsertLocation<'a>) {
let body_stmts = &mut constructor.body.as_mut().unwrap().statements;
let mut body_stmts_iter = body_stmts.iter_mut();

loop {
let mut body_stmts_iter_enumerated = body_stmts_iter.by_ref().enumerate();
if let Some((index, stmt)) = body_stmts_iter_enumerated.next() {
// This is not a real loop. It always breaks on 1st iteration.
// Only here so that can break out of it from within inner `for` loop.
#[expect(clippy::never_loop)]
'outer: loop {
let body_stmts = &mut constructor.body.as_mut().unwrap().statements;
for (index, stmt) in body_stmts.iter_mut().enumerate() {
// If statement is standalone `super()`, insert inits after `super()`.
// We can avoid a `_super` function for this common case.
if let Statement::ExpressionStatement(expr_stmt) = &*stmt {
if let Expression::CallExpression(call_expr) = &expr_stmt.expression {
if call_expr.callee.is_super() {
let insert_location =
InstanceInitsInsertLocation::ExistingConstructor(index + 1);
return (self.constructor_scope_id, insert_location);
if let Statement::ExpressionStatement(expr_stmt) = stmt {
if let Expression::CallExpression(call_expr) = &mut expr_stmt.expression {
if let Expression::Super(super_) = &call_expr.callee {
// Found `super()` as top-level statement
if self.super_binding.is_none() {
// This is the first `super()` found.
// So can just insert initializers after it - no need for `_super` function.
let insert_location =
InstanceInitsInsertLocation::ExistingConstructor(index + 1);
return (self.constructor_scope_id, insert_location);
}

// `super()` was previously found in nested position before this.
// So we do need a `_super` function.
// But we don't need to look any further for any other `super()` calls,
// because calling `super()` after this would be an immediate error.
let span = super_.span;
self.replace_super(call_expr, span);

break 'outer;
}
}
}

// Traverse statement looking for `super()` deeper in the statement
self.visit_statement(stmt);
if self.super_binding.is_some() {
break;
}
} else {
}

if self.super_binding.is_none() {
// No `super()` anywhere in constructor.
// This is weird, but legal code. It would be a runtime error if the class is constructed
// (unless the constructor returns early).
Expand All @@ -716,34 +729,23 @@ impl<'a, 'c> ConstructorBodySuperReplacer<'a, 'c> {
// What we get is completely legal output and correct `Semantic`, just longer than it
// could be. But this should very rarely happen in practice, and minifier will delete
// the `_super` function as dead code.
// So set `super_binding` and exit the loop, so it's treated as if `super()` was found
// in a nested position.
// TODO: Delete the initializers instead.
let super_func_scope_id = self.create_super_func_scope();
let super_binding = self.create_super_binding();
let insert_location =
InstanceInitsInsertLocation::SuperFnInsideConstructor(super_binding);
return (super_func_scope_id, insert_location);
self.super_binding = Some(self.create_super_binding());
}
}

// `super()` found in nested position. There may be more `super()`s in constructor.
// Convert them all to `_super()`.
for stmt in body_stmts_iter {
self.visit_statement(stmt);
break;
}

let super_func_scope_id = self.create_super_func_scope();
let super_binding = self.super_binding.unwrap();
let insert_location = InstanceInitsInsertLocation::SuperFnInsideConstructor(super_binding);
(super_func_scope_id, insert_location)
}

/// Create scope for `_super` function inside constructor body
fn create_super_func_scope(&mut self) -> ScopeId {
self.ctx.scopes_mut().add_scope(
let super_func_scope_id = self.ctx.scopes_mut().add_scope(
Some(self.constructor_scope_id),
NodeId::DUMMY,
ScopeFlags::Function | ScopeFlags::Arrow | ScopeFlags::StrictMode,
)
);
let super_binding = self.super_binding.unwrap();
let insert_location = InstanceInitsInsertLocation::SuperFnInsideConstructor(super_binding);
(super_func_scope_id, insert_location)
}
}

Expand Down
4 changes: 2 additions & 2 deletions tasks/transform_conformance/snapshots/oxc.snap.md
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
commit: 54a8389f

Passed: 114/130
Passed: 116/132

# All Passed:
* babel-plugin-transform-class-static-block
Expand All @@ -16,7 +16,7 @@ Passed: 114/130
* regexp


# babel-plugin-transform-class-properties (15/19)
# babel-plugin-transform-class-properties (17/21)
* static-super-assignment-target/input.js
x Output mismatch

Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
class C extends S {
prop = 1;
constructor() {
return {};
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
class C extends S {
constructor() {
var _super = (..._args) => (super(..._args), babelHelpers.defineProperty(this, "prop", 1), this);
return {};
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,46 @@
class C extends S {
prop = 1;
constructor() {
// Insert prop initializer after `super()`
super();
this.self = this;
}
}

class C2 extends S {
prop = 1;
constructor() {
class Inner extends S {
constructor() {
// Don't transform - different `super`
super();
}
}

// Insert prop initializer after `super()`
super();
this.self = this;
}
}

class C3 extends S {
prop = 1;
constructor() {
if (condition) {
// Transform to `_super()`
super(1, 2);
return this;
}

// Transform to `_super()`
super(3, 4);

if (condition2) {
// Don't transform
super(5, 6);
}

// Don't transform
super(7, 8);
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,47 @@
class C extends S {
constructor() {
// Insert prop initializer after `super()`
super();
babelHelpers.defineProperty(this, "prop", 1);
this.self = this;
}
}

class C2 extends S {
constructor() {
class Inner extends S {
constructor() {
// Don't transform - different `super`
super();
}
}

// Insert prop initializer after `super()`
super();
babelHelpers.defineProperty(this, "prop", 1);
this.self = this;
}
}

class C3 extends S {
constructor() {
var _super = (..._args) => (super(..._args), babelHelpers.defineProperty(this, "prop", 1), this);

if (condition) {
// Transform to `_super()`
_super(1, 2);
return this;
}

// Transform to `_super()`
_super(3, 4);

if (condition2) {
// Don't transform
super(5, 6);
}

// Don't transform
super(7, 8);
}
}
Loading