From 3e19e4e0047cfbd906b3a20fd11bd8e33c7491f0 Mon Sep 17 00:00:00 2001 From: Boshen Date: Wed, 22 Jan 2025 09:08:56 +0800 Subject: [PATCH 01/14] perf(minifier): remove the useless empty statement removal code in statement fusion (#8646) --- crates/oxc_minifier/src/peephole/statement_fusion.rs | 8 -------- 1 file changed, 8 deletions(-) diff --git a/crates/oxc_minifier/src/peephole/statement_fusion.rs b/crates/oxc_minifier/src/peephole/statement_fusion.rs index a4be4ba66b01f..20fa9b8198462 100644 --- a/crates/oxc_minifier/src/peephole/statement_fusion.rs +++ b/crates/oxc_minifier/src/peephole/statement_fusion.rs @@ -17,10 +17,6 @@ impl<'a> PeepholeOptimizations { stmts: &mut Vec<'a, Statement<'a>>, ctx: &mut TraverseCtx<'a>, ) { - self.fuse_statements(stmts, ctx); - } - - fn fuse_statements(&mut self, stmts: &mut Vec<'a, Statement<'a>>, ctx: &mut TraverseCtx<'a>) { let len = stmts.len(); if len <= 1 { @@ -57,10 +53,6 @@ impl<'a> PeepholeOptimizations { } } } - - if self.is_current_function_changed() { - stmts.retain(|stmt| !matches!(stmt, Statement::EmptyStatement(_))); - } } fn is_fusable_control_statement(stmt: &Statement<'a>) -> bool { From dc912fa58e661707c2ef4caad1298f8fdd040a40 Mon Sep 17 00:00:00 2001 From: Tapan Prakash Date: Wed, 22 Jan 2025 06:39:39 +0530 Subject: [PATCH 02/14] fix(linter): Added missing $schema property to default config (#8625) The $schema property was not added when the --init command was used to create the configuration. Now, $schema is added based on the availability of configuration_schema.json in the current working directory. --------- Co-authored-by: autofix-ci[bot] <114827586+autofix-ci[bot]@users.noreply.github.com> --- apps/oxlint/src/lint.rs | 19 ++++++++++++++++++- 1 file changed, 18 insertions(+), 1 deletion(-) diff --git a/apps/oxlint/src/lint.rs b/apps/oxlint/src/lint.rs index 62d6fbd969d47..8b1c29621ef07 100644 --- a/apps/oxlint/src/lint.rs +++ b/apps/oxlint/src/lint.rs @@ -12,6 +12,7 @@ use oxc_linter::{ LintFilter, LintOptions, LintService, LintServiceOptions, Linter, Oxlintrc, }; use oxc_span::VALID_EXTENSIONS; +use serde_json::Value; use crate::{ cli::{CliRunResult, LintCommand, LintResult, MiscOptions, Runner, WarningOptions}, @@ -132,7 +133,23 @@ impl Runner for LintRunner { if misc_options.print_config { return CliRunResult::PrintConfigResult { config_file }; } else if basic_options.init { - match fs::write(Self::DEFAULT_OXLINTRC, config_file) { + let schema_relative_path = "node_modules/oxlint/configuration_schema.json"; + let configuration = if self.cwd.join(schema_relative_path).is_file() { + let mut config_json: Value = serde_json::from_str(&config_file).unwrap(); + if let Value::Object(ref mut obj) = config_json { + let mut json_object = serde_json::Map::new(); + json_object.insert( + "$schema".to_string(), + format!("./{schema_relative_path}").into(), + ); + json_object.extend(obj.clone()); + *obj = json_object; + } + serde_json::to_string_pretty(&config_json).unwrap() + } else { + config_file + }; + match fs::write(Self::DEFAULT_OXLINTRC, configuration) { Ok(()) => { return CliRunResult::ConfigFileInitResult { message: "Configuration file created".to_string(), From 40316afa7e1c08bb57e2074a53c84fad8eecd6ce Mon Sep 17 00:00:00 2001 From: "Alexander S." Date: Wed, 22 Jan 2025 02:10:40 +0100 Subject: [PATCH 03/14] fix(linter): fix github `endColumn` output (#8647) added `start` and `end` for `Info`, so every reporter can use both if they want. Then end calculation is a bit hacky, but i looks like it works. --- .../oxlint/src/output_formatter/checkstyle.rs | 4 +-- apps/oxlint/src/output_formatter/github.rs | 10 ++++-- apps/oxlint/src/output_formatter/stylish.rs | 2 +- apps/oxlint/src/output_formatter/unix.rs | 4 +-- crates/oxc_diagnostics/src/reporter.rs | 32 +++++++++++++++---- 5 files changed, 37 insertions(+), 15 deletions(-) diff --git a/apps/oxlint/src/output_formatter/checkstyle.rs b/apps/oxlint/src/output_formatter/checkstyle.rs index 05655fb3dfad4..adcbd4eed824a 100644 --- a/apps/oxlint/src/output_formatter/checkstyle.rs +++ b/apps/oxlint/src/output_formatter/checkstyle.rs @@ -51,14 +51,14 @@ fn format_checkstyle(diagnostics: &[Error]) -> String { let messages = infos .iter() .fold(String::new(), |mut acc, info| { - let Info { line, column, message, severity, rule_id, .. } = info; + let Info { start, message, severity, rule_id, .. } = info; let severity = match severity { Severity::Error => "error", _ => "warning", }; let message = rule_id.as_ref().map_or_else(|| xml_escape(message), |rule_id| Cow::Owned(format!("{} ({rule_id})", xml_escape(message)))); let source = rule_id.as_ref().map_or_else(|| Cow::Borrowed(""), |rule_id| Cow::Owned(format!("eslint.rules.{rule_id}"))); - let line = format!(r#""#); + let line = format!(r#""#, start.line, start.column); acc.push_str(&line); acc }); diff --git a/apps/oxlint/src/output_formatter/github.rs b/apps/oxlint/src/output_formatter/github.rs index d6b9a33481835..f5739366e0641 100644 --- a/apps/oxlint/src/output_formatter/github.rs +++ b/apps/oxlint/src/output_formatter/github.rs @@ -35,7 +35,7 @@ impl DiagnosticReporter for GithubReporter { } fn format_github(diagnostic: &Error) -> String { - let Info { line, column, filename, message, severity, rule_id } = Info::new(diagnostic); + let Info { start, end, filename, message, severity, rule_id } = Info::new(diagnostic); let severity = match severity { Severity::Error => "error", Severity::Warning | miette::Severity::Advice => "warning", @@ -44,7 +44,11 @@ fn format_github(diagnostic: &Error) -> String { let filename = escape_property(&filename); let message = escape_data(&message); format!( - "::{severity} file={filename},line={line},endLine={line},col={column},endColumn={column},title={title}::{message}\n" + "::{severity} file={filename},line={},endLine={},col={},endColumn={},title={title}::{message}\n", + start.line, + end.line, + start.column, + end.column ) } @@ -108,6 +112,6 @@ mod test { let result = reporter.render_error(error); assert!(result.is_some()); - assert_eq!(result.unwrap(), "::warning file=file%3A//test.ts,line=1,endLine=1,col=1,endColumn=1,title=oxlint::error message\n"); + assert_eq!(result.unwrap(), "::warning file=file%3A//test.ts,line=1,endLine=1,col=1,endColumn=9,title=oxlint::error message\n"); } } diff --git a/apps/oxlint/src/output_formatter/stylish.rs b/apps/oxlint/src/output_formatter/stylish.rs index e6669d61ed82a..7f9e4e2e17bb9 100644 --- a/apps/oxlint/src/output_formatter/stylish.rs +++ b/apps/oxlint/src/output_formatter/stylish.rs @@ -49,7 +49,7 @@ fn format_stylish(diagnostics: &[Error]) -> String { let mut grouped: FxHashMap> = FxHashMap::default(); let mut sorted = diagnostics.iter().collect::>(); - sorted.sort_by_key(|diagnostic| Info::new(diagnostic).line); + sorted.sort_by_key(|diagnostic| Info::new(diagnostic).start.line); for diagnostic in sorted { let info = Info::new(diagnostic); diff --git a/apps/oxlint/src/output_formatter/unix.rs b/apps/oxlint/src/output_formatter/unix.rs index 4c34f334ebd4d..12cfa99699602 100644 --- a/apps/oxlint/src/output_formatter/unix.rs +++ b/apps/oxlint/src/output_formatter/unix.rs @@ -45,14 +45,14 @@ impl DiagnosticReporter for UnixReporter { /// fn format_unix(diagnostic: &Error) -> String { - let Info { line, column, filename, message, severity, rule_id } = Info::new(diagnostic); + let Info { start, end: _, filename, message, severity, rule_id } = Info::new(diagnostic); let severity = match severity { Severity::Error => "Error", _ => "Warning", }; let rule_id = rule_id.map_or_else(|| Cow::Borrowed(""), |rule_id| Cow::Owned(format!("/{rule_id}"))); - format!("{filename}:{line}:{column}: {message} [{severity}{rule_id}]\n") + format!("{filename}:{}:{}: {message} [{severity}{rule_id}]\n", start.line, start.column) } #[cfg(test)] diff --git a/crates/oxc_diagnostics/src/reporter.rs b/crates/oxc_diagnostics/src/reporter.rs index 34443c273a934..ffe44986c4586 100644 --- a/crates/oxc_diagnostics/src/reporter.rs +++ b/crates/oxc_diagnostics/src/reporter.rs @@ -1,5 +1,7 @@ //! [Reporters](DiagnosticReporter) for rendering and writing diagnostics. +use miette::SourceSpan; + use crate::{Error, Severity}; /// Reporters are responsible for rendering diagnostics to some format and writing them to some @@ -54,18 +56,23 @@ pub trait DiagnosticReporter { } pub struct Info { - pub line: usize, - pub column: usize, + pub start: InfoPosition, + pub end: InfoPosition, pub filename: String, pub message: String, pub severity: Severity, pub rule_id: Option, } +pub struct InfoPosition { + pub line: usize, + pub column: usize, +} + impl Info { pub fn new(diagnostic: &Error) -> Self { - let mut line = 0; - let mut column = 0; + let mut start = InfoPosition { line: 0, column: 0 }; + let mut end = InfoPosition { line: 0, column: 0 }; let mut filename = String::new(); let mut message = String::new(); let mut severity = Severity::Warning; @@ -74,8 +81,18 @@ impl Info { if let Some(source) = diagnostic.source_code() { if let Some(label) = labels.next() { if let Ok(span_content) = source.read_span(label.inner(), 0, 0) { - line = span_content.line() + 1; - column = span_content.column() + 1; + start.line = span_content.line() + 1; + start.column = span_content.column() + 1; + + let end_offset = label.inner().offset() + label.inner().len(); + + if let Ok(span_content) = + source.read_span(&SourceSpan::from((end_offset, 0)), 0, 0) + { + end.line = span_content.line() + 1; + end.column = span_content.column() + 1; + } + if let Some(name) = span_content.name() { filename = name.to_string(); }; @@ -92,6 +109,7 @@ impl Info { } } } - Self { line, column, filename, message, severity, rule_id } + + Self { start, end, filename, message, severity, rule_id } } } From 00dc63f6a51cf2024c34227657de8e363a687e44 Mon Sep 17 00:00:00 2001 From: Boshen <1430279+Boshen@users.noreply.github.com> Date: Wed, 22 Jan 2025 02:49:52 +0000 Subject: [PATCH 04/14] perf(minifier): only substitute typed array constructor once (#8649) --- .../src/peephole/substitute_alternate_syntax.rs | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/crates/oxc_minifier/src/peephole/substitute_alternate_syntax.rs b/crates/oxc_minifier/src/peephole/substitute_alternate_syntax.rs index 3e9e627576401..207a2763b8ece 100644 --- a/crates/oxc_minifier/src/peephole/substitute_alternate_syntax.rs +++ b/crates/oxc_minifier/src/peephole/substitute_alternate_syntax.rs @@ -123,9 +123,6 @@ impl<'a, 'b> PeepholeOptimizations { self.try_compress_normal_assignment_to_combined_assignment(e, ctx); self.try_compress_normal_assignment_to_combined_logical_assignment(e, ctx); } - Expression::NewExpression(e) => { - self.try_compress_typed_array_constructor(e, ctx); - } _ => {} } @@ -165,6 +162,10 @@ impl<'a, 'b> PeepholeOptimizations { return; } + if let Expression::NewExpression(e) = expr { + self.try_compress_typed_array_constructor(e, ctx); + } + if let Some(folded_expr) = match expr { Expression::Identifier(ident) => self.try_compress_undefined(ident, ctx), Expression::BooleanLiteral(_) => self.try_compress_boolean(expr, ctx), @@ -1219,17 +1220,16 @@ impl<'a, 'b> PeepholeOptimizations { e: &mut NewExpression<'a>, ctx: Ctx<'a, 'b>, ) { + debug_assert!(!self.in_fixed_loop); let Expression::Identifier(ident) = &e.callee else { return }; let name = ident.name.as_str(); if !Self::is_typed_array_name(name) || !ctx.is_global_reference(ident) { return; } - if e.arguments.len() == 1 && e.arguments[0].as_expression().is_some_and(Expression::is_number_0) { e.arguments.clear(); - self.mark_current_function_as_changed(); } } From 9953ac7cadfbe0660e7c42d971b884055e352a23 Mon Sep 17 00:00:00 2001 From: Boshen Date: Wed, 22 Jan 2025 16:19:06 +0800 Subject: [PATCH 05/14] perf(minifier): add `LatePeepholeOptimizations` (#8651) This PR adds a `LatePeepholeOptimizations` pass for minifications that don't interact with the fixed point loop. While working on this I found a couple of cases where the previous fixed point loop is not idempotent. --- .../src/constant_evaluation/mod.rs | 26 +- crates/oxc_minifier/src/compressor.rs | 9 +- .../peephole/convert_to_dotted_properties.rs | 24 +- .../src/peephole/fold_constants.rs | 15 +- .../src/peephole/minimize_conditions.rs | 56 ++-- crates/oxc_minifier/src/peephole/mod.rs | 66 ++-- .../src/peephole/remove_dead_code.rs | 16 +- .../peephole/substitute_alternate_syntax.rs | 296 ++++++++---------- crates/oxc_minifier/tests/ast_passes/mod.rs | 19 +- tasks/minsize/minsize.snap | 10 +- 10 files changed, 271 insertions(+), 266 deletions(-) diff --git a/crates/oxc_ecmascript/src/constant_evaluation/mod.rs b/crates/oxc_ecmascript/src/constant_evaluation/mod.rs index 22ba2b37e9da0..7f69447e1f863 100644 --- a/crates/oxc_ecmascript/src/constant_evaluation/mod.rs +++ b/crates/oxc_ecmascript/src/constant_evaluation/mod.rs @@ -188,6 +188,7 @@ pub trait ConstantEvaluation<'a> { Some(ConstantValue::String(Cow::Borrowed(lit.value.as_str()))) } Expression::StaticMemberExpression(e) => self.eval_static_member_expression(e), + Expression::ComputedMemberExpression(e) => self.eval_computed_member_expression(e), _ => None, } } @@ -421,7 +422,30 @@ pub trait ConstantEvaluation<'a> { match expr.property.name.as_str() { "length" => { if let Some(ConstantValue::String(s)) = self.eval_expression(&expr.object) { - // TODO(perf): no need to actually convert, only need the length + Some(ConstantValue::Number(s.encode_utf16().count().to_f64().unwrap())) + } else { + if expr.object.may_have_side_effects() { + return None; + } + + if let Expression::ArrayExpression(arr) = &expr.object { + Some(ConstantValue::Number(arr.elements.len().to_f64().unwrap())) + } else { + None + } + } + } + _ => None, + } + } + + fn eval_computed_member_expression( + &self, + expr: &ComputedMemberExpression<'a>, + ) -> Option> { + match &expr.expression { + Expression::StringLiteral(s) if s.value == "length" => { + if let Some(ConstantValue::String(s)) = self.eval_expression(&expr.object) { Some(ConstantValue::Number(s.encode_utf16().count().to_f64().unwrap())) } else { if expr.object.may_have_side_effects() { diff --git a/crates/oxc_minifier/src/compressor.rs b/crates/oxc_minifier/src/compressor.rs index 59df444b541db..c9514c26cb5f1 100644 --- a/crates/oxc_minifier/src/compressor.rs +++ b/crates/oxc_minifier/src/compressor.rs @@ -4,7 +4,10 @@ use oxc_semantic::{ScopeTree, SemanticBuilder, SymbolTable}; use oxc_traverse::ReusableTraverseCtx; use crate::{ - peephole::{DeadCodeElimination, Normalize, NormalizeOptions, PeepholeOptimizations}, + peephole::{ + DeadCodeElimination, LatePeepholeOptimizations, Normalize, NormalizeOptions, + PeepholeOptimizations, + }, CompressOptions, }; @@ -33,8 +36,8 @@ impl<'a> Compressor<'a> { let mut ctx = ReusableTraverseCtx::new(scopes, symbols, self.allocator); let normalize_options = NormalizeOptions { convert_while_to_fors: true }; Normalize::new(normalize_options, self.options).build(program, &mut ctx); - PeepholeOptimizations::new(self.options.target, true).run_in_loop(program, &mut ctx); - PeepholeOptimizations::new(self.options.target, false).build(program, &mut ctx); + PeepholeOptimizations::new(self.options.target).run_in_loop(program, &mut ctx); + LatePeepholeOptimizations::new(self.options.target).build(program, &mut ctx); } pub fn dead_code_elimination(self, program: &mut Program<'a>) { diff --git a/crates/oxc_minifier/src/peephole/convert_to_dotted_properties.rs b/crates/oxc_minifier/src/peephole/convert_to_dotted_properties.rs index f0888e6957947..2e617b645fc49 100644 --- a/crates/oxc_minifier/src/peephole/convert_to_dotted_properties.rs +++ b/crates/oxc_minifier/src/peephole/convert_to_dotted_properties.rs @@ -2,30 +2,20 @@ use oxc_ast::ast::*; use oxc_syntax::identifier::is_identifier_name; use oxc_traverse::TraverseCtx; -use super::PeepholeOptimizations; +use super::LatePeepholeOptimizations; use crate::ctx::Ctx; -impl<'a> PeepholeOptimizations { +impl<'a> LatePeepholeOptimizations { /// Converts property accesses from quoted string or bracket access syntax to dot or unquoted string /// syntax, where possible. Dot syntax is more compact. /// /// - pub fn convert_to_dotted_properties( - &mut self, - expr: &mut MemberExpression<'a>, - ctx: &mut TraverseCtx<'a>, - ) { - if !self.in_fixed_loop { - self.try_compress_computed_member_expression(expr, Ctx(ctx)); - } - } - + /// /// `foo['bar']` -> `foo.bar` /// `foo?.['bar']` -> `foo?.bar` - fn try_compress_computed_member_expression( - &mut self, + pub fn convert_to_dotted_properties( expr: &mut MemberExpression<'a>, - ctx: Ctx<'a, '_>, + ctx: &mut TraverseCtx<'a>, ) { let MemberExpression::ComputedMemberExpression(e) = expr else { return }; let Expression::StringLiteral(s) = &e.expression else { return }; @@ -35,7 +25,6 @@ impl<'a> PeepholeOptimizations { *expr = MemberExpression::StaticMemberExpression( ctx.ast.alloc_static_member_expression(e.span, object, property, e.optional), ); - self.mark_current_function_as_changed(); return; } let v = s.value.as_str(); @@ -43,7 +32,8 @@ impl<'a> PeepholeOptimizations { return; } if let Some(n) = Ctx::string_to_equivalent_number_value(v) { - e.expression = ctx.ast.expression_numeric_literal(s.span, n, None, NumberBase::Decimal); + e.expression = + Ctx(ctx).ast.expression_numeric_literal(s.span, n, None, NumberBase::Decimal); } } } diff --git a/crates/oxc_minifier/src/peephole/fold_constants.rs b/crates/oxc_minifier/src/peephole/fold_constants.rs index ef2d403f170ad..c864e9b2a8e16 100644 --- a/crates/oxc_minifier/src/peephole/fold_constants.rs +++ b/crates/oxc_minifier/src/peephole/fold_constants.rs @@ -29,6 +29,7 @@ impl<'a, 'b> PeepholeOptimizations { .or_else(|| Self::try_fold_binary_typeof_comparison(e, ctx)), Expression::UnaryExpression(e) => Self::try_fold_unary_expr(e, ctx), Expression::StaticMemberExpression(e) => Self::try_fold_static_member_expr(e, ctx), + Expression::ComputedMemberExpression(e) => Self::try_fold_computed_member_expr(e, ctx), Expression::LogicalExpression(e) => Self::try_fold_logical_expr(e, ctx), Expression::ChainExpression(e) => Self::try_fold_optional_chain(e, ctx), Expression::CallExpression(e) => Self::try_fold_number_constructor(e, ctx), @@ -56,12 +57,19 @@ impl<'a, 'b> PeepholeOptimizations { } fn try_fold_static_member_expr( - static_member_expr: &mut StaticMemberExpression<'a>, + e: &mut StaticMemberExpression<'a>, ctx: Ctx<'a, 'b>, ) -> Option> { // TODO: tryFoldObjectPropAccess(n, left, name) - ctx.eval_static_member_expression(static_member_expr) - .map(|value| ctx.value_to_expr(static_member_expr.span, value)) + ctx.eval_static_member_expression(e).map(|value| ctx.value_to_expr(e.span, value)) + } + + fn try_fold_computed_member_expr( + e: &mut ComputedMemberExpression<'a>, + ctx: Ctx<'a, 'b>, + ) -> Option> { + // TODO: tryFoldObjectPropAccess(n, left, name) + ctx.eval_computed_member_expression(e).map(|value| ctx.value_to_expr(e.span, value)) } fn try_fold_logical_expr( @@ -1618,6 +1626,7 @@ mod test { test("x = [].length", "x = 0"); test("x = [1,2,3].length", "x = 3"); // test("x = [a,b].length", "x = 2"); + test("x = 'abc'['length']", "x = 3"); // Not handled yet test("x = [,,1].length", "x = 3"); diff --git a/crates/oxc_minifier/src/peephole/minimize_conditions.rs b/crates/oxc_minifier/src/peephole/minimize_conditions.rs index c6bf934bc1355..b456e68086012 100644 --- a/crates/oxc_minifier/src/peephole/minimize_conditions.rs +++ b/crates/oxc_minifier/src/peephole/minimize_conditions.rs @@ -1,8 +1,9 @@ use oxc_allocator::Vec; use oxc_ast::{ast::*, NONE}; use oxc_ecmascript::constant_evaluation::{ConstantEvaluation, ValueType}; +use oxc_semantic::ReferenceFlags; use oxc_span::{cmp::ContentEq, GetSpan}; -use oxc_traverse::{Ancestor, TraverseCtx}; +use oxc_traverse::{Ancestor, MaybeBoundIdentifier, TraverseCtx}; use crate::ctx::Ctx; @@ -22,15 +23,15 @@ impl<'a> PeepholeOptimizations { ctx: &mut TraverseCtx<'a>, ) { let mut changed = false; - let mut changed2 = false; - Self::try_replace_if(stmts, &mut changed2, ctx); - while changed2 { - changed2 = false; - Self::try_replace_if(stmts, &mut changed2, ctx); - if stmts.iter().any(|stmt| matches!(stmt, Statement::EmptyStatement(_))) { + loop { + let mut local_change = false; + Self::try_replace_if(stmts, &mut local_change, ctx); + if local_change { stmts.retain(|stmt| !matches!(stmt, Statement::EmptyStatement(_))); + changed = local_change; + } else { + break; } - changed = changed2; } if changed { self.mark_current_function_as_changed(); @@ -77,25 +78,27 @@ impl<'a> PeepholeOptimizations { expr: &mut Expression<'a>, ctx: &mut TraverseCtx<'a>, ) { + let mut changed = false; loop { - let mut changed = false; + let mut local_change = false; if let Expression::ConditionalExpression(logical_expr) = expr { + if Self::try_fold_expr_in_boolean_context(&mut logical_expr.test, Ctx(ctx)) { + local_change = true; + } if let Some(e) = Self::try_minimize_conditional(logical_expr, ctx) { *expr = e; - changed = true; - } - } - if let Expression::ConditionalExpression(logical_expr) = expr { - if Self::try_fold_expr_in_boolean_context(&mut logical_expr.test, Ctx(ctx)) { - changed = true; + local_change = true; } } - if changed { - self.mark_current_function_as_changed(); + if local_change { + changed = true; } else { break; } } + if changed { + self.mark_current_function_as_changed(); + } if let Some(folded_expr) = match expr { Expression::UnaryExpression(e) => Self::try_minimize_not(e, ctx), @@ -124,6 +127,15 @@ impl<'a> PeepholeOptimizations { e.operator = e.operator.equality_inverse_operator().unwrap(); Some(ctx.ast.move_expression(&mut expr.argument)) } + Expression::ConditionalExpression(conditional_expr) => { + if let Expression::BinaryExpression(e) = &mut conditional_expr.test { + if e.operator.is_equality() { + e.operator = e.operator.equality_inverse_operator().unwrap(); + return Some(ctx.ast.move_expression(&mut expr.argument)); + } + } + None + } _ => None, } } @@ -331,9 +343,13 @@ impl<'a> PeepholeOptimizations { unreachable!() }; let return_stmt = return_stmt.unbox(); - match return_stmt.argument { - Some(e) => e, - None => ctx.ast.void_0(return_stmt.span), + if let Some(e) = return_stmt.argument { + e + } else { + let name = "undefined"; + let symbol_id = ctx.scopes().find_binding(ctx.current_scope_id(), name); + let ident = MaybeBoundIdentifier::new(Atom::from(name), symbol_id); + ident.create_expression(ReferenceFlags::read(), ctx) } } diff --git a/crates/oxc_minifier/src/peephole/mod.rs b/crates/oxc_minifier/src/peephole/mod.rs index de00649ac2606..72223943c40ca 100644 --- a/crates/oxc_minifier/src/peephole/mod.rs +++ b/crates/oxc_minifier/src/peephole/mod.rs @@ -20,10 +20,6 @@ use rustc_hash::FxHashSet; pub struct PeepholeOptimizations { target: ESTarget, - /// `in_fixed_loop`: Do not compress syntaxes that are hard to analyze inside the fixed loop. - /// Opposite of `late` in Closure Compiler. - in_fixed_loop: bool, - /// Walk the ast in a fixed point loop until no changes are made. /// `prev_function_changed`, `functions_changed` and `current_function` track changes /// in top level and each function. No minification code are run if the function is not changed @@ -37,10 +33,9 @@ pub struct PeepholeOptimizations { } impl<'a> PeepholeOptimizations { - pub fn new(target: ESTarget, in_fixed_loop: bool) -> Self { + pub fn new(target: ESTarget) -> Self { Self { target, - in_fixed_loop, iteration: 0, prev_functions_changed: FxHashSet::default(), functions_changed: FxHashSet::default(), @@ -84,7 +79,7 @@ impl<'a> PeepholeOptimizations { } fn is_prev_function_changed(&self) -> bool { - if !self.in_fixed_loop || self.iteration == 0 { + if self.iteration == 0 { return true; } if let Some((_, prev_changed, _)) = self.current_function_stack.last() { @@ -159,13 +154,6 @@ impl<'a> Traverse<'a> for PeepholeOptimizations { self.minimize_exit_points(body, ctx); } - fn exit_class_body(&mut self, body: &mut ClassBody<'a>, ctx: &mut TraverseCtx<'a>) { - if !self.is_prev_function_changed() { - return; - } - self.remove_dead_code_exit_class_body(body, ctx); - } - fn exit_variable_declaration( &mut self, decl: &mut VariableDeclaration<'a>, @@ -195,17 +183,6 @@ impl<'a> Traverse<'a> for PeepholeOptimizations { self.substitute_call_expression(expr, ctx); } - fn exit_member_expression( - &mut self, - expr: &mut MemberExpression<'a>, - ctx: &mut TraverseCtx<'a>, - ) { - if !self.is_prev_function_changed() { - return; - } - self.convert_to_dotted_properties(expr, ctx); - } - fn exit_object_property(&mut self, prop: &mut ObjectProperty<'a>, ctx: &mut TraverseCtx<'a>) { if !self.is_prev_function_changed() { return; @@ -263,11 +240,42 @@ impl<'a> Traverse<'a> for PeepholeOptimizations { } self.substitute_accessor_property(prop, ctx); } +} + +/// Changes that do not interfere with optimizations that are run inside the fixed-point loop, +/// which can be done as a last AST pass. +pub struct LatePeepholeOptimizations { + target: ESTarget, +} + +impl<'a> LatePeepholeOptimizations { + pub fn new(target: ESTarget) -> Self { + Self { target } + } + + pub fn build(&mut self, program: &mut Program<'a>, ctx: &mut ReusableTraverseCtx<'a>) { + traverse_mut_with_ctx(self, program, ctx); + } +} + +impl<'a> Traverse<'a> for LatePeepholeOptimizations { + fn exit_member_expression( + &mut self, + expr: &mut MemberExpression<'a>, + ctx: &mut TraverseCtx<'a>, + ) { + Self::convert_to_dotted_properties(expr, ctx); + } + + fn exit_class_body(&mut self, body: &mut ClassBody<'a>, ctx: &mut TraverseCtx<'a>) { + Self::remove_dead_code_exit_class_body(body, ctx); + } + + fn exit_expression(&mut self, expr: &mut Expression<'a>, ctx: &mut TraverseCtx<'a>) { + Self::substitute_exit_expression(expr, ctx); + } fn exit_catch_clause(&mut self, catch: &mut CatchClause<'a>, ctx: &mut TraverseCtx<'a>) { - if !self.is_prev_function_changed() { - return; - } self.substitute_catch_clause(catch, ctx); } } @@ -278,7 +286,7 @@ pub struct DeadCodeElimination { impl<'a> DeadCodeElimination { pub fn new() -> Self { - Self { inner: PeepholeOptimizations::new(ESTarget::ESNext, false) } + Self { inner: PeepholeOptimizations::new(ESTarget::ESNext) } } pub fn build(&mut self, program: &mut Program<'a>, ctx: &mut ReusableTraverseCtx<'a>) { diff --git a/crates/oxc_minifier/src/peephole/remove_dead_code.rs b/crates/oxc_minifier/src/peephole/remove_dead_code.rs index 9681cf5d488e1..e3cdbba8feba1 100644 --- a/crates/oxc_minifier/src/peephole/remove_dead_code.rs +++ b/crates/oxc_minifier/src/peephole/remove_dead_code.rs @@ -9,7 +9,7 @@ use oxc_traverse::{Ancestor, TraverseCtx}; use crate::{ctx::Ctx, keep_var::KeepVar}; -use super::PeepholeOptimizations; +use super::{LatePeepholeOptimizations, PeepholeOptimizations}; /// Remove Dead Code from the AST. /// @@ -74,16 +74,6 @@ impl<'a, 'b> PeepholeOptimizations { } } - pub fn remove_dead_code_exit_class_body( - &mut self, - body: &mut ClassBody<'a>, - _ctx: &mut TraverseCtx<'a>, - ) { - if !self.in_fixed_loop { - Self::remove_empty_class_static_block(body); - } - } - /// Removes dead code thats comes after `return` statements after inlining `if` statements fn dead_code_elimination(&mut self, stmts: &mut Vec<'a, Statement<'a>>, ctx: Ctx<'a, 'b>) { // Remove code after `return` and `throw` statements @@ -581,8 +571,10 @@ impl<'a, 'b> PeepholeOptimizations { }; (params_empty && body_empty).then(|| ctx.ast.statement_empty(e.span)) } +} - fn remove_empty_class_static_block(body: &mut ClassBody<'a>) { +impl<'a> LatePeepholeOptimizations { + pub fn remove_dead_code_exit_class_body(body: &mut ClassBody<'a>, _ctx: &mut TraverseCtx<'a>) { body.body.retain(|e| !matches!(e, ClassElement::StaticBlock(s) if s.body.is_empty())); } } diff --git a/crates/oxc_minifier/src/peephole/substitute_alternate_syntax.rs b/crates/oxc_minifier/src/peephole/substitute_alternate_syntax.rs index 207a2763b8ece..9055e21b148b0 100644 --- a/crates/oxc_minifier/src/peephole/substitute_alternate_syntax.rs +++ b/crates/oxc_minifier/src/peephole/substitute_alternate_syntax.rs @@ -18,21 +18,13 @@ use oxc_traverse::{Ancestor, TraverseCtx}; use crate::ctx::Ctx; -use super::PeepholeOptimizations; +use super::{LatePeepholeOptimizations, PeepholeOptimizations}; /// A peephole optimization that minimizes code by simplifying conditional /// expressions, replacing IFs with HOOKs, replacing object constructors /// with literals, and simplifying returns. /// -impl<'a, 'b> PeepholeOptimizations { - pub fn substitute_catch_clause( - &mut self, - catch: &mut CatchClause<'a>, - ctx: &mut TraverseCtx<'a>, - ) { - self.compress_catch_clause(catch, ctx); - } - +impl<'a> PeepholeOptimizations { pub fn substitute_object_property( &mut self, prop: &mut ObjectProperty<'a>, @@ -156,38 +148,6 @@ impl<'a, 'b> PeepholeOptimizations { *expr = folded_expr; self.mark_current_function_as_changed(); } - - // Out of fixed loop syntax changes happen last. - if self.in_fixed_loop { - return; - } - - if let Expression::NewExpression(e) = expr { - self.try_compress_typed_array_constructor(e, ctx); - } - - if let Some(folded_expr) = match expr { - Expression::Identifier(ident) => self.try_compress_undefined(ident, ctx), - Expression::BooleanLiteral(_) => self.try_compress_boolean(expr, ctx), - _ => None, - } { - *expr = folded_expr; - self.mark_current_function_as_changed(); - } - } - - fn compress_catch_clause(&mut self, catch: &mut CatchClause<'_>, ctx: &mut TraverseCtx<'a>) { - if !self.in_fixed_loop && self.target >= ESTarget::ES2019 { - if let Some(param) = &catch.param { - if let BindingPatternKind::BindingIdentifier(ident) = ¶m.pattern.kind { - if catch.body.body.is_empty() - || ctx.symbols().get_resolved_references(ident.symbol_id()).count() == 0 - { - catch.param = None; - } - }; - } - } } fn swap_binary_expressions(e: &mut BinaryExpression<'a>) { @@ -199,68 +159,11 @@ impl<'a, 'b> PeepholeOptimizations { } } - /// Transforms `undefined` => `void 0` - fn try_compress_undefined( - &self, - ident: &IdentifierReference<'a>, - ctx: Ctx<'a, 'b>, - ) -> Option> { - debug_assert!(!self.in_fixed_loop); - if !ctx.is_identifier_undefined(ident) { - return None; - } - // `delete undefined` returns `false` - // `delete void 0` returns `true` - if matches!(ctx.parent(), Ancestor::UnaryExpressionArgument(e) if e.operator().is_delete()) - { - return None; - } - Some(ctx.ast.void_0(ident.span)) - } - - /// Transforms boolean expression `true` => `!0` `false` => `!1`. - /// Do not compress `true` in `Object.defineProperty(exports, 'Foo', {enumerable: true, ...})`. - fn try_compress_boolean( - &self, - expr: &mut Expression<'a>, - ctx: Ctx<'a, 'b>, - ) -> Option> { - debug_assert!(!self.in_fixed_loop); - let Expression::BooleanLiteral(lit) = expr else { return None }; - let parent = ctx.ancestry.parent(); - let no_unary = { - if let Ancestor::BinaryExpressionRight(u) = parent { - !matches!( - u.operator(), - BinaryOperator::Addition // Other effect, like string concatenation. - | BinaryOperator::Instanceof // Relational operator. - | BinaryOperator::In - | BinaryOperator::StrictEquality // It checks type, so we should not fold. - | BinaryOperator::StrictInequality - ) - } else { - false - } - }; - // XOR: We should use `!neg` when it is not in binary expression. - let num = ctx.ast.expression_numeric_literal( - lit.span, - if lit.value ^ no_unary { 0.0 } else { 1.0 }, - None, - NumberBase::Decimal, - ); - Some(if no_unary { - num - } else { - ctx.ast.expression_unary(lit.span, UnaryOperator::LogicalNot, num) - }) - } - /// `() => { return foo })` -> `() => foo` fn try_compress_arrow_expression( &mut self, arrow_expr: &mut ArrowFunctionExpression<'a>, - ctx: Ctx<'a, 'b>, + ctx: Ctx<'a, '_>, ) { if !arrow_expr.expression && arrow_expr.body.directives.is_empty() @@ -290,7 +193,7 @@ impl<'a, 'b> PeepholeOptimizations { /// Enabled by `compress.typeofs` fn try_compress_typeof_undefined( expr: &mut BinaryExpression<'a>, - ctx: Ctx<'a, 'b>, + ctx: Ctx<'a, '_>, ) -> Option> { let Expression::UnaryExpression(unary_expr) = &expr.left else { return None }; if !unary_expr.operator.is_typeof() { @@ -343,7 +246,7 @@ impl<'a, 'b> PeepholeOptimizations { /// - `document.all == null` is `true` fn try_compress_is_null_or_undefined( expr: &mut LogicalExpression<'a>, - ctx: Ctx<'a, 'b>, + ctx: Ctx<'a, '_>, ) -> Option> { let op = expr.operator; let target_ops = match op { @@ -389,7 +292,7 @@ impl<'a, 'b> PeepholeOptimizations { right: &mut Expression<'a>, span: Span, (find_op, replace_op): (BinaryOperator, BinaryOperator), - ctx: Ctx<'a, 'b>, + ctx: Ctx<'a, '_>, ) -> Option> { enum LeftPairValueResult { Null(Span), @@ -478,7 +381,7 @@ impl<'a, 'b> PeepholeOptimizations { fn try_compress_logical_expression_to_assignment_expression( &self, expr: &mut LogicalExpression<'a>, - ctx: Ctx<'a, 'b>, + ctx: Ctx<'a, '_>, ) -> Option> { if self.target < ESTarget::ES2020 { return None; @@ -509,7 +412,7 @@ impl<'a, 'b> PeepholeOptimizations { /// `a || (b || c);` -> `(a || b) || c;` fn try_rotate_logical_expression( expr: &mut LogicalExpression<'a>, - ctx: Ctx<'a, 'b>, + ctx: Ctx<'a, '_>, ) -> Option> { let Expression::LogicalExpression(right) = &mut expr.right else { return None }; if right.operator != expr.operator { @@ -551,7 +454,7 @@ impl<'a, 'b> PeepholeOptimizations { fn has_no_side_effect_for_evaluation_same_target( assignment_target: &AssignmentTarget, expr: &Expression, - ctx: Ctx<'a, 'b>, + ctx: Ctx<'a, '_>, ) -> bool { match (&assignment_target, &expr) { ( @@ -642,7 +545,7 @@ impl<'a, 'b> PeepholeOptimizations { left: &Expression<'a>, right: &Expression<'a>, span: Span, - ctx: Ctx<'a, 'b>, + ctx: Ctx<'a, '_>, inversed: bool, ) -> Option> { let pair = Self::commutative_pair( @@ -767,7 +670,7 @@ impl<'a, 'b> PeepholeOptimizations { fn try_fold_loose_equals_undefined( e: &mut BinaryExpression<'a>, - ctx: Ctx<'a, 'b>, + ctx: Ctx<'a, '_>, ) -> Option> { // `foo == void 0` -> `foo == null`, `foo == undefined` -> `foo == null` // `foo != void 0` -> `foo == null`, `foo == undefined` -> `foo == null` @@ -824,7 +727,7 @@ impl<'a, 'b> PeepholeOptimizations { fn compress_variable_declarator( &mut self, decl: &mut VariableDeclarator<'a>, - ctx: Ctx<'a, 'b>, + ctx: Ctx<'a, '_>, ) { // Destructuring Pattern has error throwing side effect. if decl.kind.is_const() || decl.id.kind.is_destructuring_pattern() { @@ -842,7 +745,7 @@ impl<'a, 'b> PeepholeOptimizations { fn try_compress_normal_assignment_to_combined_assignment( &mut self, expr: &mut AssignmentExpression<'a>, - ctx: Ctx<'a, 'b>, + ctx: Ctx<'a, '_>, ) { if !matches!(expr.operator, AssignmentOperator::Assign) { return; @@ -867,7 +770,7 @@ impl<'a, 'b> PeepholeOptimizations { fn try_compress_normal_assignment_to_combined_logical_assignment( &mut self, expr: &mut AssignmentExpression<'a>, - ctx: Ctx<'a, 'b>, + ctx: Ctx<'a, '_>, ) { if self.target < ESTarget::ES2020 { return; @@ -899,7 +802,7 @@ impl<'a, 'b> PeepholeOptimizations { fn try_compress_assignment_to_update_expression( expr: &mut AssignmentExpression<'a>, - ctx: Ctx<'a, 'b>, + ctx: Ctx<'a, '_>, ) -> Option> { let target = expr.left.as_simple_assignment_target_mut()?; if !matches!(expr.operator, AssignmentOperator::Subtraction) { @@ -936,7 +839,7 @@ impl<'a, 'b> PeepholeOptimizations { /// - `x ? a = 0 : a = 1` -> `a = x ? 0 : 1` fn try_merge_conditional_expression_inside( expr: &mut ConditionalExpression<'a>, - ctx: Ctx<'a, 'b>, + ctx: Ctx<'a, '_>, ) -> Option> { let ( Expression::AssignmentExpression(consequent), @@ -979,7 +882,7 @@ impl<'a, 'b> PeepholeOptimizations { /// `BigInt(1)` -> `1` fn try_fold_simple_function_call( call_expr: &mut CallExpression<'a>, - ctx: Ctx<'a, 'b>, + ctx: Ctx<'a, '_>, ) -> Option> { if call_expr.optional || call_expr.arguments.len() >= 2 { return None; @@ -1060,7 +963,7 @@ impl<'a, 'b> PeepholeOptimizations { } /// Fold `Object` or `Array` constructor - fn get_fold_constructor_name(callee: &Expression<'a>, ctx: Ctx<'a, 'b>) -> Option<&'a str> { + fn get_fold_constructor_name(callee: &Expression<'a>, ctx: Ctx<'a, '_>) -> Option<&'a str> { match callee { Expression::StaticMemberExpression(e) => { if !matches!(&e.object, Expression::Identifier(ident) if ident.name == "window") { @@ -1088,7 +991,7 @@ impl<'a, 'b> PeepholeOptimizations { span: Span, name: &'a str, args: &mut Vec<'a, Argument<'a>>, - ctx: Ctx<'a, 'b>, + ctx: Ctx<'a, '_>, ) -> Option> { match name { "Object" if args.is_empty() => { @@ -1161,7 +1064,7 @@ impl<'a, 'b> PeepholeOptimizations { /// `new RegExp()` -> `RegExp()` fn try_fold_new_expression( e: &mut NewExpression<'a>, - ctx: Ctx<'a, 'b>, + ctx: Ctx<'a, '_>, ) -> Option> { let Expression::Identifier(ident) = &e.callee else { return None }; let name = ident.name.as_str(); @@ -1214,49 +1117,10 @@ impl<'a, 'b> PeepholeOptimizations { ) } - /// `new Int8Array(0)` -> `new Int8Array()` (also for other TypedArrays) - fn try_compress_typed_array_constructor( - &mut self, - e: &mut NewExpression<'a>, - ctx: Ctx<'a, 'b>, - ) { - debug_assert!(!self.in_fixed_loop); - let Expression::Identifier(ident) = &e.callee else { return }; - let name = ident.name.as_str(); - if !Self::is_typed_array_name(name) || !ctx.is_global_reference(ident) { - return; - } - if e.arguments.len() == 1 - && e.arguments[0].as_expression().is_some_and(Expression::is_number_0) - { - e.arguments.clear(); - } - } - - /// Whether the name matches any TypedArray name. - /// - /// See for the list of TypedArrays. - fn is_typed_array_name(name: &str) -> bool { - matches!( - name, - "Int8Array" - | "Uint8Array" - | "Uint8ClampedArray" - | "Int16Array" - | "Uint16Array" - | "Int32Array" - | "Uint32Array" - | "Float32Array" - | "Float64Array" - | "BigInt64Array" - | "BigUint64Array" - ) - } - fn try_compress_chain_call_expression( &mut self, chain_expr: &mut ChainExpression<'a>, - ctx: Ctx<'a, 'b>, + ctx: Ctx<'a, '_>, ) { if let ChainElement::CallExpression(call_expr) = &mut chain_expr.expression { // `window.Object?.()` -> `Object?.()` @@ -1273,7 +1137,7 @@ impl<'a, 'b> PeepholeOptimizations { } } - fn try_fold_template_literal(t: &TemplateLiteral, ctx: Ctx<'a, 'b>) -> Option> { + fn try_fold_template_literal(t: &TemplateLiteral, ctx: Ctx<'a, '_>) -> Option> { t.to_js_string().map(|val| ctx.ast.expression_string_literal(t.span(), val, None)) } @@ -1385,6 +1249,106 @@ impl<'a, 'b> PeepholeOptimizations { } } +impl<'a> LatePeepholeOptimizations { + pub fn substitute_exit_expression(expr: &mut Expression<'a>, ctx: &mut TraverseCtx<'a>) { + if let Expression::NewExpression(e) = expr { + Self::try_compress_typed_array_constructor(e, ctx); + } + + if let Some(folded_expr) = match expr { + Expression::Identifier(ident) => Self::try_compress_undefined(ident, ctx), + Expression::BooleanLiteral(_) => Self::try_compress_boolean(expr, ctx), + _ => None, + } { + *expr = folded_expr; + } + } + + /// `new Int8Array(0)` -> `new Int8Array()` (also for other TypedArrays) + fn try_compress_typed_array_constructor(e: &mut NewExpression<'a>, ctx: &mut TraverseCtx<'a>) { + let Expression::Identifier(ident) = &e.callee else { return }; + let name = ident.name.as_str(); + if !Self::is_typed_array_name(name) || !Ctx(ctx).is_global_reference(ident) { + return; + } + if e.arguments.len() == 1 + && e.arguments[0].as_expression().is_some_and(Expression::is_number_0) + { + e.arguments.clear(); + } + } + + /// Transforms `undefined` => `void 0` + fn try_compress_undefined( + ident: &IdentifierReference<'a>, + ctx: &mut TraverseCtx<'a>, + ) -> Option> { + if !Ctx(ctx).is_identifier_undefined(ident) { + return None; + } + // `delete undefined` returns `false` + // `delete void 0` returns `true` + if matches!(ctx.parent(), Ancestor::UnaryExpressionArgument(e) if e.operator().is_delete()) + { + return None; + } + Some(ctx.ast.void_0(ident.span)) + } + + /// Transforms boolean expression `true` => `!0` `false` => `!1`. + fn try_compress_boolean( + expr: &mut Expression<'a>, + ctx: &mut TraverseCtx<'a>, + ) -> Option> { + let Expression::BooleanLiteral(lit) = expr else { return None }; + let num = ctx.ast.expression_numeric_literal( + lit.span, + if lit.value { 0.0 } else { 1.0 }, + None, + NumberBase::Decimal, + ); + Some(ctx.ast.expression_unary(lit.span, UnaryOperator::LogicalNot, num)) + } + + pub fn substitute_catch_clause( + &mut self, + catch: &mut CatchClause<'a>, + ctx: &mut TraverseCtx<'a>, + ) { + if self.target >= ESTarget::ES2019 { + if let Some(param) = &catch.param { + if let BindingPatternKind::BindingIdentifier(ident) = ¶m.pattern.kind { + if catch.body.body.is_empty() + || ctx.symbols().get_resolved_references(ident.symbol_id()).count() == 0 + { + catch.param = None; + } + }; + } + } + } + + /// Whether the name matches any TypedArray name. + /// + /// See for the list of TypedArrays. + fn is_typed_array_name(name: &str) -> bool { + matches!( + name, + "Int8Array" + | "Uint8Array" + | "Uint8ClampedArray" + | "Int16Array" + | "Uint16Array" + | "Int32Array" + | "Uint32Array" + | "Float32Array" + | "Float64Array" + | "BigInt64Array" + | "BigUint64Array" + ) + } +} + /// Port from #[cfg(test)] mod test { @@ -1437,20 +1401,20 @@ mod test { #[test] fn test_fold_true_false_comparison() { - test("x == true", "x == 1"); - test("x == false", "x == 0"); - test("x != true", "x != 1"); - test("x < true", "x < 1"); - test("x <= true", "x <= 1"); - test("x > true", "x > 1"); - test("x >= true", "x >= 1"); + test("x == true", "x == !0"); + test("x == false", "x == !1"); + test("x != true", "x != !0"); + test("x < true", "x < !0"); + test("x <= true", "x <= !0"); + test("x > true", "x > !0"); + test("x >= true", "x >= !0"); test("x instanceof true", "x instanceof !0"); test("x + false", "x + !1"); // Order: should perform the nearest. test("x == x instanceof false", "x == x instanceof !1"); - test("x in x >> true", "x in x >> 1"); + test("x in x >> true", "x in x >> !0"); test("x == fake(false)", "x == fake(!1)"); // The following should not be folded. diff --git a/crates/oxc_minifier/tests/ast_passes/mod.rs b/crates/oxc_minifier/tests/ast_passes/mod.rs index 9eb056f66da34..d2073a126da43 100644 --- a/crates/oxc_minifier/tests/ast_passes/mod.rs +++ b/crates/oxc_minifier/tests/ast_passes/mod.rs @@ -42,16 +42,15 @@ fn integration() { }", ); - // FIXME - // test_idempotent( - // "require('./index.js')(function (e, os) { - // if (e) return console.log(e) - // return console.log(JSON.stringify(os)) - // })", - // r#"require("./index.js")(function(e, os) { - // return console.log(e || JSON.stringify(os)); - // });"#, - // ); + test_idempotent( + "require('./index.js')(function (e, os) { + if (e) return console.log(e) + return console.log(JSON.stringify(os)) + })", + r#"require("./index.js")(function(e, os) { + return console.log(e || JSON.stringify(os)); + });"#, + ); test_idempotent( "if (!(foo instanceof Var) || open) { diff --git a/tasks/minsize/minsize.snap b/tasks/minsize/minsize.snap index 46e5a40009af3..d4a288450060d 100644 --- a/tasks/minsize/minsize.snap +++ b/tasks/minsize/minsize.snap @@ -9,19 +9,19 @@ Original | minified | minified | gzip | gzip | Fixture 342.15 kB | 118.19 kB | 118.14 kB | 44.45 kB | 44.37 kB | vue.js -544.10 kB | 71.75 kB | 72.48 kB | 26.15 kB | 26.20 kB | lodash.js +544.10 kB | 71.74 kB | 72.48 kB | 26.14 kB | 26.20 kB | lodash.js 555.77 kB | 272.89 kB | 270.13 kB | 90.90 kB | 90.80 kB | d3.js -1.01 MB | 460.18 kB | 458.89 kB | 126.77 kB | 126.71 kB | bundle.min.js +1.01 MB | 460.18 kB | 458.89 kB | 126.78 kB | 126.71 kB | bundle.min.js 1.25 MB | 652.90 kB | 646.76 kB | 163.54 kB | 163.73 kB | three.js -2.14 MB | 724.01 kB | 724.14 kB | 179.94 kB | 181.07 kB | victory.js +2.14 MB | 724.01 kB | 724.14 kB | 179.93 kB | 181.07 kB | victory.js 3.20 MB | 1.01 MB | 1.01 MB | 332.01 kB | 331.56 kB | echarts.js -6.69 MB | 2.31 MB | 2.31 MB | 491.99 kB | 488.28 kB | antd.js +6.69 MB | 2.31 MB | 2.31 MB | 491.97 kB | 488.28 kB | antd.js -10.95 MB | 3.48 MB | 3.49 MB | 905.39 kB | 915.50 kB | typescript.js +10.95 MB | 3.48 MB | 3.49 MB | 905.34 kB | 915.50 kB | typescript.js From 50295474ccf1f316a9251cf0cfde5743f7853257 Mon Sep 17 00:00:00 2001 From: overlookmotel <557937+overlookmotel@users.noreply.github.com> Date: Wed, 22 Jan 2025 12:22:27 +0000 Subject: [PATCH 06/14] docs(semantic): fix and reformat doc comments (#8652) Update doc comment for `TempUnresolvedReferences` which was out of date. Reformat other comments. --- crates/oxc_semantic/src/unresolved_stack.rs | 22 ++++++++++++--------- 1 file changed, 13 insertions(+), 9 deletions(-) diff --git a/crates/oxc_semantic/src/unresolved_stack.rs b/crates/oxc_semantic/src/unresolved_stack.rs index ebba979791c9c..50dc8247c31c0 100644 --- a/crates/oxc_semantic/src/unresolved_stack.rs +++ b/crates/oxc_semantic/src/unresolved_stack.rs @@ -4,7 +4,8 @@ use rustc_hash::FxHashMap; use oxc_span::Atom; use oxc_syntax::reference::ReferenceId; -/// The difference with Scope's `UnresolvedReferences` is that this type uses Atom as the key. its clone is very cheap! +/// Unlike `ScopeTree`'s `UnresolvedReferences`, this type uses `Atom` as the key, +/// and uses a heap-allocated hashmap (not arena-allocated) type TempUnresolvedReferences<'a> = FxHashMap, Vec>; // Stack used to accumulate unresolved refs while traversing scopes. @@ -21,16 +22,19 @@ pub(crate) struct UnresolvedReferencesStack<'a> { } impl<'a> UnresolvedReferencesStack<'a> { - // Initial scope depth. - // Start on 1 (`Program` scope depth). - // SAFETY: Must be >= 1 to ensure soundness of `current_and_parent_mut`. + /// Initial scope depth. + /// Start on 1 (`Program` scope depth). + /// SAFETY: Must be >= 1 to ensure soundness of `current_and_parent_mut`. const INITIAL_DEPTH: usize = 1; - // Most programs will have at least 1 place where scope depth reaches 16, - // so initialize `stack` with this length, to reduce reallocations as it grows. - // This is just an estimate of a good initial size, but certainly better than - // `Vec`'s default initial capacity of 4. - // SAFETY: Must be >= 2 to ensure soundness of `current_and_parent_mut`. + + /// Most programs will have at least 1 place where scope depth reaches 16, + /// so initialize `stack` with this length, to reduce reallocations as it grows. + /// This is just an estimate of a good initial size, but certainly better than + /// `Vec`'s default initial capacity of 4. + /// SAFETY: Must be >= 2 to ensure soundness of `current_and_parent_mut`. const INITIAL_SIZE: usize = 16; + + /// Assert invariant #[allow(clippy::assertions_on_constants)] const _SIZE_CHECK: () = assert!(Self::INITIAL_SIZE > Self::INITIAL_DEPTH); From de76eb1b90a16bd54dcc66fd3f73db08792b6f30 Mon Sep 17 00:00:00 2001 From: overlookmotel <557937+overlookmotel@users.noreply.github.com> Date: Wed, 22 Jan 2025 12:29:56 +0000 Subject: [PATCH 07/14] refactor(allocator): reorder `Box` methods (#8654) Pure refactor. Move `Box::unbox` down. `Box::new_in` is the most important method, so should be at the top. --- crates/oxc_allocator/src/boxed.rs | 56 +++++++++++++++---------------- 1 file changed, 27 insertions(+), 29 deletions(-) diff --git a/crates/oxc_allocator/src/boxed.rs b/crates/oxc_allocator/src/boxed.rs index 02d6d41b443d2..e78d7191b7774 100644 --- a/crates/oxc_allocator/src/boxed.rs +++ b/crates/oxc_allocator/src/boxed.rs @@ -31,35 +31,6 @@ use crate::Allocator; /// with a [`Drop`] type. pub struct Box<'alloc, T: ?Sized>(NonNull, PhantomData<(&'alloc (), T)>); -impl Box<'_, T> { - /// Take ownership of the value stored in this [`Box`], consuming the box in - /// the process. - /// - /// # Examples - /// ``` - /// use oxc_allocator::{Allocator, Box}; - /// - /// let arena = Allocator::default(); - /// - /// // Put `5` into the arena and on the heap. - /// let boxed: Box = Box::new_in(5, &arena); - /// // Move it back to the stack. `boxed` has been consumed. - /// let i = boxed.unbox(); - /// - /// assert_eq!(i, 5); - /// ``` - #[inline] - pub fn unbox(self) -> T { - // SAFETY: - // This pointer read is safe because the reference `self.0` is - // guaranteed to be unique--not just now, but we're guaranteed it's not - // borrowed from some other reference. This in turn is because we never - // construct a `Box` with a borrowed reference, only with a fresh - // one just allocated from a Bump. - unsafe { ptr::read(self.0.as_ptr()) } - } -} - impl Box<'_, T> { /// Put a `value` into a memory arena and get back a [`Box`] with ownership /// to the allocation. @@ -97,6 +68,33 @@ impl Box<'_, T> { Self(NonNull::dangling(), PhantomData) } + + /// Take ownership of the value stored in this [`Box`], consuming the box in + /// the process. + /// + /// # Examples + /// ``` + /// use oxc_allocator::{Allocator, Box}; + /// + /// let arena = Allocator::default(); + /// + /// // Put `5` into the arena and on the heap. + /// let boxed: Box = Box::new_in(5, &arena); + /// // Move it back to the stack. `boxed` has been consumed. + /// let i = boxed.unbox(); + /// + /// assert_eq!(i, 5); + /// ``` + #[inline] + pub fn unbox(self) -> T { + // SAFETY: + // This pointer read is safe because the reference `self.0` is + // guaranteed to be unique - not just now, but we're guaranteed it's not + // borrowed from some other reference. This in turn is because we never + // construct a `Box` with a borrowed reference, only with a fresh + // one just allocated from a `Bump`. + unsafe { ptr::read(self.0.as_ptr()) } + } } impl Box<'_, T> { From d1c5dc4d12f5f32e66bc73ac662aa2f1d17f2e72 Mon Sep 17 00:00:00 2001 From: overlookmotel <557937+overlookmotel@users.noreply.github.com> Date: Wed, 22 Jan 2025 13:13:15 +0000 Subject: [PATCH 08/14] fix(semantic): fix const assertions in `UnresolvedReferencesStack` (#8653) The `_SIZE_CHECK` assertion was ignored, because `const`s are only evaluated if they're referenced in a function which is called. Fix that by referencing the const in `UnresolvedReferencesStack::new`. Also add more assertions to cover all the invariants. --- crates/oxc_semantic/src/unresolved_stack.rs | 12 +++++++++--- 1 file changed, 9 insertions(+), 3 deletions(-) diff --git a/crates/oxc_semantic/src/unresolved_stack.rs b/crates/oxc_semantic/src/unresolved_stack.rs index 50dc8247c31c0..34df37475c719 100644 --- a/crates/oxc_semantic/src/unresolved_stack.rs +++ b/crates/oxc_semantic/src/unresolved_stack.rs @@ -34,11 +34,17 @@ impl<'a> UnresolvedReferencesStack<'a> { /// SAFETY: Must be >= 2 to ensure soundness of `current_and_parent_mut`. const INITIAL_SIZE: usize = 16; - /// Assert invariant - #[allow(clippy::assertions_on_constants)] - const _SIZE_CHECK: () = assert!(Self::INITIAL_SIZE > Self::INITIAL_DEPTH); + /// Assert invariants + const ASSERT_INVARIANTS: () = { + assert!(Self::INITIAL_DEPTH >= 1); + assert!(Self::INITIAL_SIZE >= 2); + assert!(Self::INITIAL_SIZE > Self::INITIAL_DEPTH); + }; pub(crate) fn new() -> Self { + // Invoke `ASSERT_INVARIANTS` assertions. Without this line, the assertions are ignored. + const { Self::ASSERT_INVARIANTS }; + let mut stack = vec![]; stack.resize_with(Self::INITIAL_SIZE, TempUnresolvedReferences::default); Self { stack, current_scope_depth: Self::INITIAL_DEPTH } From 0f85bc691849ab0a4305d527b11ff07b49341ee1 Mon Sep 17 00:00:00 2001 From: overlookmotel <557937+overlookmotel@users.noreply.github.com> Date: Wed, 22 Jan 2025 13:17:57 +0000 Subject: [PATCH 09/14] refactor(allocator): reduce repeat code to prevent `Drop` types in arena (#8655) Pure refactor. Reduce repeated code. --- crates/oxc_allocator/src/boxed.rs | 20 ++++++++++---------- crates/oxc_allocator/src/hash_map.rs | 25 ++++++++++++++++--------- crates/oxc_allocator/src/lib.rs | 6 +----- crates/oxc_allocator/src/vec.rs | 23 ++++++++++------------- 4 files changed, 37 insertions(+), 37 deletions(-) diff --git a/crates/oxc_allocator/src/boxed.rs b/crates/oxc_allocator/src/boxed.rs index e78d7191b7774..e14206d23eaaa 100644 --- a/crates/oxc_allocator/src/boxed.rs +++ b/crates/oxc_allocator/src/boxed.rs @@ -7,7 +7,6 @@ use std::{ fmt::{self, Debug, Formatter}, hash::{Hash, Hasher}, marker::PhantomData, - mem::needs_drop, ops::{self, Deref}, ptr::{self, NonNull}, }; @@ -31,6 +30,13 @@ use crate::Allocator; /// with a [`Drop`] type. pub struct Box<'alloc, T: ?Sized>(NonNull, PhantomData<(&'alloc (), T)>); +impl Box<'_, T> { + /// Const assertion that `T` is not `Drop`. + /// Must be referenced in all methods which create a `Box`. + const ASSERT_T_IS_NOT_DROP: () = + assert!(!std::mem::needs_drop::(), "Cannot create a Box where T is a Drop type"); +} + impl Box<'_, T> { /// Put a `value` into a memory arena and get back a [`Box`] with ownership /// to the allocation. @@ -48,9 +54,7 @@ impl Box<'_, T> { #[expect(clippy::inline_always)] #[inline(always)] pub fn new_in(value: T, allocator: &Allocator) -> Self { - const { - assert!(!needs_drop::(), "Cannot create a Box where T is a Drop type"); - } + const { Self::ASSERT_T_IS_NOT_DROP }; Self(NonNull::from(allocator.alloc(value)), PhantomData) } @@ -62,9 +66,7 @@ impl Box<'_, T> { /// Only purpose is for mocking types without allocating for const assertions. #[allow(unsafe_code, clippy::missing_safety_doc)] pub const unsafe fn dangling() -> Self { - const { - assert!(!needs_drop::(), "Cannot create a Box where T is a Drop type"); - } + const { Self::ASSERT_T_IS_NOT_DROP }; Self(NonNull::dangling(), PhantomData) } @@ -115,9 +117,7 @@ impl Box<'_, T> { #[expect(clippy::inline_always)] #[inline(always)] pub(crate) const unsafe fn from_non_null(ptr: NonNull) -> Self { - const { - assert!(!needs_drop::(), "Cannot create a Box where T is a Drop type"); - } + const { Self::ASSERT_T_IS_NOT_DROP }; Self(ptr, PhantomData) } diff --git a/crates/oxc_allocator/src/hash_map.rs b/crates/oxc_allocator/src/hash_map.rs index 837bedcec3013..0481009404467 100644 --- a/crates/oxc_allocator/src/hash_map.rs +++ b/crates/oxc_allocator/src/hash_map.rs @@ -9,7 +9,7 @@ use std::{ hash::Hash, - mem::{needs_drop, ManuallyDrop}, + mem::ManuallyDrop, ops::{Deref, DerefMut}, }; @@ -59,16 +59,26 @@ unsafe impl Sync for HashMap<'_, K, V> {} // Wrap them in `ManuallyDrop` to prevent that. impl<'alloc, K, V> HashMap<'alloc, K, V> { + /// Const assertions that `K` and `V` are not `Drop`. + /// Must be referenced in all methods which create a `HashMap`. + const ASSERT_K_AND_V_ARE_NOT_DROP: () = { + assert!( + !std::mem::needs_drop::(), + "Cannot create a HashMap where K is a Drop type" + ); + assert!( + !std::mem::needs_drop::(), + "Cannot create a HashMap where V is a Drop type" + ); + }; + /// Creates an empty [`HashMap`]. It will be allocated with the given allocator. /// /// The hash map is initially created with a capacity of 0, so it will not allocate /// until it is first inserted into. #[inline(always)] pub fn new_in(allocator: &'alloc Allocator) -> Self { - const { - assert!(!needs_drop::(), "Cannot create a HashMap where K is a Drop type"); - assert!(!needs_drop::(), "Cannot create a HashMap where V is a Drop type"); - } + const { Self::ASSERT_K_AND_V_ARE_NOT_DROP }; let inner = FxHashMap::with_hasher_in(FxBuildHasher, allocator.bump()); Self(ManuallyDrop::new(inner)) @@ -80,10 +90,7 @@ impl<'alloc, K, V> HashMap<'alloc, K, V> { /// If capacity is 0, the hash map will not allocate. #[inline(always)] pub fn with_capacity_in(capacity: usize, allocator: &'alloc Allocator) -> Self { - const { - assert!(!needs_drop::(), "Cannot create a HashMap where K is a Drop type"); - assert!(!needs_drop::(), "Cannot create a HashMap where V is a Drop type"); - } + const { Self::ASSERT_K_AND_V_ARE_NOT_DROP }; let inner = FxHashMap::with_capacity_and_hasher_in(capacity, FxBuildHasher, allocator.bump()); diff --git a/crates/oxc_allocator/src/lib.rs b/crates/oxc_allocator/src/lib.rs index add3cbd7f8cd9..aeb5c777cbb6f 100644 --- a/crates/oxc_allocator/src/lib.rs +++ b/crates/oxc_allocator/src/lib.rs @@ -14,8 +14,6 @@ #![warn(missing_docs)] -use std::mem::needs_drop; - use bumpalo::Bump; mod address; @@ -291,9 +289,7 @@ impl Allocator { #[expect(clippy::inline_always)] #[inline(always)] pub fn alloc(&self, val: T) -> &mut T { - const { - assert!(!needs_drop::(), "Cannot allocate Drop type in arena"); - } + const { assert!(!std::mem::needs_drop::(), "Cannot allocate Drop type in arena") }; self.bump.alloc(val) } diff --git a/crates/oxc_allocator/src/vec.rs b/crates/oxc_allocator/src/vec.rs index dc798bdf98e8d..5ff17ce1400f5 100644 --- a/crates/oxc_allocator/src/vec.rs +++ b/crates/oxc_allocator/src/vec.rs @@ -9,7 +9,7 @@ use std::{ self, fmt::{self, Debug}, hash::{Hash, Hasher}, - mem::{needs_drop, ManuallyDrop}, + mem::ManuallyDrop, ops, ptr::NonNull, slice::SliceIndex, @@ -43,6 +43,11 @@ unsafe impl Send for Vec<'_, T> {} unsafe impl Sync for Vec<'_, T> {} impl<'alloc, T> Vec<'alloc, T> { + /// Const assertion that `T` is not `Drop`. + /// Must be referenced in all methods which create a `Vec`. + const ASSERT_T_IS_NOT_DROP: () = + assert!(!std::mem::needs_drop::(), "Cannot create a Vec where T is a Drop type"); + /// Constructs a new, empty `Vec`. /// /// The vector will not allocate until elements are pushed onto it. @@ -58,9 +63,7 @@ impl<'alloc, T> Vec<'alloc, T> { /// ``` #[inline(always)] pub fn new_in(allocator: &'alloc Allocator) -> Self { - const { - assert!(!needs_drop::(), "Cannot create a Vec where T is a Drop type"); - } + const { Self::ASSERT_T_IS_NOT_DROP }; Self(ManuallyDrop::new(InnerVec::new_in(allocator.bump()))) } @@ -113,9 +116,7 @@ impl<'alloc, T> Vec<'alloc, T> { /// ``` #[inline(always)] pub fn with_capacity_in(capacity: usize, allocator: &'alloc Allocator) -> Self { - const { - assert!(!needs_drop::(), "Cannot create a Vec where T is a Drop type"); - } + const { Self::ASSERT_T_IS_NOT_DROP }; Self(ManuallyDrop::new(InnerVec::with_capacity_in(capacity, allocator.bump()))) } @@ -126,9 +127,7 @@ impl<'alloc, T> Vec<'alloc, T> { /// This is behaviorially identical to [`FromIterator::from_iter`]. #[inline] pub fn from_iter_in>(iter: I, allocator: &'alloc Allocator) -> Self { - const { - assert!(!needs_drop::(), "Cannot create a Vec where T is a Drop type"); - } + const { Self::ASSERT_T_IS_NOT_DROP }; let iter = iter.into_iter(); let hint = iter.size_hint(); @@ -155,9 +154,7 @@ impl<'alloc, T> Vec<'alloc, T> { /// ``` #[inline] pub fn from_array_in(array: [T; N], allocator: &'alloc Allocator) -> Self { - const { - assert!(!needs_drop::(), "Cannot create a Vec where T is a Drop type"); - } + const { Self::ASSERT_T_IS_NOT_DROP }; let boxed = Box::new_in(array, allocator); let ptr = Box::into_non_null(boxed).as_ptr().cast::(); From ae8db53d65dc840233aaf06a469cfa8209b3d843 Mon Sep 17 00:00:00 2001 From: overlookmotel <557937+overlookmotel@users.noreply.github.com> Date: Wed, 22 Jan 2025 13:45:32 +0000 Subject: [PATCH 10/14] refactor(allocator): move `Allocator` into own module (#8656) Pure refactor. `oxc_allocator` crate is getting quite crowded now. Move `Allocator` into a separate file. --- crates/oxc_allocator/src/allocator.rs | 460 ++++++++++++++++++++++++++ crates/oxc_allocator/src/lib.rs | 454 +------------------------ 2 files changed, 462 insertions(+), 452 deletions(-) create mode 100644 crates/oxc_allocator/src/allocator.rs diff --git a/crates/oxc_allocator/src/allocator.rs b/crates/oxc_allocator/src/allocator.rs new file mode 100644 index 0000000000000..0848e5529c621 --- /dev/null +++ b/crates/oxc_allocator/src/allocator.rs @@ -0,0 +1,460 @@ +use bumpalo::Bump; + +/// A bump-allocated memory arena. +/// +/// # Anatomy of an Allocator +/// +/// [`Allocator`] is flexibly sized. It grows as required as you allocate data into it. +/// +/// To do that, an [`Allocator`] consists of multiple memory chunks. +/// +/// [`Allocator::new`] creates a new allocator without any chunks. When you first allocate an object +/// into it, it will lazily create an initial chunk, the size of which is determined by the size of that +/// first allocation. +/// +/// As more data is allocated into the [`Allocator`], it will likely run out of capacity. At that point, +/// a new memory chunk is added, and further allocations will use this new chunk (until it too runs out +/// of capacity, and *another* chunk is added). +/// +/// The data from the 1st chunk is not copied into the 2nd one. It stays where it is, which means +/// `&` or `&mut` references to data in the first chunk remain valid. This is unlike e.g. `Vec` which +/// copies all existing data when it grows. +/// +/// Each chunk is at least double the size of the last one, so growth in capacity is exponential. +/// +/// [`Allocator::reset`] keeps only the last chunk (the biggest one), and discards any other chunks, +/// returning their memory to the global allocator. The last chunk has its cursor rewound back to +/// the start, so it's empty, ready to be re-used for allocating more data. +/// +/// # Recycling allocators +/// +/// For good performance, it's ideal to create an [`Allocator`], and re-use it over and over, rather than +/// repeatedly creating and dropping [`Allocator`]s. +/// +/// ``` +/// // This is good! +/// use oxc_allocator::Allocator; +/// let mut allocator = Allocator::new(); +/// +/// # fn do_stuff(_n: usize, _allocator: &Allocator) {} +/// for i in 0..100 { +/// do_stuff(i, &allocator); +/// // Reset the allocator, freeing the memory used by `do_stuff` +/// allocator.reset(); +/// } +/// ``` +/// +/// ``` +/// // DON'T DO THIS! +/// # use oxc_allocator::Allocator; +/// # fn do_stuff(_n: usize, _allocator: &Allocator) {} +/// for i in 0..100 { +/// let allocator = Allocator::new(); +/// do_stuff(i, &allocator); +/// } +/// ``` +/// +/// ``` +/// // DON'T DO THIS EITHER! +/// # use oxc_allocator::Allocator; +/// # let allocator = Allocator::new(); +/// # fn do_stuff(_n: usize, _allocator: &Allocator) {} +/// for i in 0..100 { +/// do_stuff(i, &allocator); +/// // We haven't reset the allocator, so we haven't freed the memory used by `do_stuff`. +/// // The allocator will grow and grow, consuming more and more memory. +/// } +/// ``` +/// +/// ## Why is re-using an [`Allocator`] good for performance? +/// +/// 3 reasons: +/// +/// #### 1. Avoid expensive system calls +/// +/// Creating an [`Allocator`] is a fairly expensive operation as it involves a call into global allocator, +/// which in turn will likely make a system call. Ditto when the [`Allocator`] is dropped. +/// Re-using an existing [`Allocator`] avoids these costs. +/// +/// #### 2. CPU cache +/// +/// Re-using an existing allocator means you're re-using the same block of memory. If that memory was +/// recently accessed, it's likely to be warm in the CPU cache, so memory accesses will be much faster +/// than accessing "cold" sections of main memory. +/// +/// This can have a very significant positive impact on performance. +/// +/// #### 3. Capacity stabilization +/// +/// The most efficient [`Allocator`] is one with only 1 chunk which has sufficient capacity for +/// everything you're going to allocate into it. +/// +/// Why? +/// +/// 1. Every allocation will occur without the allocator needing to grow. +/// +/// 2. This makes the "is there sufficient capacity to allocate this?" check in [`alloc`] completely +/// predictable (the answer is always "yes"). The CPU's branch predictor swiftly learns this, +/// speeding up operation. +/// +/// 3. When the [`Allocator`] is reset, there are no excess chunks to discard, so no system calls. +/// +/// Because [`reset`] keeps only the biggest chunk (see above), re-using the same [`Allocator`] +/// for multiple similar workloads will result in the [`Allocator`] swiftly stabilizing at a capacity +/// which is sufficient to service those workloads with a single chunk. +/// +/// If workload is completely uniform, it reaches stable state on the 3rd round. +/// +/// ``` +/// # use oxc_allocator::Allocator; +/// let mut allocator = Allocator::new(); +/// +/// fn workload(allocator: &Allocator) { +/// // Allocate 4 MB of data in small chunks +/// for i in 0..1_000_000u32 { +/// allocator.alloc(i); +/// } +/// } +/// +/// // 1st round +/// workload(&allocator); +/// +/// // `allocator` has capacity for 4 MB data, but split into many chunks. +/// // `reset` throws away all chunks except the last one which will be approx 2 MB. +/// allocator.reset(); +/// +/// // 2nd round +/// workload(&allocator); +/// +/// // `workload` filled the 2 MB chunk, so a 2nd chunk was created of double the size (4 MB). +/// // `reset` discards the smaller chunk, leaving only a single 4 MB chunk. +/// allocator.reset(); +/// +/// // 3rd round +/// // `allocator` now has sufficient capacity for all allocations in a single 4 MB chunk. +/// workload(&allocator); +/// +/// // `reset` has no chunks to discard. It keeps the single 4 MB chunk. No system calls. +/// allocator.reset(); +/// +/// // More rounds +/// // All serviced without needing to grow the allocator, and with no system calls. +/// for _ in 0..100 { +/// workload(&allocator); +/// allocator.reset(); +/// } +/// ``` +/// +/// # No `Drop`s +/// +/// Objects allocated into Oxc memory arenas are never [`Dropped`](Drop). +/// Memory is released in bulk when the allocator is dropped, without dropping the individual +/// objects in the arena. +/// +/// Therefore, it would produce a memory leak if you allocated [`Drop`] types into the arena +/// which own memory allocations outside the arena. +/// +/// Static checks make this impossible to do. [`Allocator::alloc`], [`Box::new_in`], [`Vec::new_in`], +/// [`HashMap::new_in`], and all other methods which store data in the arena will refuse to compile +/// if called with a [`Drop`] type. +/// +/// ```ignore +/// use oxc_allocator::{Allocator, Box}; +/// +/// let allocator = Allocator::new(); +/// +/// struct Foo { +/// pub a: i32 +/// } +/// +/// impl std::ops::Drop for Foo { +/// fn drop(&mut self) {} +/// } +/// +/// // This will fail to compile because `Foo` implements `Drop` +/// let foo = Box::new_in(Foo { a: 0 }, &allocator); +/// +/// struct Bar { +/// v: std::vec::Vec, +/// } +/// +/// // This will fail to compile because `Bar` contains a `std::vec::Vec`, and it implements `Drop` +/// let bar = Box::new_in(Bar { v: vec![1, 2, 3] }, &allocator); +/// ``` +/// +/// # Examples +/// +/// Consumers of the [`oxc` umbrella crate](https://crates.io/crates/oxc) pass +/// [`Allocator`] references to other tools. +/// +/// ```ignore +/// use oxc::{allocator::Allocator, parser::Parser, span::SourceType}; +/// +/// let allocator = Allocator::default(); +/// let parsed = Parser::new(&allocator, "let x = 1;", SourceType::default()); +/// assert!(parsed.errors.is_empty()); +/// ``` +/// +/// [`reset`]: Allocator::reset +/// [`alloc`]: Allocator::alloc +/// [`Box::new_in`]: crate::Box::new_in +/// [`Vec::new_in`]: crate::Vec::new_in +/// [`HashMap::new_in`]: crate::HashMap::new_in +#[derive(Default)] +pub struct Allocator { + bump: Bump, +} + +impl Allocator { + /// Create a new [`Allocator`] with no initial capacity. + /// + /// This method does not reserve any memory to back the allocator. Memory for allocator's initial + /// chunk will be reserved lazily, when you make the first allocation into this [`Allocator`] + /// (e.g. with [`Allocator::alloc`], [`Box::new_in`], [`Vec::new_in`], [`HashMap::new_in`]). + /// + /// If you can estimate the amount of memory the allocator will require to fit what you intend to + /// allocate into it, it is generally preferable to create that allocator with [`with_capacity`], + /// which reserves that amount of memory upfront. This will avoid further system calls to allocate + /// further chunks later on. This point is less important if you're re-using the allocator multiple + /// times. + /// + /// See [`Allocator`] docs for more information on efficient use of [`Allocator`]. + /// + /// [`with_capacity`]: Allocator::with_capacity + /// [`Box::new_in`]: crate::Box::new_in + /// [`Vec::new_in`]: crate::Vec::new_in + /// [`HashMap::new_in`]: crate::HashMap::new_in + // + // `#[inline(always)]` because just delegates to `bumpalo` method + #[expect(clippy::inline_always)] + #[inline(always)] + pub fn new() -> Self { + Self { bump: Bump::new() } + } + + /// Create a new [`Allocator`] with specified capacity. + /// + /// See [`Allocator`] docs for more information on efficient use of [`Allocator`]. + // + // `#[inline(always)]` because just delegates to `bumpalo` method + #[expect(clippy::inline_always)] + #[inline(always)] + pub fn with_capacity(capacity: usize) -> Self { + Self { bump: Bump::with_capacity(capacity) } + } + + /// Allocate an object in this [`Allocator`] and return an exclusive reference to it. + /// + /// # Panics + /// Panics if reserving space for `T` fails. + /// + /// # Examples + /// ``` + /// use oxc_allocator::Allocator; + /// + /// let allocator = Allocator::default(); + /// let x = allocator.alloc([1u8; 20]); + /// assert_eq!(x, &[1u8; 20]); + /// ``` + // + // `#[inline(always)]` because this is a very hot path and `Bump::alloc` is a very small function. + // We always want it to be inlined. + #[expect(clippy::inline_always)] + #[inline(always)] + pub fn alloc(&self, val: T) -> &mut T { + const { assert!(!std::mem::needs_drop::(), "Cannot allocate Drop type in arena") }; + + self.bump.alloc(val) + } + + /// Copy a string slice into this [`Allocator`] and return a reference to it. + /// + /// # Panics + /// Panics if reserving space for the string fails. + /// + /// # Examples + /// ``` + /// use oxc_allocator::Allocator; + /// let allocator = Allocator::default(); + /// let hello = allocator.alloc_str("hello world"); + /// assert_eq!(hello, "hello world"); + /// ``` + // + // `#[inline(always)]` because this is a hot path and `Bump::alloc_str` is a very small function. + // We always want it to be inlined. + #[expect(clippy::inline_always)] + #[inline(always)] + pub fn alloc_str<'alloc>(&'alloc self, src: &str) -> &'alloc mut str { + self.bump.alloc_str(src) + } + + /// Reset this allocator. + /// + /// Performs mass deallocation on everything allocated in this arena by resetting the pointer + /// into the underlying chunk of memory to the start of the chunk. + /// Does not run any `Drop` implementations on deallocated objects. + /// + /// If this arena has allocated multiple chunks to bump allocate into, then the excess chunks + /// are returned to the global allocator. + /// + /// # Examples + /// ``` + /// use oxc_allocator::Allocator; + /// + /// let mut allocator = Allocator::default(); + /// + /// // Allocate a bunch of things. + /// { + /// for i in 0..100 { + /// allocator.alloc(i); + /// } + /// } + /// + /// // Reset the arena. + /// allocator.reset(); + /// + /// // Allocate some new things in the space previously occupied by the + /// // original things. + /// for j in 200..400 { + /// allocator.alloc(j); + /// } + /// ``` + // + // `#[inline(always)]` because it just delegates to `bumpalo` + #[expect(clippy::inline_always)] + #[inline(always)] + pub fn reset(&mut self) { + self.bump.reset(); + } + + /// Calculate the total capacity of this [`Allocator`] including all chunks, in bytes. + /// + /// Note: This is the total amount of memory the [`Allocator`] owns NOT the total size of data + /// that's been allocated in it. If you want the latter, use [`used_bytes`] instead. + /// + /// # Examples + /// ``` + /// use oxc_allocator::Allocator; + /// + /// let capacity = 64 * 1024; // 64 KiB + /// let mut allocator = Allocator::with_capacity(capacity); + /// allocator.alloc(123u64); // 8 bytes + /// + /// // Result is the capacity (64 KiB), not the size of allocated data (8 bytes). + /// // `Allocator::with_capacity` may allocate a bit more than requested. + /// assert!(allocator.capacity() >= capacity); + /// ``` + /// + /// [`used_bytes`]: Allocator::used_bytes + // + // `#[inline(always)]` because it just delegates to `bumpalo` + #[expect(clippy::inline_always)] + #[inline(always)] + pub fn capacity(&self) -> usize { + self.bump.allocated_bytes() + } + + /// Calculate the total size of data used in this [`Allocator`], in bytes. + /// + /// This is the total amount of memory that has been *used* in the [`Allocator`], NOT the amount of + /// memory the [`Allocator`] owns. If you want the latter, use [`capacity`] instead. + /// + /// The result includes: + /// + /// 1. Padding bytes between objects which have been allocated to preserve alignment of types + /// where they have different alignments or have larger-than-typical alignment. + /// 2. Excess capacity in [`Vec`]s, [`String`]s and [`HashMap`]s. + /// 3. Objects which were allocated but later dropped. [`Allocator`] does not re-use allocations, + /// so anything which is allocated into arena continues to take up "dead space", even after it's + /// no longer referenced anywhere. + /// 4. "Dead space" left over where a [`Vec`], [`String`] or [`HashMap`] has grown and had to make + /// a new allocation to accommodate its new larger size. Its old allocation continues to take up + /// "dead" space in the allocator, unless it was the most recent allocation. + /// + /// In practice, this almost always means that the result returned from this function will be an + /// over-estimate vs the amount of "live" data in the arena. + /// + /// However, if you are using the result of this method to create a new `Allocator` to clone + /// an AST into, it is theoretically possible (though very unlikely) that it may be a slight + /// under-estimate of the capacity required in new allocator to clone the AST into, depending + /// on the order that `&str`s were allocated into arena in parser vs the order they get allocated + /// during cloning. The order allocations are made in affects the amount of padding bytes required. + /// + /// # Examples + /// ``` + /// use oxc_allocator::{Allocator, Vec}; + /// + /// let capacity = 64 * 1024; // 64 KiB + /// let mut allocator = Allocator::with_capacity(capacity); + /// + /// allocator.alloc(1u8); // 1 byte with alignment 1 + /// allocator.alloc(2u8); // 1 byte with alignment 1 + /// allocator.alloc(3u64); // 8 bytes with alignment 8 + /// + /// // Only 10 bytes were allocated, but 16 bytes were used, in order to align `3u64` on 8 + /// assert_eq!(allocator.used_bytes(), 16); + /// + /// allocator.reset(); + /// + /// let mut vec = Vec::::with_capacity_in(2, &allocator); + /// + /// // Allocate something else, so `vec`'s allocation is not the most recent + /// allocator.alloc(123u64); + /// + /// // `vec` has to grow beyond it's initial capacity + /// vec.extend([1, 2, 3, 4]); + /// + /// // `vec` takes up 32 bytes, and `123u64` takes up 8 bytes = 40 total. + /// // But there's an additional 16 bytes consumed for `vec`'s original capacity of 2, + /// // which is still using up space + /// assert_eq!(allocator.used_bytes(), 56); + /// ``` + /// + /// [`capacity`]: Allocator::capacity + /// [`Vec`]: crate::Vec + /// [`String`]: crate::String + /// [`HashMap`]: crate::HashMap + pub fn used_bytes(&self) -> usize { + let mut bytes = 0; + // SAFETY: No allocations are made while `chunks_iter` is alive. No data is read from the chunks. + let chunks_iter = unsafe { self.bump.iter_allocated_chunks_raw() }; + for (_, size) in chunks_iter { + bytes += size; + } + bytes + } + + /// Get inner [`bumpalo::Bump`]. + /// + /// This method is not public. We don't want to expose `Bump` to user. + /// The fact that we're using `bumpalo` is an internal implementation detail. + // + // `#[inline(always)]` because it's a no-op + #[expect(clippy::inline_always)] + #[inline(always)] + pub(crate) fn bump(&self) -> &Bump { + &self.bump + } +} + +/// SAFETY: Not actually safe, but for enabling `Send` for downstream crates. +unsafe impl Send for Allocator {} +/// SAFETY: Not actually safe, but for enabling `Sync` for downstream crates. +unsafe impl Sync for Allocator {} + +#[cfg(test)] +mod test { + use super::Allocator; + + #[test] + fn test_api() { + let mut allocator = Allocator::default(); + { + let array = allocator.alloc([123; 10]); + assert_eq!(array, &[123; 10]); + let str = allocator.alloc_str("hello"); + assert_eq!(str, "hello"); + } + allocator.reset(); + } +} diff --git a/crates/oxc_allocator/src/lib.rs b/crates/oxc_allocator/src/lib.rs index aeb5c777cbb6f..e06e352e3f720 100644 --- a/crates/oxc_allocator/src/lib.rs +++ b/crates/oxc_allocator/src/lib.rs @@ -14,9 +14,8 @@ #![warn(missing_docs)] -use bumpalo::Bump; - mod address; +mod allocator; mod allocator_api2; mod boxed; mod clone_in; @@ -26,459 +25,10 @@ pub mod string; mod vec; pub use address::{Address, GetAddress}; +pub use allocator::Allocator; pub use boxed::Box; pub use clone_in::CloneIn; pub use convert::{FromIn, IntoIn}; pub use hash_map::HashMap; pub use string::String; pub use vec::Vec; - -/// A bump-allocated memory arena. -/// -/// # Anatomy of an Allocator -/// -/// [`Allocator`] is flexibly sized. It grows as required as you allocate data into it. -/// -/// To do that, an [`Allocator`] consists of multiple memory chunks. -/// -/// [`Allocator::new`] creates a new allocator without any chunks. When you first allocate an object -/// into it, it will lazily create an initial chunk, the size of which is determined by the size of that -/// first allocation. -/// -/// As more data is allocated into the [`Allocator`], it will likely run out of capacity. At that point, -/// a new memory chunk is added, and further allocations will use this new chunk (until it too runs out -/// of capacity, and *another* chunk is added). -/// -/// The data from the 1st chunk is not copied into the 2nd one. It stays where it is, which means -/// `&` or `&mut` references to data in the first chunk remain valid. This is unlike e.g. `Vec` which -/// copies all existing data when it grows. -/// -/// Each chunk is at least double the size of the last one, so growth in capacity is exponential. -/// -/// [`Allocator::reset`] keeps only the last chunk (the biggest one), and discards any other chunks, -/// returning their memory to the global allocator. The last chunk has its cursor rewound back to -/// the start, so it's empty, ready to be re-used for allocating more data. -/// -/// # Recycling allocators -/// -/// For good performance, it's ideal to create an [`Allocator`], and re-use it over and over, rather than -/// repeatedly creating and dropping [`Allocator`]s. -/// -/// ``` -/// // This is good! -/// use oxc_allocator::Allocator; -/// let mut allocator = Allocator::new(); -/// -/// # fn do_stuff(_n: usize, _allocator: &Allocator) {} -/// for i in 0..100 { -/// do_stuff(i, &allocator); -/// // Reset the allocator, freeing the memory used by `do_stuff` -/// allocator.reset(); -/// } -/// ``` -/// -/// ``` -/// // DON'T DO THIS! -/// # use oxc_allocator::Allocator; -/// # fn do_stuff(_n: usize, _allocator: &Allocator) {} -/// for i in 0..100 { -/// let allocator = Allocator::new(); -/// do_stuff(i, &allocator); -/// } -/// ``` -/// -/// ``` -/// // DON'T DO THIS EITHER! -/// # use oxc_allocator::Allocator; -/// # let allocator = Allocator::new(); -/// # fn do_stuff(_n: usize, _allocator: &Allocator) {} -/// for i in 0..100 { -/// do_stuff(i, &allocator); -/// // We haven't reset the allocator, so we haven't freed the memory used by `do_stuff`. -/// // The allocator will grow and grow, consuming more and more memory. -/// } -/// ``` -/// -/// ## Why is re-using an [`Allocator`] good for performance? -/// -/// 3 reasons: -/// -/// #### 1. Avoid expensive system calls -/// -/// Creating an [`Allocator`] is a fairly expensive operation as it involves a call into global allocator, -/// which in turn will likely make a system call. Ditto when the [`Allocator`] is dropped. -/// Re-using an existing [`Allocator`] avoids these costs. -/// -/// #### 2. CPU cache -/// -/// Re-using an existing allocator means you're re-using the same block of memory. If that memory was -/// recently accessed, it's likely to be warm in the CPU cache, so memory accesses will be much faster -/// than accessing "cold" sections of main memory. -/// -/// This can have a very significant positive impact on performance. -/// -/// #### 3. Capacity stabilization -/// -/// The most efficient [`Allocator`] is one with only 1 chunk which has sufficient capacity for -/// everything you're going to allocate into it. -/// -/// Why? -/// -/// 1. Every allocation will occur without the allocator needing to grow. -/// -/// 2. This makes the "is there sufficient capacity to allocate this?" check in [`alloc`] completely -/// predictable (the answer is always "yes"). The CPU's branch predictor swiftly learns this, -/// speeding up operation. -/// -/// 3. When the [`Allocator`] is reset, there are no excess chunks to discard, so no system calls. -/// -/// Because [`reset`] keeps only the biggest chunk (see above), re-using the same [`Allocator`] -/// for multiple similar workloads will result in the [`Allocator`] swiftly stabilizing at a capacity -/// which is sufficient to service those workloads with a single chunk. -/// -/// If workload is completely uniform, it reaches stable state on the 3rd round. -/// -/// ``` -/// # use oxc_allocator::Allocator; -/// let mut allocator = Allocator::new(); -/// -/// fn workload(allocator: &Allocator) { -/// // Allocate 4 MB of data in small chunks -/// for i in 0..1_000_000u32 { -/// allocator.alloc(i); -/// } -/// } -/// -/// // 1st round -/// workload(&allocator); -/// -/// // `allocator` has capacity for 4 MB data, but split into many chunks. -/// // `reset` throws away all chunks except the last one which will be approx 2 MB. -/// allocator.reset(); -/// -/// // 2nd round -/// workload(&allocator); -/// -/// // `workload` filled the 2 MB chunk, so a 2nd chunk was created of double the size (4 MB). -/// // `reset` discards the smaller chunk, leaving only a single 4 MB chunk. -/// allocator.reset(); -/// -/// // 3rd round -/// // `allocator` now has sufficient capacity for all allocations in a single 4 MB chunk. -/// workload(&allocator); -/// -/// // `reset` has no chunks to discard. It keeps the single 4 MB chunk. No system calls. -/// allocator.reset(); -/// -/// // More rounds -/// // All serviced without needing to grow the allocator, and with no system calls. -/// for _ in 0..100 { -/// workload(&allocator); -/// allocator.reset(); -/// } -/// ``` -/// -/// [`reset`]: Allocator::reset -/// [`alloc`]: Allocator::alloc -/// -/// # No `Drop`s -/// -/// Objects allocated into Oxc memory arenas are never [`Dropped`](Drop). -/// Memory is released in bulk when the allocator is dropped, without dropping the individual -/// objects in the arena. -/// -/// Therefore, it would produce a memory leak if you allocated [`Drop`] types into the arena -/// which own memory allocations outside the arena. -/// -/// Static checks make this impossible to do. [`Allocator::alloc`], [`Box::new_in`], [`Vec::new_in`], -/// [`HashMap::new_in`], and all other methods which store data in the arena will refuse to compile -/// if called with a [`Drop`] type. -/// -/// ```ignore -/// use oxc_allocator::{Allocator, Box}; -/// -/// let allocator = Allocator::new(); -/// -/// struct Foo { -/// pub a: i32 -/// } -/// -/// impl std::ops::Drop for Foo { -/// fn drop(&mut self) {} -/// } -/// -/// // This will fail to compile because `Foo` implements `Drop` -/// let foo = Box::new_in(Foo { a: 0 }, &allocator); -/// -/// struct Bar { -/// v: std::vec::Vec, -/// } -/// -/// // This will fail to compile because `Bar` contains a `std::vec::Vec`, and it implements `Drop` -/// let bar = Box::new_in(Bar { v: vec![1, 2, 3] }, &allocator); -/// ``` -/// -/// # Examples -/// -/// Consumers of the [`oxc` umbrella crate](https://crates.io/crates/oxc) pass -/// [`Allocator`] references to other tools. -/// -/// ```ignore -/// use oxc::{allocator::Allocator, parser::Parser, span::SourceType}; -/// -/// let allocator = Allocator::default(); -/// let parsed = Parser::new(&allocator, "let x = 1;", SourceType::default()); -/// assert!(parsed.errors.is_empty()); -/// ``` -#[derive(Default)] -pub struct Allocator { - bump: Bump, -} - -impl Allocator { - /// Create a new [`Allocator`] with no initial capacity. - /// - /// This method does not reserve any memory to back the allocator. Memory for allocator's initial - /// chunk will be reserved lazily, when you make the first allocation into this [`Allocator`] - /// (e.g. with [`Allocator::alloc`], [`Box::new_in`], [`Vec::new_in`], [`HashMap::new_in`]). - /// - /// If you can estimate the amount of memory the allocator will require to fit what you intend to - /// allocate into it, it is generally preferable to create that allocator with [`with_capacity`], - /// which reserves that amount of memory upfront. This will avoid further system calls to allocate - /// further chunks later on. This point is less important if you're re-using the allocator multiple - /// times. - /// - /// See [`Allocator`] docs for more information on efficient use of [`Allocator`]. - /// - /// [`with_capacity`]: Allocator::with_capacity - // - // `#[inline(always)]` because just delegates to `bumpalo` method - #[expect(clippy::inline_always)] - #[inline(always)] - pub fn new() -> Self { - Self { bump: Bump::new() } - } - - /// Create a new [`Allocator`] with specified capacity. - /// - /// See [`Allocator`] docs for more information on efficient use of [`Allocator`]. - // - // `#[inline(always)]` because just delegates to `bumpalo` method - #[expect(clippy::inline_always)] - #[inline(always)] - pub fn with_capacity(capacity: usize) -> Self { - Self { bump: Bump::with_capacity(capacity) } - } - - /// Allocate an object in this [`Allocator`] and return an exclusive reference to it. - /// - /// # Panics - /// Panics if reserving space for `T` fails. - /// - /// # Examples - /// ``` - /// use oxc_allocator::Allocator; - /// - /// let allocator = Allocator::default(); - /// let x = allocator.alloc([1u8; 20]); - /// assert_eq!(x, &[1u8; 20]); - /// ``` - // - // `#[inline(always)]` because this is a very hot path and `Bump::alloc` is a very small function. - // We always want it to be inlined. - #[expect(clippy::inline_always)] - #[inline(always)] - pub fn alloc(&self, val: T) -> &mut T { - const { assert!(!std::mem::needs_drop::(), "Cannot allocate Drop type in arena") }; - - self.bump.alloc(val) - } - - /// Copy a string slice into this [`Allocator`] and return a reference to it. - /// - /// # Panics - /// Panics if reserving space for the string fails. - /// - /// # Examples - /// ``` - /// use oxc_allocator::Allocator; - /// let allocator = Allocator::default(); - /// let hello = allocator.alloc_str("hello world"); - /// assert_eq!(hello, "hello world"); - /// ``` - // - // `#[inline(always)]` because this is a hot path and `Bump::alloc_str` is a very small function. - // We always want it to be inlined. - #[expect(clippy::inline_always)] - #[inline(always)] - pub fn alloc_str<'alloc>(&'alloc self, src: &str) -> &'alloc mut str { - self.bump.alloc_str(src) - } - - /// Reset this allocator. - /// - /// Performs mass deallocation on everything allocated in this arena by resetting the pointer - /// into the underlying chunk of memory to the start of the chunk. - /// Does not run any `Drop` implementations on deallocated objects. - /// - /// If this arena has allocated multiple chunks to bump allocate into, then the excess chunks - /// are returned to the global allocator. - /// - /// # Examples - /// ``` - /// use oxc_allocator::Allocator; - /// - /// let mut allocator = Allocator::default(); - /// - /// // Allocate a bunch of things. - /// { - /// for i in 0..100 { - /// allocator.alloc(i); - /// } - /// } - /// - /// // Reset the arena. - /// allocator.reset(); - /// - /// // Allocate some new things in the space previously occupied by the - /// // original things. - /// for j in 200..400 { - /// allocator.alloc(j); - /// } - /// ``` - // - // `#[inline(always)]` because it just delegates to `bumpalo` - #[expect(clippy::inline_always)] - #[inline(always)] - pub fn reset(&mut self) { - self.bump.reset(); - } - - /// Calculate the total capacity of this [`Allocator`] including all chunks, in bytes. - /// - /// Note: This is the total amount of memory the [`Allocator`] owns NOT the total size of data - /// that's been allocated in it. If you want the latter, use [`used_bytes`] instead. - /// - /// # Examples - /// ``` - /// use oxc_allocator::Allocator; - /// - /// let capacity = 64 * 1024; // 64 KiB - /// let mut allocator = Allocator::with_capacity(capacity); - /// allocator.alloc(123u64); // 8 bytes - /// - /// // Result is the capacity (64 KiB), not the size of allocated data (8 bytes). - /// // `Allocator::with_capacity` may allocate a bit more than requested. - /// assert!(allocator.capacity() >= capacity); - /// ``` - /// - /// [`used_bytes`]: Allocator::used_bytes - // - // `#[inline(always)]` because it just delegates to `bumpalo` - #[expect(clippy::inline_always)] - #[inline(always)] - pub fn capacity(&self) -> usize { - self.bump.allocated_bytes() - } - - /// Calculate the total size of data used in this [`Allocator`], in bytes. - /// - /// This is the total amount of memory that has been *used* in the [`Allocator`], NOT the amount of - /// memory the [`Allocator`] owns. If you want the latter, use [`capacity`] instead. - /// - /// The result includes: - /// - /// 1. Padding bytes between objects which have been allocated to preserve alignment of types - /// where they have different alignments or have larger-than-typical alignment. - /// 2. Excess capacity in [`Vec`]s, [`String`]s and [`HashMap`]s. - /// 3. Objects which were allocated but later dropped. [`Allocator`] does not re-use allocations, - /// so anything which is allocated into arena continues to take up "dead space", even after it's - /// no longer referenced anywhere. - /// 4. "Dead space" left over where a [`Vec`], [`String`] or [`HashMap`] has grown and had to make - /// a new allocation to accommodate its new larger size. Its old allocation continues to take up - /// "dead" space in the allocator, unless it was the most recent allocation. - /// - /// In practice, this almost always means that the result returned from this function will be an - /// over-estimate vs the amount of "live" data in the arena. - /// - /// However, if you are using the result of this method to create a new `Allocator` to clone - /// an AST into, it is theoretically possible (though very unlikely) that it may be a slight - /// under-estimate of the capacity required in new allocator to clone the AST into, depending - /// on the order that `&str`s were allocated into arena in parser vs the order they get allocated - /// during cloning. The order allocations are made in affects the amount of padding bytes required. - /// - /// # Examples - /// ``` - /// use oxc_allocator::{Allocator, Vec}; - /// - /// let capacity = 64 * 1024; // 64 KiB - /// let mut allocator = Allocator::with_capacity(capacity); - /// - /// allocator.alloc(1u8); // 1 byte with alignment 1 - /// allocator.alloc(2u8); // 1 byte with alignment 1 - /// allocator.alloc(3u64); // 8 bytes with alignment 8 - /// - /// // Only 10 bytes were allocated, but 16 bytes were used, in order to align `3u64` on 8 - /// assert_eq!(allocator.used_bytes(), 16); - /// - /// allocator.reset(); - /// - /// let mut vec = Vec::::with_capacity_in(2, &allocator); - /// - /// // Allocate something else, so `vec`'s allocation is not the most recent - /// allocator.alloc(123u64); - /// - /// // `vec` has to grow beyond it's initial capacity - /// vec.extend([1, 2, 3, 4]); - /// - /// // `vec` takes up 32 bytes, and `123u64` takes up 8 bytes = 40 total. - /// // But there's an additional 16 bytes consumed for `vec`'s original capacity of 2, - /// // which is still using up space - /// assert_eq!(allocator.used_bytes(), 56); - /// ``` - /// - /// [`capacity`]: Allocator::capacity - pub fn used_bytes(&self) -> usize { - let mut bytes = 0; - // SAFETY: No allocations are made while `chunks_iter` is alive. No data is read from the chunks. - let chunks_iter = unsafe { self.bump.iter_allocated_chunks_raw() }; - for (_, size) in chunks_iter { - bytes += size; - } - bytes - } - - /// Get inner [`bumpalo::Bump`]. - /// - /// This method is not public. We don't want to expose `bumpalo::Allocator` to user. - /// The fact that we're using `bumpalo` is an internal implementation detail. - // - // `#[inline(always)]` because it's a no-op - #[expect(clippy::inline_always)] - #[inline(always)] - pub(crate) fn bump(&self) -> &Bump { - &self.bump - } -} - -/// SAFETY: Not actually safe, but for enabling `Send` for downstream crates. -unsafe impl Send for Allocator {} -/// SAFETY: Not actually safe, but for enabling `Sync` for downstream crates. -unsafe impl Sync for Allocator {} - -#[cfg(test)] -mod test { - use crate::Allocator; - - #[test] - fn test_api() { - let mut allocator = Allocator::default(); - { - let array = allocator.alloc([123; 10]); - assert_eq!(array, &[123; 10]); - let str = allocator.alloc_str("hello"); - assert_eq!(str, "hello"); - } - allocator.reset(); - } -} From 3802d28233454fef70fc44edd9fc80aa81f982c9 Mon Sep 17 00:00:00 2001 From: Boshen <1430279+Boshen@users.noreply.github.com> Date: Wed, 22 Jan 2025 15:17:47 +0000 Subject: [PATCH 11/14] refactor(minifier): clean up `try_minimize_conditional` (#8660) --- .../src/peephole/minimize_conditions.rs | 211 +++++++++--------- .../src/peephole/replace_known_methods.rs | 8 +- tasks/minsize/minsize.snap | 14 +- 3 files changed, 113 insertions(+), 120 deletions(-) diff --git a/crates/oxc_minifier/src/peephole/minimize_conditions.rs b/crates/oxc_minifier/src/peephole/minimize_conditions.rs index b456e68086012..4a5e199f74902 100644 --- a/crates/oxc_minifier/src/peephole/minimize_conditions.rs +++ b/crates/oxc_minifier/src/peephole/minimize_conditions.rs @@ -353,130 +353,108 @@ impl<'a> PeepholeOptimizations { } } - // https://github.com/evanw/esbuild/blob/v0.24.2/internal/js_ast/js_ast_helpers.go#L2745 + // fn try_minimize_conditional( expr: &mut ConditionalExpression<'a>, ctx: &mut TraverseCtx<'a>, ) -> Option> { - // `(a, b) ? c : d` -> `a, b ? c : d` - if let Expression::SequenceExpression(sequence_expr) = &mut expr.test { - if sequence_expr.expressions.len() > 1 { - let span = expr.span(); - let mut sequence = ctx.ast.move_expression(&mut expr.test); - let Expression::SequenceExpression(sequence_expr) = &mut sequence else { - unreachable!() - }; - let test = sequence_expr.expressions.pop().unwrap(); - sequence_expr.expressions.push(ctx.ast.expression_conditional( - span, - test, - ctx.ast.move_expression(&mut expr.consequent), - ctx.ast.move_expression(&mut expr.alternate), - )); - return Some(sequence); + match &mut expr.test { + // `x != y ? b : c` -> `x == y ? c : b` + Expression::BinaryExpression(test_expr) => { + if matches!( + test_expr.operator, + BinaryOperator::Inequality | BinaryOperator::StrictInequality + ) { + test_expr.operator = test_expr.operator.equality_inverse_operator().unwrap(); + let test = ctx.ast.move_expression(&mut expr.test); + let consequent = ctx.ast.move_expression(&mut expr.consequent); + let alternate = ctx.ast.move_expression(&mut expr.alternate); + return Some( + ctx.ast.expression_conditional(expr.span, test, alternate, consequent), + ); + } } - } - - // `!a ? b : c` -> `a ? c : b` - if let Expression::UnaryExpression(test_expr) = &mut expr.test { - if test_expr.operator.is_not() - // Skip `!!!a` - && !matches!(test_expr.argument, Expression::UnaryExpression(_)) - { - let test = ctx.ast.move_expression(&mut test_expr.argument); - let consequent = ctx.ast.move_expression(&mut expr.consequent); - let alternate = ctx.ast.move_expression(&mut expr.alternate); - return Some( - ctx.ast.expression_conditional(expr.span, test, alternate, consequent), - ); + // "(a, b) ? c : d" => "a, b ? c : d" + Expression::SequenceExpression(sequence_expr) => { + if sequence_expr.expressions.len() > 1 { + let span = expr.span(); + let mut sequence = ctx.ast.move_expression(&mut expr.test); + let Expression::SequenceExpression(sequence_expr) = &mut sequence else { + unreachable!() + }; + let mut cond_expr = ctx.ast.conditional_expression( + span, + sequence_expr.expressions.pop().unwrap(), + ctx.ast.move_expression(&mut expr.consequent), + ctx.ast.move_expression(&mut expr.alternate), + ); + let expr = + Self::try_minimize_conditional(&mut cond_expr, ctx).unwrap_or_else(|| { + Expression::ConditionalExpression(ctx.ast.alloc(cond_expr)) + }); + sequence_expr.expressions.push(expr); + return Some(sequence); + } } - } - - // `x != y ? b : c` -> `x == y ? c : b` - if let Expression::BinaryExpression(test_expr) = &mut expr.test { - if matches!( - test_expr.operator, - BinaryOperator::Inequality | BinaryOperator::StrictInequality - ) { - test_expr.operator = test_expr.operator.equality_inverse_operator().unwrap(); - let test = ctx.ast.move_expression(&mut expr.test); - let consequent = ctx.ast.move_expression(&mut expr.consequent); - let alternate = ctx.ast.move_expression(&mut expr.alternate); - return Some( - ctx.ast.expression_conditional(expr.span, test, alternate, consequent), - ); + // "!a ? b : c" => "a ? c : b" + Expression::UnaryExpression(test_expr) => { + if test_expr.operator.is_not() { + let test = ctx.ast.move_expression(&mut test_expr.argument); + let consequent = ctx.ast.move_expression(&mut expr.alternate); + let alternate = ctx.ast.move_expression(&mut expr.consequent); + return Some( + ctx.ast.expression_conditional(expr.span, test, consequent, alternate), + ); + } } + // "a ? a : b" => "a || b" + Expression::Identifier(id) => { + if let Expression::Identifier(id2) = &expr.consequent { + if id.name == id2.name { + return Some(ctx.ast.expression_logical( + expr.span, + ctx.ast.move_expression(&mut expr.test), + LogicalOperator::Or, + ctx.ast.move_expression(&mut expr.alternate), + )); + } + } + // "a ? b : a" => "a && b" + if let Expression::Identifier(id2) = &expr.alternate { + if id.name == id2.name { + return Some(ctx.ast.expression_logical( + expr.span, + ctx.ast.move_expression(&mut expr.test), + LogicalOperator::And, + ctx.ast.move_expression(&mut expr.consequent), + )); + } + } + } + _ => {} } - // TODO: `/* @__PURE__ */ a() ? b : b` -> `b` - - // `a ? b : b` -> `a, b` - if expr.alternate.content_eq(&expr.consequent) { - let expressions = ctx.ast.vec_from_array([ - ctx.ast.move_expression(&mut expr.test), - ctx.ast.move_expression(&mut expr.consequent), - ]); - return Some(ctx.ast.expression_sequence(expr.span, expressions)); - } - - // `a ? true : false` -> `!!a` - // `a ? false : true` -> `!a` - if let ( - Expression::Identifier(_), - Expression::BooleanLiteral(consequent_lit), - Expression::BooleanLiteral(alternate_lit), - ) = (&expr.test, &expr.consequent, &expr.alternate) + // "a ? true : false" => "!!a" + // "a ? false : true" => "!a" + if let (Expression::BooleanLiteral(left), Expression::BooleanLiteral(right)) = + (&expr.consequent, &expr.alternate) { - match (consequent_lit.value, alternate_lit.value) { + let op = UnaryOperator::LogicalNot; + match (left.value, right.value) { (false, true) => { let ident = ctx.ast.move_expression(&mut expr.test); - return Some(ctx.ast.expression_unary( - expr.span, - UnaryOperator::LogicalNot, - ident, - )); + return Some(ctx.ast.expression_unary(expr.span, op, ident)); } (true, false) => { let ident = ctx.ast.move_expression(&mut expr.test); - return Some(ctx.ast.expression_unary( - expr.span, - UnaryOperator::LogicalNot, - ctx.ast.expression_unary(expr.span, UnaryOperator::LogicalNot, ident), - )); + let unary = ctx.ast.expression_unary(expr.span, op, ident); + return Some(ctx.ast.expression_unary(expr.span, op, unary)); } _ => {} } } - // `a ? a : b` -> `a || b` - if let (Expression::Identifier(test_ident), Expression::Identifier(consequent_ident)) = - (&expr.test, &expr.consequent) - { - if test_ident.name == consequent_ident.name { - return Some(ctx.ast.expression_logical( - expr.span, - ctx.ast.move_expression(&mut expr.test), - LogicalOperator::Or, - ctx.ast.move_expression(&mut expr.alternate), - )); - } - } - - // `a ? b : a` -> `a && b` - if let (Expression::Identifier(test_ident), Expression::Identifier(alternate_ident)) = - (&expr.test, &expr.alternate) - { - if test_ident.name == alternate_ident.name { - return Some(ctx.ast.expression_logical( - expr.span, - ctx.ast.move_expression(&mut expr.test), - LogicalOperator::And, - ctx.ast.move_expression(&mut expr.consequent), - )); - } - } - - // `a ? b ? c : d : d` -> `a && b ? c : d` + // "a ? b ? c : d : d" => "a && b ? c : d" if let Expression::ConditionalExpression(consequent) = &mut expr.consequent { if consequent.alternate.content_eq(&expr.alternate) { return Some(ctx.ast.expression_conditional( @@ -493,7 +471,7 @@ impl<'a> PeepholeOptimizations { } } - // `a ? b : c ? b : d` -> `a || c ? b : d` + // "a ? b : c ? b : d" => "a || c ? b : d" if let Expression::ConditionalExpression(alternate) = &mut expr.alternate { if alternate.consequent.content_eq(&expr.consequent) { return Some(ctx.ast.expression_conditional( @@ -510,7 +488,7 @@ impl<'a> PeepholeOptimizations { } } - // `a ? c : (b, c)` -> `(a || b), c` + // "a ? c : (b, c)" => "(a || b), c" if let Expression::SequenceExpression(alternate) = &mut expr.alternate { if alternate.expressions.len() == 2 && alternate.expressions[1].content_eq(&expr.consequent) @@ -530,7 +508,7 @@ impl<'a> PeepholeOptimizations { } } - // `a ? (b, c) : c` -> `(a && b), c` + // "a ? (b, c) : c" => "(a && b), c" if let Expression::SequenceExpression(consequent) = &mut expr.consequent { if consequent.expressions.len() == 2 && consequent.expressions[1].content_eq(&expr.alternate) @@ -550,7 +528,7 @@ impl<'a> PeepholeOptimizations { } } - // `a ? b || c : c` => "(a && b) || c" + // "a ? b || c : c" => "(a && b) || c" if let Expression::LogicalExpression(logical_expr) = &mut expr.consequent { if logical_expr.operator == LogicalOperator::Or && logical_expr.right.content_eq(&expr.alternate) @@ -569,7 +547,7 @@ impl<'a> PeepholeOptimizations { } } - // `a ? c : b && c` -> `(a || b) && c` + // "a ? c : b && c" => "(a || b) && c" if let Expression::LogicalExpression(logical_expr) = &mut expr.alternate { if logical_expr.operator == LogicalOperator::And && logical_expr.right.content_eq(&expr.consequent) @@ -715,6 +693,21 @@ impl<'a> PeepholeOptimizations { )); } + if expr.alternate.content_eq(&expr.consequent) { + // TODO: + // "/* @__PURE__ */ a() ? b : b" => "b" + // if ctx.ExprCanBeRemovedIfUnused(test) { + // return yes + // } + + // "a ? b : b" => "a, b" + let expressions = ctx.ast.vec_from_array([ + ctx.ast.move_expression(&mut expr.test), + ctx.ast.move_expression(&mut expr.consequent), + ]); + return Some(ctx.ast.expression_sequence(expr.span, expressions)); + } + None } diff --git a/crates/oxc_minifier/src/peephole/replace_known_methods.rs b/crates/oxc_minifier/src/peephole/replace_known_methods.rs index 95afd49c5e602..686bdd16e6f57 100644 --- a/crates/oxc_minifier/src/peephole/replace_known_methods.rs +++ b/crates/oxc_minifier/src/peephole/replace_known_methods.rs @@ -333,16 +333,16 @@ impl<'a> PeepholeOptimizations { /// `[].concat(a).concat(b)` -> `[].concat(a, b)` /// `"".concat(a).concat(b)` -> `"".concat(a, b)` fn try_fold_concat_chain(&mut self, node: &mut Expression<'a>, ctx: &mut TraverseCtx<'a>) { - if matches!(ctx.parent(), Ancestor::StaticMemberExpressionObject(_)) { - return; - } - let original_span = if let Expression::CallExpression(root_call_expr) = node { root_call_expr.span } else { return; }; + if matches!(ctx.parent(), Ancestor::StaticMemberExpressionObject(_)) { + return; + } + let mut current_node: &mut Expression = node; let mut collected_arguments = ctx.ast.vec(); let new_root_callee: &mut Expression<'a>; diff --git a/tasks/minsize/minsize.snap b/tasks/minsize/minsize.snap index d4a288450060d..6c5b9ebbc3ff6 100644 --- a/tasks/minsize/minsize.snap +++ b/tasks/minsize/minsize.snap @@ -9,19 +9,19 @@ Original | minified | minified | gzip | gzip | Fixture 342.15 kB | 118.19 kB | 118.14 kB | 44.45 kB | 44.37 kB | vue.js -544.10 kB | 71.74 kB | 72.48 kB | 26.14 kB | 26.20 kB | lodash.js +544.10 kB | 71.73 kB | 72.48 kB | 26.15 kB | 26.20 kB | lodash.js 555.77 kB | 272.89 kB | 270.13 kB | 90.90 kB | 90.80 kB | d3.js -1.01 MB | 460.18 kB | 458.89 kB | 126.78 kB | 126.71 kB | bundle.min.js +1.01 MB | 460.17 kB | 458.89 kB | 126.78 kB | 126.71 kB | bundle.min.js -1.25 MB | 652.90 kB | 646.76 kB | 163.54 kB | 163.73 kB | three.js +1.25 MB | 652.85 kB | 646.76 kB | 163.53 kB | 163.73 kB | three.js -2.14 MB | 724.01 kB | 724.14 kB | 179.93 kB | 181.07 kB | victory.js +2.14 MB | 724.00 kB | 724.14 kB | 179.93 kB | 181.07 kB | victory.js -3.20 MB | 1.01 MB | 1.01 MB | 332.01 kB | 331.56 kB | echarts.js +3.20 MB | 1.01 MB | 1.01 MB | 332.00 kB | 331.56 kB | echarts.js -6.69 MB | 2.31 MB | 2.31 MB | 491.97 kB | 488.28 kB | antd.js +6.69 MB | 2.31 MB | 2.31 MB | 491.95 kB | 488.28 kB | antd.js -10.95 MB | 3.48 MB | 3.49 MB | 905.34 kB | 915.50 kB | typescript.js +10.95 MB | 3.48 MB | 3.49 MB | 905.35 kB | 915.50 kB | typescript.js From a730f999a8d66ad72865d5f016e51ac28246a692 Mon Sep 17 00:00:00 2001 From: Dunqing <29533304+Dunqing@users.noreply.github.com> Date: Wed, 22 Jan 2025 15:51:27 +0000 Subject: [PATCH 12/14] refactor(transformer): move `create_prototype_member` to utils module (#8657) This function can also used in #8614, so move it. --- .../src/es2022/class_properties/super_converter.rs | 7 ++----- .../src/es2022/class_properties/utils.rs | 10 ---------- crates/oxc_transformer/src/utils/ast_builder.rs | 10 ++++++++++ 3 files changed, 12 insertions(+), 15 deletions(-) diff --git a/crates/oxc_transformer/src/es2022/class_properties/super_converter.rs b/crates/oxc_transformer/src/es2022/class_properties/super_converter.rs index 85b940b7c13c2..52a6a73f372ad 100644 --- a/crates/oxc_transformer/src/es2022/class_properties/super_converter.rs +++ b/crates/oxc_transformer/src/es2022/class_properties/super_converter.rs @@ -6,12 +6,9 @@ use oxc_ast::ast::*; use oxc_span::SPAN; use oxc_traverse::{ast_operations::get_var_name_from_node, TraverseCtx}; -use crate::Helper; +use crate::{utils::ast_builder::create_prototype_member, Helper}; -use super::{ - utils::{create_assignment, create_prototype_member}, - ClassProperties, -}; +use super::{utils::create_assignment, ClassProperties}; #[derive(Debug)] pub(super) enum ClassPropertiesSuperConverterMode { diff --git a/crates/oxc_transformer/src/es2022/class_properties/utils.rs b/crates/oxc_transformer/src/es2022/class_properties/utils.rs index 55f2c0188c302..be9c50d54220b 100644 --- a/crates/oxc_transformer/src/es2022/class_properties/utils.rs +++ b/crates/oxc_transformer/src/es2022/class_properties/utils.rs @@ -71,13 +71,3 @@ pub(super) fn debug_assert_expr_is_not_parenthesis_or_typescript_syntax( "Should not be: {expr:?} in {path:?}", ); } - -/// `object` -> `object.prototype`. -pub(super) fn create_prototype_member<'a>( - object: Expression<'a>, - ctx: &mut TraverseCtx<'a>, -) -> Expression<'a> { - let property = ctx.ast.identifier_name(SPAN, Atom::from("prototype")); - let static_member = ctx.ast.member_expression_static(SPAN, object, property, false); - Expression::from(static_member) -} diff --git a/crates/oxc_transformer/src/utils/ast_builder.rs b/crates/oxc_transformer/src/utils/ast_builder.rs index f4ac299df9ddc..91c4fdd4efff2 100644 --- a/crates/oxc_transformer/src/utils/ast_builder.rs +++ b/crates/oxc_transformer/src/utils/ast_builder.rs @@ -71,3 +71,13 @@ pub(crate) fn wrap_statements_in_arrow_function_iife<'a>( )); ctx.ast.expression_call(span, arrow, NONE, ctx.ast.vec(), false) } + +/// `object` -> `object.prototype`. +pub(crate) fn create_prototype_member<'a>( + object: Expression<'a>, + ctx: &mut TraverseCtx<'a>, +) -> Expression<'a> { + let property = ctx.ast.identifier_name(SPAN, Atom::from("prototype")); + let static_member = ctx.ast.member_expression_static(SPAN, object, property, false); + Expression::from(static_member) +} From ae895d8c0e3fe31fe809ccc27bc4d4831b9997d5 Mon Sep 17 00:00:00 2001 From: Boshen <1430279+Boshen@users.noreply.github.com> Date: Wed, 22 Jan 2025 16:04:35 +0000 Subject: [PATCH 13/14] refactor(minifier): use `NonEmptyStack` for function stack (#8661) --- Cargo.lock | 1 + crates/oxc_minifier/Cargo.toml | 1 + crates/oxc_minifier/src/peephole/mod.rs | 40 ++++++++++--------------- 3 files changed, 17 insertions(+), 25 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 4b40a3703bcfc..eac4924798e85 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -1850,6 +1850,7 @@ dependencies = [ "oxc_allocator", "oxc_ast", "oxc_codegen", + "oxc_data_structures", "oxc_ecmascript", "oxc_mangler", "oxc_parser", diff --git a/crates/oxc_minifier/Cargo.toml b/crates/oxc_minifier/Cargo.toml index 5212e6e03a5d8..68d12eab1198b 100644 --- a/crates/oxc_minifier/Cargo.toml +++ b/crates/oxc_minifier/Cargo.toml @@ -24,6 +24,7 @@ doctest = false oxc_allocator = { workspace = true } oxc_ast = { workspace = true } oxc_codegen = { workspace = true } +oxc_data_structures = { workspace = true } oxc_ecmascript = { workspace = true, features = ["constant_evaluation"] } oxc_mangler = { workspace = true } oxc_parser = { workspace = true } diff --git a/crates/oxc_minifier/src/peephole/mod.rs b/crates/oxc_minifier/src/peephole/mod.rs index 72223943c40ca..7d80a35984ed7 100644 --- a/crates/oxc_minifier/src/peephole/mod.rs +++ b/crates/oxc_minifier/src/peephole/mod.rs @@ -11,6 +11,7 @@ mod substitute_alternate_syntax; use oxc_allocator::Vec; use oxc_ast::ast::*; +use oxc_data_structures::stack::NonEmptyStack; use oxc_syntax::{es_target::ESTarget, scope::ScopeId}; use oxc_traverse::{traverse_mut_with_ctx, ReusableTraverseCtx, Traverse, TraverseCtx}; @@ -28,8 +29,8 @@ pub struct PeepholeOptimizations { prev_functions_changed: FxHashSet, functions_changed: FxHashSet, /// Track the current function as a stack. - current_function_stack: - std::vec::Vec<(ScopeId, /* prev changed */ bool, /* current changed */ bool)>, + current_function: + NonEmptyStack<(ScopeId, /* prev changed */ bool, /* current changed */ bool)>, } impl<'a> PeepholeOptimizations { @@ -39,7 +40,7 @@ impl<'a> PeepholeOptimizations { iteration: 0, prev_functions_changed: FxHashSet::default(), functions_changed: FxHashSet::default(), - current_function_stack: std::vec::Vec::new(), + current_function: NonEmptyStack::new((ScopeId::new(0), true, false)), } } @@ -64,43 +65,32 @@ impl<'a> PeepholeOptimizations { } fn mark_current_function_as_changed(&mut self) { - if let Some((_scope_id, _prev_changed, current_changed)) = - self.current_function_stack.last_mut() - { - *current_changed = true; - } + let (_scope_id, _prev_changed, current_changed) = self.current_function.last_mut(); + *current_changed = true; } pub fn is_current_function_changed(&self) -> bool { - if let Some((_, _, current_changed)) = self.current_function_stack.last() { - return *current_changed; - } - false + let (_, _, current_changed) = self.current_function.last(); + *current_changed } fn is_prev_function_changed(&self) -> bool { - if self.iteration == 0 { - return true; - } - if let Some((_, prev_changed, _)) = self.current_function_stack.last() { - return *prev_changed; - } - false + let (_, prev_changed, _) = self.current_function.last(); + *prev_changed } fn enter_program_or_function(&mut self, scope_id: ScopeId) { - self.current_function_stack.push(( + self.current_function.push(( scope_id, - self.prev_functions_changed.contains(&scope_id), + self.iteration == 0 || self.prev_functions_changed.contains(&scope_id), false, )); } fn exit_program_or_function(&mut self) { - if let Some((scope_id, _, changed)) = self.current_function_stack.pop() { - if changed { - self.functions_changed.insert(scope_id); - } + let (scope_id, _, changed) = self.current_function.pop(); + if changed { + self.functions_changed.insert(scope_id); } } } From 3be03926e8a5097e5c1fe249b8bff0009ec4468d Mon Sep 17 00:00:00 2001 From: overlookmotel <557937+overlookmotel@users.noreply.github.com> Date: Wed, 22 Jan 2025 16:22:32 +0000 Subject: [PATCH 14/14] docs(lexer): fix doc comment (#8664) The comment showing expansion of `ascii_byte_handler!` macro was inaccurate. --- crates/oxc_parser/src/lexer/byte_handlers.rs | 1 - 1 file changed, 1 deletion(-) diff --git a/crates/oxc_parser/src/lexer/byte_handlers.rs b/crates/oxc_parser/src/lexer/byte_handlers.rs index ab09604843af9..1be5ae6fe01a9 100644 --- a/crates/oxc_parser/src/lexer/byte_handlers.rs +++ b/crates/oxc_parser/src/lexer/byte_handlers.rs @@ -108,7 +108,6 @@ macro_rules! byte_handler { /// // SAFETY: This macro is only used for ASCII characters /// unsafe { /// use assert_unchecked::assert_unchecked; -/// let s = lexer.current.chars.as_str(); /// assert_unchecked!(!lexer.source.is_eof()); /// assert_unchecked!(lexer.source.peek_byte_unchecked() < 128); /// }