Skip to content

Commit

Permalink
feat(codegen): implement BinaryExpressionVisitor (#4548)
Browse files Browse the repository at this point in the history
part of oxc-project/backlog#58

`monitor-oxc` run: https://github.com/oxc-project/monitor-oxc/actions/runs/10179047831
binary expression stack length tally using `counts` in top 100 npm packages from monitor-oxc:

```
29772 counts
(  1)    17652 (59.3%, 59.3%): 0
(  2)     5772 (19.4%, 78.7%): 1
(  3)     3204 (10.8%, 89.4%): 2
(  4)     1276 ( 4.3%, 93.7%): 3
(  5)      616 ( 2.1%, 95.8%): 4
(  6)      308 ( 1.0%, 96.8%): 5
(  7)      202 ( 0.7%, 97.5%): 6
(  8)      168 ( 0.6%, 98.1%): 7
(  9)      114 ( 0.4%, 98.5%): 9
( 10)       90 ( 0.3%, 98.8%): 8
( 11)       84 ( 0.3%, 99.0%): 13
( 12)       58 ( 0.2%, 99.2%): 10
( 13)       48 ( 0.2%, 99.4%): 12
( 14)       32 ( 0.1%, 99.5%): 11
( 15)       20 ( 0.1%, 99.6%): 134
( 16)       16 ( 0.1%, 99.6%): 18
( 17)       16 ( 0.1%, 99.7%): 20
( 18)       12 ( 0.0%, 99.7%): 19
( 19)       12 ( 0.0%, 99.8%): 35
( 20)       12 ( 0.0%, 99.8%): 51
( 21)       10 ( 0.0%, 99.8%): 15
( 22)        6 ( 0.0%, 99.9%): 17
( 23)        6 ( 0.0%, 99.9%): 21
( 24)        6 ( 0.0%, 99.9%): 45
( 25)        4 ( 0.0%, 99.9%): 14
( 26)        4 ( 0.0%, 99.9%): 26
( 27)        4 ( 0.0%, 99.9%): 53
( 28)        2 ( 0.0%, 99.9%): 172
( 29)        2 ( 0.0%, 99.9%): 214
( 30)        2 ( 0.0%,100.0%): 22
( 31)        2 ( 0.0%,100.0%): 27
( 32)        2 ( 0.0%,100.0%): 28
( 33)        2 ( 0.0%,100.0%): 29
( 34)        2 ( 0.0%,100.0%): 31
( 35)        2 ( 0.0%,100.0%): 36
( 36)        2 ( 0.0%,100.0%): 46
( 37)        2 ( 0.0%,100.0%): 55
```
  • Loading branch information
Boshen committed Jul 31, 2024
1 parent b87bf70 commit a558492
Show file tree
Hide file tree
Showing 11 changed files with 624 additions and 420 deletions.
193 changes: 193 additions & 0 deletions crates/oxc_codegen/src/binary_expr_visitor.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,193 @@
//! Visit binary and logical expression in a loop without recursion.
//!
//! Reference: <https://github.com/evanw/esbuild/blob/78f89e41d5e8a7088f4820351c6305cc339f8820/internal/js_printer/js_printer.go#L3266>
use std::ops::Not;

use oxc_ast::ast::{BinaryExpression, Expression, LogicalExpression};
use oxc_syntax::operator::{BinaryOperator, LogicalOperator};
use oxc_syntax::precedence::{GetPrecedence, Precedence};

use crate::{
gen::{Gen, GenExpr},
Codegen, Context,
};

#[derive(Clone, Copy)]
pub enum Binaryish<'a> {
Binary(&'a BinaryExpression<'a>),
Logical(&'a LogicalExpression<'a>),
}

impl<'a> Binaryish<'a> {
pub fn left(&self) -> &'a Expression<'a> {
match self {
Self::Binary(e) => e.left.without_parenthesized(),
Self::Logical(e) => e.left.without_parenthesized(),
}
}

pub fn right(&self) -> &'a Expression<'a> {
match self {
Self::Binary(e) => e.right.without_parenthesized(),
Self::Logical(e) => e.right.without_parenthesized(),
}
}

pub fn operator(&self) -> BinaryishOperator {
match self {
Self::Binary(e) => BinaryishOperator::Binary(e.operator),
Self::Logical(e) => BinaryishOperator::Logical(e.operator),
}
}
}

