feat(ast)!: generate ast_builder.rs.

[rzvxa]

Every structure has 2 builder methods:

1. xxx e.g. block_statement

    #[inline]
    pub fn block_statement(self, span: Span, body: Vec<'a, Statement<'a>>) -> BlockStatement<'a> {
        BlockStatement { span, body, scope_id: Default::default() }
    }

2. alloc_xxx e.g. alloc_block_statement

    #[inline]
    pub fn alloc_block_statement(
        self,
        span: Span,
        body: Vec<'a, Statement<'a>>,
    ) -> Box<'a, BlockStatement<'a>> {
        self.block_statement(span, body).into_in(self.allocator)
    }

We generate 3 types of methods for enums:

1. yyy_xxx e.g. statement_block

    #[inline]
    pub fn statement_block(self, span: Span, body: Vec<'a, Statement<'a>>) -> Statement<'a> {
        Statement::BlockStatement(self.alloc(self.block_statement(span, body)))
    }

2. yyy_from_xxx e.g. statement_from_block

    #[inline]
    pub fn statement_from_block<T>(self, inner: T) -> Statement<'a>
    where
        T: IntoIn<'a, Box<'a, BlockStatement<'a>>>,
    {
        Statement::BlockStatement(inner.into_in(self.allocator))
    }

3. yyy_xxx where xxx is inherited e.g. statement_declaration

    #[inline]
    pub fn statement_declaration(self, inner: Declaration<'a>) -> Statement<'a> {
        Statement::from(inner)
    }

---

Generic parameters:

We no longer accept Box<'a, ADT>, Atom or &'a str, Instead we use IntoIn<'a, Box<'a, ADT>>, IntoIn<'a, Atom<'a>> and IntoIn<'a, &'a str> respectively.
It allows us to rewrite things like this:

let ident = IdentifierReference::new(SPAN, Atom::from("require"));
let number_literal_expr = self.ast.expression_numeric_literal(
    right_expr.span(),
    num,
    raw,
    self.ast.new_str(num.to_string().as_str()),
    NumberBase::Decimal,
);

As this:

let ident = IdentifierReference::new(SPAN, "require");
let number_literal_expr = self.ast.expression_numeric_literal(
    right_expr.span(),
    num,
    raw,
    num.to_string(),
    NumberBase::Decimal,
);

[graphite-app]

Your org has enabled the Graphite merge queue for merging into main

Add the label “merge” to the PR and Graphite will automatically add it to the merge queue when it’s ready to merge. Or use the label “hotfix” to add to the merge queue as a hot fix.

You must have a Graphite account and log in to Graphite in order to use the merge queue. Sign up using this link.

[rzvxa]

• feat(ast): allow conversion from Expression into Statement with FromIn trait. #4124
• refactor(ast/ast_builder)!: shorter allocator utility method names. #4122
• feat(ast_codegen): add cli arguments to use with oxc_ast_codegen executable. #4117
• refactor(ast_codegen): cleanup usages of the generated outputs. #4116
• feat(ast)!: generate ast_builder.rs. #3890 👈
• feat(ast_codegen): add ast builder generator. #4098
• feat(span): add various implementations of FromIn for Atom. #4090
• feat(allocator): introduce FromIn and IntoIn traits. #4088
• main

This stack of pull requests is managed by Graphite. Learn more about stacking.

Join @rzvxa and the rest of your teammates on Graphite

[overlookmotel]

This one is trickier, because all kinds of weird methods have been added to AST builder over time, and I don't think there's much consistency in:

• Which methods are implemented and which are not.
• What params methods take - some receive T, others Box<T>.
• What methods return - T, Box<T>, or Expression / Statement.
• Naming of methods.

I think we might need to standardize the methods in the existing manually-written impl first, before moving over to a codegen-ed version.

There's also a danger of generating lots more code than we have at present, and hurting compile times, if we try to make the codegen-ed version cover all possible cases. If the amount of code generated is excessive, maybe we need attrs on the AST types which indicate which builders to generate?

#[visited_node]
#[builder(Expression)]
struct BooleanLiteral {
    #[cfg_attr(feature = "serialize", serde(flatten))]
    pub span: Span,
    pub value: bool,
}

This would generate:

impl<'a> AstBuilder<'a> {
    // Always generated by default
    fn boolean_literal(self, span: Span, value: bool) -> BooleanLiteral {
        BooleanLiteral { span, value }
    }

    // Generated because of `#[builder(Expression)]` attr
    fn boolean_literal_expression(self, span: Span, value: bool) -> Expression<'a> {
        Expression::BooleanLiteral(self.alloc(BooleanLiteral { span, value }))
    }
}

Other methods that we could have generated (but don't want to):

impl<'a> AstBuilder<'a> {
    fn boolean_literal_array_expression_element(self, span: Span, value: bool) -> ArrayExpressionElement<'a> {
        ArrayExpressionElement::BooleanLiteral(self.alloc(BooleanLiteral { span, value }))
    }

    fn boolean_literal_property_key(self, span: Span, value: bool) -> PropertyKey<'a> {
        PropertyKey::BooleanLiteral(self.alloc(BooleanLiteral { span, value }))
    }

    fn boolean_literal_argument(self, span: Span, value: bool) -> Argument<'a> {
        Argument::BooleanLiteral(self.alloc(BooleanLiteral { span, value }))
    }
}

[overlookmotel]

Probably what I suggested above is too complicated. Maybe better to tackle this incrementally?

e.g. Start by replacing all the methods for Expressions and Statements as 1 PR. Then continue whittling away the easy/standard ones (e.g. Declarations) in further PRs, until most of manual impls file is cleared out, and then it's easier to see what oddities remain.

For all Expressions, we could generate:

