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.

Sign up for GitHub

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

refactor(transformer/class-properties): use stack of ClassDetails #7947

Merged
merged 1 commit into from
Dec 17, 2024
Merged

refactor(transformer/class-properties): use stack of ClassDetails #7947

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
104 changes: 40 additions & 64 deletions crates/oxc_transformer/src/es2022/class_properties/class.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -12,13 +12,12 @@ use oxc_syntax::{
};
use oxc_traverse::{BoundIdentifier, TraverseCtx};

use crate::common::helper_loader::Helper;
use crate::{common::helper_loader::Helper, TransformCtx};

use super::{
constructor::InstanceInitsInsertLocation,
private_props::{PrivateProp, PrivateProps},
utils::{create_assignment, create_variable_declaration, exprs_into_stmts},
ClassBindings, ClassProperties, FxIndexMap,
ClassBindings, ClassDetails, ClassProperties, FxIndexMap, PrivateProp,
};

impl<'a, 'ctx> ClassProperties<'a, 'ctx> {
Expand Down Expand Up @@ -63,10 +62,8 @@ impl<'a, 'ctx> ClassProperties<'a, 'ctx> {

// Return number of expressions to be inserted before/after the class
let mut expr_count = self.insert_before.len() + self.insert_after_exprs.len();

let private_props = self.private_props_stack.last();
if let Some(private_props) = private_props {
expr_count += private_props.props.len();
if let Some(private_props) = &self.current_class().private_props {
expr_count += private_props.len();
}

if expr_count > 0 {
Expand Down Expand Up @@ -99,32 +96,32 @@ impl<'a, 'ctx> ClassProperties<'a, 'ctx> {
// TODO: Deduct static private props from `expr_count`.
// Or maybe should store count and increment it when create private static props?
// They're probably pretty rare, so it'll be rarely used.
expr_count += 1 + usize::from(self.class_bindings.temp.is_some());
let class_details = self.classes_stack.last();
expr_count += 1 + usize::from(class_details.bindings.temp.is_some());

let mut exprs = ctx.ast.vec_with_capacity(expr_count);

// Insert `_prop = new WeakMap()` expressions for private instance props
// (or `_prop = _classPrivateFieldLooseKey("prop")` if loose mode).
// Babel has these always go first, regardless of order of class elements.
// Also insert `var _prop;` temp var declarations for private static props.
let private_props = self.private_props_stack.last();
if let Some(private_props) = private_props {
if let Some(private_props) = &class_details.private_props {
// Insert `var _prop;` declarations here rather than when binding was created to maintain
// same order of `var` declarations as Babel.
// `c = class C { #x = 1; static y = 2; }` -> `var _C, _x;`
// TODO(improve-on-babel): Simplify this.
if self.private_fields_as_properties {
exprs.extend(private_props.props.iter().map(|(name, prop)| {
exprs.extend(private_props.iter().map(|(name, prop)| {
// Insert `var _prop;` declaration
self.ctx.var_declarations.insert_var(&prop.binding, ctx);

// `_prop = _classPrivateFieldLooseKey("prop")`
let value = self.create_private_prop_key_loose(name, ctx);
let value = Self::create_private_prop_key_loose(name, self.ctx, ctx);
create_assignment(&prop.binding, value, ctx)
}));
} else {
let mut weakmap_symbol_id = None;
exprs.extend(private_props.props.values().filter_map(|prop| {
exprs.extend(private_props.values().filter_map(|prop| {
// Insert `var _prop;` declaration
self.ctx.var_declarations.insert_var(&prop.binding, ctx);

Expand All @@ -144,7 +141,7 @@ impl<'a, 'ctx> ClassProperties<'a, 'ctx> {

// Insert class + static property assignments + static blocks
let class_expr = ctx.ast.move_expression(expr);
if let Some(binding) = &self.class_bindings.temp {
if let Some(binding) = &class_details.bindings.temp {
// Insert `var _Class` statement, if it wasn't already in `transform_class`
if !self.temp_var_is_created {
self.ctx.var_declarations.insert_var(binding, ctx);
Expand Down Expand Up @@ -193,7 +190,8 @@ impl<'a, 'ctx> ClassProperties<'a, 'ctx> {

// TODO: Run other transforms on inserted statements. How?

if let Some(temp_binding) = &self.class_bindings.temp {
let class_details = self.classes_stack.last_mut();
if let Some(temp_binding) = &class_details.bindings.temp {
// Binding for class name is required
if let Some(ident) = &class.id {
// Insert `var _Class` statement, if it wasn't already in `transform_class`
Expand Down Expand Up @@ -225,13 +223,13 @@ impl<'a, 'ctx> ClassProperties<'a, 'ctx> {
);
}

if let Some(private_props) = self.private_props_stack.last() {
if let Some(private_props) = &class_details.private_props {
if self.private_fields_as_properties {
self.ctx.statement_injector.insert_many_before(
&stmt_address,
private_props.props.iter().map(|(name, prop)| {
private_props.iter().map(|(name, prop)| {
// `var _prop = _classPrivateFieldLooseKey("prop");`
let value = self.create_private_prop_key_loose(name, ctx);
let value = Self::create_private_prop_key_loose(name, self.ctx, ctx);
create_variable_declaration(&prop.binding, value, ctx)
}),
);
Expand All @@ -240,7 +238,7 @@ impl<'a, 'ctx> ClassProperties<'a, 'ctx> {
let mut weakmap_symbol_id = None;
self.ctx.statement_injector.insert_many_before(
&stmt_address,
private_props.props.values().filter_map(|prop| {
private_props.values().filter_map(|prop| {
if prop.is_static {
return None;
}
Expand All @@ -258,15 +256,27 @@ impl<'a, 'ctx> ClassProperties<'a, 'ctx> {
.statement_injector
.insert_many_after(&stmt_address, self.insert_after_stmts.drain(..));
}

// Update class bindings prior to traversing class body and insertion of statements/expressions
// before/after the class. See comments on `ClassBindings`.
// Static private fields reference class name (not temp var) in class declarations.
// `class Class { static #prop; method() { return obj.#prop; } }`
// -> `method() { return _assertClassBrand(Class, obj, _prop)._; }`
// (note `Class` in `_assertClassBrand(Class, ...)`, not `_Class`)
// So set "temp" binding to actual class name while visiting class body.
// Note: If declaration is `export default class {}` with no name, and class has static props,
// then class has had name binding created already in `transform_class`.
// So name binding is always `Some`.
class_details.bindings.temp = class_details.bindings.name.clone();
}

/// `_classPrivateFieldLooseKey("prop")`
fn create_private_prop_key_loose(
&self,
name: &Atom<'a>,
transform_ctx: &TransformCtx<'a>,
ctx: &mut TraverseCtx<'a>,
) -> Expression<'a> {
self.ctx.helper_call_expr(
transform_ctx.helper_call_expr(
Helper::ClassPrivateFieldLooseKey,
SPAN,
ctx.ast.vec1(Argument::from(ctx.ast.expression_string_literal(
Expand Down Expand Up @@ -349,7 +359,7 @@ impl<'a, 'ctx> ClassProperties<'a, 'ctx> {

// Exit if nothing to transform
if instance_prop_count == 0 && !has_static_prop && !has_static_block {
self.private_props_stack.push(None);
self.classes_stack.push(ClassDetails::default());
return;
}

Expand Down Expand Up @@ -379,25 +389,14 @@ impl<'a, 'ctx> ClassProperties<'a, 'ctx> {
None
};

self.class_bindings = ClassBindings::new(class_name_binding, class_temp_binding);
let class_bindings = ClassBindings::new(class_name_binding, class_temp_binding);

// Add entry to `private_props_stack`
if private_props.is_empty() {
self.private_props_stack.push(None);
} else {
// `class_bindings.temp` in the `PrivateProps` entry is the temp var (if one has been created).
// Private fields in static prop initializers use the temp var in the transpiled output
// e.g. `_assertClassBrand(_Class, obj, _prop)`.
// At end of this function, if it's a class declaration, we set `class_bindings.temp` to be
// the binding for the class name, for when the class body is visited, because in the class
// body, private fields use the class name
// e.g. `_assertClassBrand(Class, obj, _prop)` (note `Class` not `_Class`).
self.private_props_stack.push(Some(PrivateProps {
props: private_props,
class_bindings: self.class_bindings.clone(),
is_declaration: self.is_declaration,
}));
}
// Add entry to `classes_stack`
self.classes_stack.push(ClassDetails {
is_declaration: self.is_declaration,
private_props: if private_props.is_empty() { None } else { Some(private_props) },
bindings: class_bindings,
});

// Determine where to insert instance property initializers in constructor
let instance_inits_insert_location = if instance_prop_count == 0 {
Expand Down Expand Up @@ -488,29 +487,6 @@ impl<'a, 'ctx> ClassProperties<'a, 'ctx> {
ctx,
);
}

// Update class bindings prior to traversing class body and insertion of statements/expressions
// before/after the class. See comments on `ClassBindings`.
if let Some(private_props) = self.private_props_stack.last_mut() {
// Transfer state of `temp` binding from `private_props_stack` to `self`.
// A temp binding may have been created while transpiling private fields in
// static prop initializers.
// TODO: Do this where `class_bindings.temp` is consumed instead?
if let Some(temp_binding) = &private_props.class_bindings.temp {
self.class_bindings.temp = Some(temp_binding.clone());
}

// Static private fields reference class name (not temp var) in class declarations.
// `class Class { static #prop; method() { return obj.#prop; } }`
// -> `method() { return _assertClassBrand(Class, obj, _prop)._; }`
// (note `Class` in `_assertClassBrand(Class, ...)`, not `_Class`)
// So set "temp" binding to actual class name while visiting class body.
// Note: If declaration is `export default class {}` with no name, and class has static props,
// then class has had name binding created already above. So name binding is always `Some`.
if self.is_declaration {
private_props.class_bindings.temp = private_props.class_bindings.name.clone();
}
}
}

/// Pop from private props stack.
Expand All @@ -523,7 +499,7 @@ impl<'a, 'ctx> ClassProperties<'a, 'ctx> {
return;
}

self.private_props_stack.pop();
self.classes_stack.pop();
}

/// Insert an expression after the class.
Expand Down
12 changes: 4 additions & 8 deletions crates/oxc_transformer/src/es2022/class_properties/class_bindings.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@ use oxc_traverse::{BoundIdentifier, TraverseCtx};
/// 2. Temp var `_Class`, which may or may not be required.
///
/// Temp var is required in the following circumstances:
///
/// * Class expression has static properties.
/// e.g. `C = class { x = 1; }`
/// * Class declaration has static properties and one of the static prop's initializers contains:
Expand All @@ -17,11 +18,6 @@ use oxc_traverse::{BoundIdentifier, TraverseCtx};
/// c. A private field referring to one of the class's static private props.
/// e.g. `class C { static #x; static y = obj.#x; }`
///
/// An instance of `ClassBindings` is stored in main `ClassProperties` transform, and a 2nd is stored
/// in `PrivateProps` for the class, if the class has any private properties.
/// If the class has private props, the instance of `ClassBindings` in `PrivateProps` is the source
/// of truth.
///
/// The logic for when transpiled private fields use a reference to class name or class temp var
/// is unfortunately rather complicated.
///
Expand All @@ -34,12 +30,12 @@ use oxc_traverse::{BoundIdentifier, TraverseCtx};
/// e.g. `class C { static #x; y = obj.#x; }`
///
/// To cover all these cases, the meaning of `temp` binding here changes while traversing the class body.
/// [`ClassProperties::transform_class`] sets `temp` binding to be a copy of the `name` binding before
/// that traversal begins. So the name `temp` is misleading at that point.
/// [`ClassProperties::transform_class_declaration`] sets `temp` binding to be a copy of the
/// `name` binding before that traversal begins. So the name `temp` is misleading at that point.
///
/// Debug assertions are used to make sure this complex logic is correct.
///
/// [`ClassProperties::transform_class`]: super::ClassProperties::transform_class
/// [`ClassProperties::transform_class_declaration`]: super::ClassProperties::transform_class_declaration
#[derive(Default, Clone)]
pub(super) struct ClassBindings<'a> {
/// Binding for class name, if class has name
Expand Down
120 changes: 120 additions & 0 deletions crates/oxc_transformer/src/es2022/class_properties/class_details.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,120 @@
use oxc_ast::ast::*;
use oxc_data_structures::stack::NonEmptyStack;
use oxc_span::Atom;
use oxc_traverse::BoundIdentifier;

use super::{ClassBindings, ClassProperties, FxIndexMap};

/// Details of a class.
///
/// These are stored in `ClassesStack`.
#[derive(Default)]
pub(super) struct ClassDetails<'a> {
/// `true` for class declaration, `false` for class expression
pub is_declaration: bool,
/// Private properties.
/// Mapping private prop name to binding for temp var.
/// This is then used as lookup when transforming e.g. `this.#x`.
/// `None` if class has no private properties.
pub private_props: Option<FxIndexMap<Atom<'a>, PrivateProp<'a>>>,
/// Bindings for class name and temp var for class
pub bindings: ClassBindings<'a>,
}

/// Details of a private property.
pub(super) struct PrivateProp<'a> {
pub binding: BoundIdentifier<'a>,
pub is_static: bool,
}

/// Stack of `ClassDetails`.
/// Pushed to when entering a class, popped when exiting.
///
/// We use a `NonEmptyStack` to make `last` and `last_mut` cheap (these are used a lot).
/// The first entry is a dummy.
///
/// This is a separate structure, rather than just storing stack as a property of `ClassProperties`
/// to work around borrow-checker. You can call `find_private_prop` and retain the return value
/// without holding a mut borrow of the whole of `&mut ClassProperties`. This allows accessing other
/// properties of `ClassProperties` while that borrow is held.
#[derive(Default)]
pub(super) struct ClassesStack<'a> {
stack: NonEmptyStack<ClassDetails<'a>>,
}

impl<'a> ClassesStack<'a> {
/// Push an entry to stack.
#[inline]
pub fn push(&mut self, class: ClassDetails<'a>) {
self.stack.push(class);
}

/// Push last entry from stack.
#[inline]
pub fn pop(&mut self) -> ClassDetails<'a> {
self.stack.pop()
}

/// Get details of current class.
#[inline]
pub fn last(&self) -> &ClassDetails<'a> {
self.stack.last()
}

/// Get details of current class as `&mut` reference.
#[inline]
pub fn last_mut(&mut self) -> &mut ClassDetails<'a> {
self.stack.last_mut()
}

/// Lookup details of private property referred to by `ident`.
pub fn find_private_prop<'b>(
&'b mut self,
ident: &PrivateIdentifier<'a>,
) -> Option<ResolvedPrivateProp<'a, 'b>> {
// Check for binding in closest class first, then enclosing classes.
// We skip the first, because this is a `NonEmptyStack` with dummy first entry.
// TODO: Check there are tests for bindings in enclosing classes.
for class in self.stack[1..].iter_mut().rev() {
if let Some(private_props) = &mut class.private_props {
if let Some(prop) = private_props.get(&ident.name) {
return Some(ResolvedPrivateProp {
prop_binding: &prop.binding,
class_bindings: &mut class.bindings,
is_static: prop.is_static,
is_declaration: class.is_declaration,
});
}
}
}
// TODO: This should be unreachable. Only returning `None` because implementation is incomplete.
None
}
}

/// Details of a private property resolved for a private field.
///
/// This is the return value of [`ClassesStack::find_private_prop`].
pub(super) struct ResolvedPrivateProp<'a, 'b> {
/// Binding for temp var representing the property
pub prop_binding: &'b BoundIdentifier<'a>,
/// Bindings for class name and temp var for class
pub class_bindings: &'b mut ClassBindings<'a>,
/// `true` if is a static property
pub is_static: bool,
/// `true` if class which defines this property is a class declaration
pub is_declaration: bool,
}

// Shortcut methods to get current class
impl<'a, 'ctx> ClassProperties<'a, 'ctx> {
/// Get details of current class.
pub(super) fn current_class(&self) -> &ClassDetails<'a> {
self.classes_stack.last()
}

/// Get details of current class as `&mut` reference.
pub(super) fn current_class_mut(&mut self) -> &mut ClassDetails<'a> {
self.classes_stack.last_mut()
}
}
Loading
Loading