#[derive(Debug, Clone, Copy, Eq, PartialEq)]
pub enum BinaryishOperator {
Binary(BinaryOperator),
Logical(LogicalOperator),
}

impl<const MINIFY: bool> Gen<MINIFY> for BinaryishOperator {
fn gen(&self, p: &mut Codegen<{ MINIFY }>, ctx: Context) {
match self {
Self::Binary(op) => op.gen(p, ctx),
Self::Logical(op) => op.gen(p, ctx),
}
}
}

impl GetPrecedence for BinaryishOperator {
fn precedence(&self) -> Precedence {
match self {
Self::Binary(op) => op.precedence(),
Self::Logical(op) => op.precedence(),
}
}
}

impl BinaryishOperator {
pub fn lower_precedence(self) -> Precedence {
match self {
Self::Binary(op) => op.lower_precedence(),
Self::Logical(op) => op.lower_precedence(),
}
}
}

#[derive(Clone, Copy)]
pub struct BinaryExpressionVisitor<'a> {
pub e: Binaryish<'a>,
pub precedence: Precedence,
pub ctx: Context,

pub left_precedence: Precedence,
pub left_ctx: Context,

pub operator: BinaryishOperator,
pub wrap: bool,
pub right_precedence: Precedence,
}

impl<'a> BinaryExpressionVisitor<'a> {
pub fn gen_expr<const MINIFY: bool>(v: Self, p: &mut Codegen<'a, { MINIFY }>) {
let mut v = v;
let stack_bottom = p.binary_expr_stack.len();
loop {
if !v.check_and_prepare(p) {
break;
}

let left = v.e.left();
let left_binary = match left {
Expression::BinaryExpression(e) => Some(Binaryish::Binary(e)),
Expression::LogicalExpression(e) => Some(Binaryish::Logical(e)),
_ => None,
};

let Some(left_binary) = left_binary else {
left.gen_expr(p, v.left_precedence, v.left_ctx);
v.visit_right_and_finish(p);
break;
};

p.binary_expr_stack.push(v);
v = BinaryExpressionVisitor {
e: left_binary,
precedence: v.left_precedence,
ctx: v.left_ctx,
left_precedence: Precedence::Lowest,
left_ctx: Context::empty(),
operator: v.operator,
wrap: false,
right_precedence: Precedence::Lowest,
};
}

loop {
let len = p.binary_expr_stack.len();
if len == 0 || len - 1 < stack_bottom {
break;
}
let v = p.binary_expr_stack.pop().unwrap();
v.visit_right_and_finish(p);
}
}

pub fn check_and_prepare<const MINIFY: bool>(&mut self, p: &mut Codegen<{ MINIFY }>) -> bool {
let e = self.e;
self.operator = e.operator();

self.wrap = self.precedence >= self.operator.precedence()
|| (self.operator == BinaryishOperator::Binary(BinaryOperator::In)
&& self.ctx.intersects(Context::FORBID_IN));

if self.wrap {
p.print_char(b'(');
self.ctx &= Context::FORBID_IN.not();
}

self.left_precedence = self.operator.lower_precedence();
self.right_precedence = self.operator.lower_precedence();

if self.operator.precedence().is_right_associative() {
self.left_precedence = self.operator.precedence();
}

if self.operator.precedence().is_left_associative() {
self.right_precedence = self.operator.precedence();
}

match self.operator {
BinaryishOperator::Logical(LogicalOperator::Coalesce) => {
if let Expression::LogicalExpression(logical_expr) = e.left() {
if matches!(logical_expr.operator, LogicalOperator::And | LogicalOperator::Or) {
self.left_precedence = Precedence::Prefix;
}
}
if let Expression::LogicalExpression(logical_expr) = e.right() {
if matches!(logical_expr.operator, LogicalOperator::And | LogicalOperator::Or) {
self.right_precedence = Precedence::Prefix;
}
}
}
BinaryishOperator::Binary(BinaryOperator::Exponential) => {
if matches!(e.left(), Expression::UnaryExpression(_)) {
self.left_precedence = Precedence::Call;
}
}
_ => {}
}

true
}

pub fn visit_right_and_finish<const MINIFY: bool>(&self, p: &mut Codegen<{ MINIFY }>) {
p.print_soft_space();
self.operator.gen(p, Context::empty());
p.print_soft_space();
self.e.right().gen_expr(p, self.right_precedence, self.ctx & Context::FORBID_IN);
if self.wrap {
p.print_char(b')');
}
}
}
96 changes: 47 additions & 49 deletions crates/oxc_codegen/src/gen.rs
Original file line number Diff line number Diff line change
Expand Up @@ -8,11 +8,14 @@ use oxc_syntax::{
identifier::{LS, PS},
keyword::is_reserved_keyword_or_global_object,
number::NumberBase,
operator::{BinaryOperator, UnaryOperator},
operator::{BinaryOperator, LogicalOperator, UnaryOperator},
precedence::{GetPrecedence, Precedence},
};

