From aebe0ea46dc709e8c2c96b74936e3337132b71e2 Mon Sep 17 00:00:00 2001
From: overlookmotel <557937+overlookmotel@users.noreply.github.com>
Date: Wed, 8 Jan 2025 01:32:56 +0000
Subject: [PATCH] perf(transformer/arrow-functions): use `NonEmptyStack`
 instead of `Stack` (#8318)

Follow-on after #8024.

Use `NonEmptyStack` instead of `Stack`.

`NonEmptyStack` is cheaper as `last` / `last_mut` always returns `T`, rather than `Option<T>`, so there's no need to branch on whether it's `Some` or not.

The only downside of `NonEmptyStack` is that it always contains 1 dummy entry at the start (so that it's never empty). This means that it always allocates, unlike `Stack` which doesn't allocate unless you push something into it.

So `NonEmptyStack` is always the better choice unless either:

1. The stack will likely never have anything pushed to it. or
2. The type the stack holds is large or expensive to construct, so there's a high cost in having to create an initial dummy value.

In this case:

* The type is `bool` - very small and cheap to construct.
* The stack will be pushed to when entering a function. Very few JS files contain no functions at all, so the stack will always need to allocate, even if we used `Stack`.

So this case doesn't satisfy either of the circumstances in which `Stack` is the better choice.
---
 .../src/common/arrow_function_converter.rs     | 18 ++++++++++--------
 1 file changed, 10 insertions(+), 8 deletions(-)

diff --git a/crates/oxc_transformer/src/common/arrow_function_converter.rs b/crates/oxc_transformer/src/common/arrow_function_converter.rs
index 30a32a712378e..4a869757b12b9 100644
--- a/crates/oxc_transformer/src/common/arrow_function_converter.rs
+++ b/crates/oxc_transformer/src/common/arrow_function_converter.rs
@@ -93,7 +93,7 @@ use rustc_hash::{FxBuildHasher, FxHashSet};
 
 use oxc_allocator::{Box as ArenaBox, Vec as ArenaVec};
 use oxc_ast::{ast::*, visit::walk_mut::walk_expression, VisitMut, NONE};
-use oxc_data_structures::stack::{NonEmptyStack, SparseStack, Stack};
+use oxc_data_structures::stack::{NonEmptyStack, SparseStack};
 use oxc_semantic::{ReferenceFlags, SymbolId};
 use oxc_span::{CompactStr, GetSpan, SPAN};
 use oxc_syntax::{
@@ -139,7 +139,7 @@ pub struct ArrowFunctionConverter<'a> {
     mode: ArrowFunctionConverterMode,
     this_var_stack: SparseStack<BoundIdentifier<'a>>,
     arguments_var_stack: SparseStack<BoundIdentifier<'a>>,
-    constructor_super_stack: Stack<bool>,
+    constructor_super_stack: NonEmptyStack<bool>,
     arguments_needs_transform_stack: NonEmptyStack<bool>,
     renamed_arguments_symbol_ids: FxHashSet<SymbolId>,
     // TODO(improve-on-babel): `FxHashMap` would suffice here. Iteration order is not important.
@@ -161,7 +161,7 @@ impl ArrowFunctionConverter<'_> {
             mode,
             this_var_stack: SparseStack::new(),
             arguments_var_stack: SparseStack::new(),
-            constructor_super_stack: Stack::new(),
+            constructor_super_stack: NonEmptyStack::new(false),
             arguments_needs_transform_stack: NonEmptyStack::new(false),
             renamed_arguments_symbol_ids: FxHashSet::default(),
             super_methods: None,
@@ -193,6 +193,11 @@ impl<'a> Traverse<'a> for ArrowFunctionConverter<'a> {
         debug_assert!(self.this_var_stack.last().is_none());
         debug_assert!(self.arguments_var_stack.len() == 1);
         debug_assert!(self.arguments_var_stack.last().is_none());
+        debug_assert!(self.constructor_super_stack.len() == 1);
+        // TODO: This assertion currently failing because we don't handle `super` in arrow functions
+        // in class static properties correctly.
+        // e.g. `class C { static f = async () => super.prop; }`
+        // debug_assert!(self.constructor_super_stack.last() == &false);
     }
 
     fn enter_function(&mut self, func: &mut Function<'a>, ctx: &mut TraverseCtx<'a>) {
@@ -336,9 +341,7 @@ impl<'a> Traverse<'a> for ArrowFunctionConverter<'a> {
                 self.get_this_identifier(this.span, ctx).map(Expression::Identifier)
             }
             Expression::Super(_) => {
-                if let Some(v) = self.constructor_super_stack.last_mut() {
-                    *v = true;
-                }
+                *self.constructor_super_stack.last_mut() = true;
                 return;
             }
             Expression::CallExpression(call) => self.transform_call_expression_for_super(call, ctx),
@@ -1079,8 +1082,7 @@ impl<'a> ArrowFunctionConverter<'a> {
         if let Some(this_var) = this_var {
             let is_constructor = ctx.scopes().get_flags(target_scope_id).is_constructor();
             let init = if is_constructor
-                && (super_method_count != 0
-                    || self.constructor_super_stack.last().copied().unwrap_or(false))
+                && (super_method_count != 0 || *self.constructor_super_stack.last())
             {
                 // `super()` is called in the constructor body, so we need to insert `_this = this;`
                 // after `super()` call. Because `this` is not available before `super()` call.