impl BooleanLiteral {
    #[inline]
    fn new(span: Span, value: bool) -> BooleanLiteral {
        BooleanLiteral { span, value }
    }

    #[inline]
    fn new_expression<'a>(span: Span, value: bool, allocator: &'a Allocator) -> Expression<'a> {
        Expression::BooleanLiteral(
            Box::new_in(BooleanLiteral::new(span, value), allocator)
        )
    }
}

impl<'a> AstBuilder<'a> {
    #[inline]
    fn boolean_literal(self, span: Span, value: bool) -> Expression<'a> {
        BooleanLiteral::new_expression(span, value, self.allocator)
    }
}

If consumer wants an Argument::BooleanLiteral, they can convert themselves with Argument::from(ast.boolean_literal(span, value)).

As I see it, the main value of AstBuilder is to hold the allocator and so provide the nicer API self.ast.boolean_literal(span, value) rather than BooleanLiteral::new_expression(span, value, self.allocator). So any function which doesn't require the allocator doesn't need to be in AstBuilder.

[rzvxa]

@overlookmotel let me mull this one for a little while. I'll come back when I have figured out a working plan to discuss.

[codspeed-hq]

CodSpeed Performance Report

Merging #3890 will not alter performance

_{Comparing 06-25-feat_ast_codegen_generate_ast_builder.rs_ (d347aed) with main (91c792a)}

Summary

✅ 29 untouched benchmarks

[rzvxa]

Related to backlog#29

Right now I can think of 3 options to unify all of our builder calls(both in terms of name and signature),

1. Let the user do some of the work

Have a bunch of enum_variant methods like this:

fn statement_block_statement(self, it: BlockStatement<'a>) -> Statement<'a> {
    Statement::BlockStatement(self.alloc(it))
}

fn block_statement(
    self,
    span: Span,
    body: Vec<'a, Statement<'a>>,
) -> BlockStatement<'a> {
    BlockStatement {
        span,
        body,
        scope_id: Default::default(),
    }
}

then we use it like this:

let block = self.ast.block_statement(span, body);
let stmt = self.ast.statement_block_statement(block);

2. More methods

We have methods like these:

fn statement_block_statement(
    self,
    span: Span,
    body: Vec<'a, Statement<'a>>,
) -> Statement<'a> {
    Statement::BlockStatement(self.alloc(self.block_statement(span, body)))
}

fn block_statement(
    self,
    span: Span,
    body: Vec<'a, Statement<'a>>,
) -> BlockStatement<'a> {
    BlockStatement {
        span,
        body,
        scope_id: Default::default(),
    }
}

and use like this:

let stmt = self.ast.statement_block_statement(span, body);

This is nice but it would mean that if you want to wrap an already existing block in a Statement enum you have to do something like this:

let stmt = Statement::BlockStatement(self.alloc(block));

Or rename the method we previously called statement_block_statement to new_statement_block_statement; and generate more functions to use like this(similar to 1):

fn statement_block_statement(self, it: BlockExpression<'a>) -> Expression<'a> {
    Statement::BlockStatement(self.alloc(it))
}

3. Using From trait

We generate these methods:

fn block_statement(
    self,
    span: Span,
    body: Vec<'a, Statement<'a>>,
) -> Box<'a, BlockStatement<'a>> {
    self.alloc(BlockStatement {
        span,
        body,
        scope_id: Default::default(),
    })
}

we also generate From implementations for variant initialization.

If we want to initialize a try we write something like this:

let try = self.ast.try_statement(
    span,
    self.ast.block_statement(block_span, block_body)
);

If we want a Statement from our block we just do this:

let stmt = self.ast.block_statement(span, body).into();

TL;DR

So to sum up here all 3 approaches that I can think of together:

// 1
let block = self.ast.block_statement(span, body);
let block_stmt = self.ast.statement_block_statement(block);
// 2
let block_stmt = self.ast.statement_block_statement(span, body);
// 3
let block_stmt = self.ast.block_statement(span, body).into();


// 1
let try_stmt = self.ast.try_statement(
    span,
    self.ast.alloc(self.ast.block_statement(block_span, block_body))
    handler,
    finalizer,
)
// 2
let try_stmt = self.ast.try_statement(
    span,
    self.ast.alloc(self.ast.block_statement(block_span, block_body))
    handler,
    finalizer,
)
// 3
let try_stmt = self.ast.try_statement(
    span,
    self.ast.block_statement(block_span, block_body).into()
    handler,
    finalizer,
)

I personally like option 3 more than the other 2 but I'm open to any objections.

@Boshen @overlookmotel I would like to know your thoughts on this, And what are your suggestions.

[rzvxa]

There is actually a fourth and fifth option which @overlookmotel mentioned these are the 3 that I can think of. So don't forget to read the previous comments.

[Boshen]

We can merge once @overlookmotel finishes the review, along with a branch in a rolldown fork with oxc pointing to this branch.

[rzvxa]

We can merge once @overlookmotel finishes the review, along with a branch in a rolldown fork with oxc pointing to this branch.

It is based on #4124, Give it a look and let me know if there is anything I should consider changing. I'm mainly not sure about the snapshot changes, I know the pure comments are a valid change(#4119) but for some reason, the snapshot path differs on my system.
https://github.com/rzvxa/rolldown/tree/migrate-to-new-builder

[overlookmotel]

I do have some thoughts/potential changes to propose, but given the size of the PR, let's get it merged ASAP, and then iterate.

[graphite-app]

Merge activity

• Jul 8, 8:47 PM EDT: The merge label 'merge' was detected. This PR will be added to the Graphite merge queue once it meets the requirements.
• Jul 8, 8:57 PM EDT: overlookmotel added this pull request to the Graphite merge queue.
• Jul 8, 9:08 PM EDT: overlookmotel merged this pull request with the Graphite merge queue.