use crate::{Codegen, Context, Operator};
use crate::{
binary_expr_visitor::{BinaryExpressionVisitor, Binaryish, BinaryishOperator},
Codegen, Context, Operator,
};

pub trait Gen<const MINIFY: bool> {
fn gen(&self, _p: &mut Codegen<{ MINIFY }>, _ctx: Context) {}
Expand Down Expand Up @@ -627,11 +630,12 @@ impl<'a, const MINIFY: bool> Gen<MINIFY> for VariableDeclarator<'a> {

impl<'a, const MINIFY: bool> Gen<MINIFY> for Function<'a> {
fn gen(&self, p: &mut Codegen<{ MINIFY }>, ctx: Context) {
p.add_source_mapping(self.span.start);
let n = p.code_len();
p.gen_comment(self.span.start);
let wrap = self.is_expression() && (p.start_of_stmt == n || p.start_of_default_export == n);
p.gen_comment(self.span.start);
p.wrap(wrap, |p| {
p.print_space_before_identifier();
p.add_source_mapping(self.span.start);
if self.declare {
p.print_str("declare ");
}
Expand Down Expand Up @@ -1101,6 +1105,7 @@ impl<'a, const MINIFY: bool> GenExpr<MINIFY> for ParenthesizedExpression<'a> {
impl<'a, const MINIFY: bool> Gen<MINIFY> for IdentifierReference<'a> {
fn gen(&self, p: &mut Codegen<{ MINIFY }>, _ctx: Context) {
let name = p.get_identifier_reference_name(self);
p.print_space_before_identifier();
p.add_source_mapping_for_name(self.span, name);
p.print_str(name);
}
Expand Down Expand Up @@ -1131,6 +1136,7 @@ impl<'a, const MINIFY: bool> Gen<MINIFY> for LabelIdentifier<'a> {
impl<const MINIFY: bool> Gen<MINIFY> for BooleanLiteral {
fn gen(&self, p: &mut Codegen<{ MINIFY }>, _ctx: Context) {
p.add_source_mapping(self.span.start);
p.print_space_before_identifier();
p.print_str(self.as_str());
}
}
Expand Down Expand Up @@ -1729,51 +1735,40 @@ impl<'a, const MINIFY: bool> GenExpr<MINIFY> for UnaryExpression<'a> {

impl<'a, const MINIFY: bool> GenExpr<MINIFY> for BinaryExpression<'a> {
fn gen_expr(&self, p: &mut Codegen<{ MINIFY }>, precedence: Precedence, ctx: Context) {
let mut ctx = ctx;
let wrap = precedence >= self.precedence()
|| (self.operator == BinaryOperator::In && ctx.intersects(Context::FORBID_IN));
if wrap {
ctx &= Context::FORBID_IN.not();
}
p.wrap(wrap, |p| {
let left_precedence = if self.operator.precedence().is_right_associative() {
// "**" can't contain certain unary expressions
if matches!(self.left.without_parenthesized(), Expression::UnaryExpression(_)) {
Precedence::Call
} else {
self.precedence()
}
} else {
self.operator.lower_precedence()
};
self.left.gen_expr(p, left_precedence, ctx);
if self.operator.is_keyword() {
p.print_space_before_identifier();
} else {
p.print_soft_space();
}
self.operator.gen(p, ctx);
let right_precedence = if self.operator.precedence().is_left_associative() {
self.precedence()
} else {
self.operator.lower_precedence()
};
self.right.gen_expr(p, right_precedence, ctx & Context::FORBID_IN);
});
let v = BinaryExpressionVisitor {
// SAFETY:
// The pointer is stored on the heap and all will be consumed in the binary expression visitor.
e: Binaryish::Binary(unsafe {
std::mem::transmute::<&BinaryExpression<'_>, &BinaryExpression<'_>>(self)
}),
precedence,
ctx,
left_precedence: Precedence::Lowest,
left_ctx: Context::empty(),
operator: BinaryishOperator::Binary(self.operator),
wrap: false,
right_precedence: Precedence::Lowest,
};
BinaryExpressionVisitor::gen_expr(v, p);
}
}

impl<const MINIFY: bool> Gen<MINIFY> for LogicalOperator {
fn gen(&self, p: &mut Codegen<{ MINIFY }>, _ctx: Context) {
p.print_str(self.as_str());
}
}

impl<const MINIFY: bool> Gen<MINIFY> for BinaryOperator {
fn gen(&self, p: &mut Codegen<{ MINIFY }>, _ctx: Context) {
let operator = self.as_str();
if self.is_keyword() {
p.print_space_before_identifier();
p.print_str(operator);
p.print_hard_space();
} else {
let op: Operator = (*self).into();
p.print_space_before_operator(op);
p.print_str(operator);
p.print_soft_space();
p.prev_op = Some(op);
p.prev_op_end = p.code().len();
}
Expand All @@ -1790,18 +1785,21 @@ impl<'a, const MINIFY: bool> Gen<MINIFY> for PrivateInExpression<'a> {

impl<'a, const MINIFY: bool> GenExpr<MINIFY> for LogicalExpression<'a> {
fn gen_expr(&self, p: &mut Codegen<{ MINIFY }>, precedence: Precedence, ctx: Context) {
// Logical expressions and coalesce expressions cannot be mixed (Syntax Error).
let mixed = matches!(
(precedence, self.precedence()),
(Precedence::NullishCoalescing, Precedence::LogicalAnd | Precedence::LogicalOr)
);
p.wrap(mixed || (precedence >= self.precedence()), |p| {
self.left.gen_expr(p, self.precedence(), ctx);
p.print_soft_space();
p.print_str(self.operator.as_str());
p.print_soft_space();
self.right.gen_expr(p, self.precedence(), ctx);
});
let v = BinaryExpressionVisitor {
// SAFETY:
// The pointer is stored on the heap and all will be consumed in the binary expression visitor.
e: Binaryish::Logical(unsafe {
std::mem::transmute::<&LogicalExpression<'_>, &LogicalExpression<'_>>(self)
}),
precedence,
ctx,
left_precedence: Precedence::Lowest,
left_ctx: Context::empty(),
operator: BinaryishOperator::Logical(self.operator),
wrap: false,
right_precedence: Precedence::Lowest,
};
BinaryExpressionVisitor::gen_expr(v, p);
}
}

Expand Down
8 changes: 7 additions & 1 deletion crates/oxc_codegen/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@
//! * [esbuild](https://github.com/evanw/esbuild/blob/main/internal/js_printer/js_printer.go)
mod annotation_comment;
mod binary_expr_visitor;
mod context;
mod gen;
mod operator;
Expand All @@ -25,11 +26,14 @@ use oxc_syntax::{
precedence::Precedence,
};

use crate::{
binary_expr_visitor::BinaryExpressionVisitor, operator::Operator,
sourcemap_builder::SourcemapBuilder,
};
pub use crate::{
context::Context,
gen::{Gen, GenExpr},
};
use crate::{operator::Operator, sourcemap_builder::SourcemapBuilder};

/// Code generator without whitespace removal.
pub type CodeGenerator<'a> = Codegen<'a, false>;
Expand Down Expand Up @@ -72,6 +76,7 @@ pub struct Codegen<'a, const MINIFY: bool> {
prev_reg_exp_end: usize,
need_space_before_dot: usize,
print_next_indent_as_space: bool,
binary_expr_stack: Vec<BinaryExpressionVisitor<'a>>,

/// For avoiding `;` if the previous statement ends with `}`.
needs_semicolon: bool,
Expand Down Expand Up @@ -130,6 +135,7 @@ impl<'a, const MINIFY: bool> Codegen<'a, MINIFY> {
needs_semicolon: false,
need_space_before_dot: 0,
print_next_indent_as_space: false,
binary_expr_stack: Vec::with_capacity(5),
prev_op_end: 0,
prev_reg_exp_end: 0,
prev_op: None,
Expand Down
Loading

0 comments on commit a558492

Please sign in to comment.