js_parser: admit a macro result to const_values only for const, and narrow a destructured result into a new node

src/ast/ast_result.rs

@@ -10,8 +10,8 @@ use bun_collections::{ArrayHashMap, StringArrayHashMap, StringHashMap};
 
 use crate::runtime;
 use crate::{
-    CharFreq, ExportsKind, Expr, InlinedEnumValue, LocRef, NamedExport, NamedImport, Part, Range,
-    Ref, Scope, SlotCounts, StoreStr, Target,
+    CharFreq, ExportsKind, Expr, ExprData, InlinedEnumValue, LocRef, LocalKind, NamedExport,
+    NamedImport, Part, Range, Ref, Scope, SlotCounts, StoreStr, Target,
 };
 
 use crate::part::List as PartList;
@@ -256,7 +256,38 @@ pub struct DynamicImportItem {
 }
 pub type DynamicImportAliases = ArrayHashMap<u32, DynamicImportUse, AutoContext, AstAlloc>;
 pub type NamedExports = StringArrayHashMap<NamedExport, StringContext, AstAlloc>;
-pub type ConstValuesMap = ArrayHashMap<Ref, Expr, AutoContext, AstAlloc>;
+/// Printed in place of each read of a `const`. A macro object or array is copied per read (#7196).
+#[derive(Clone, Copy)]
+#[repr(transparent)]
+pub struct ConstValue(Expr);
+
+impl ConstValue {
+    /// `has_default`: a destructuring leaf with a default binds the default over `undefined`.
+    #[inline]
+    pub fn new(kind: LocalKind, has_default: bool, value: Expr) -> Option<Self> {
+        let admit = kind == LocalKind::KConst
+            && match &value.data {
+                ExprData::ENumber(_)
+                | ExprData::EBoolean(_)
+                | ExprData::EBranchBoolean(_)
+                | ExprData::ENull(_)
+                | ExprData::EInlinedEnum(_) => true,
+                ExprData::EUndefined(_) => !has_default,
+                ExprData::EString(str) => str.next.is_none(),
+                ExprData::EArray(array) => array.was_originally_macro,
+                ExprData::EObject(object) => object.was_originally_macro,
+                _ => false,
+            };
+        admit.then_some(Self(value))
+    }
+
+    #[inline]
+    pub fn expr(self) -> Expr {
+        self.0
+    }
+}
+
+pub type ConstValuesMap = ArrayHashMap<Ref, ConstValue, AutoContext, AstAlloc>;
 pub type TsEnumsMap =
     ArrayHashMap<Ref, StringHashMap<InlinedEnumValue, AstAlloc>, AutoContext, AstAlloc>;
 /// `import X ...; X.name` where `X` resolved to a module namespace while

src/ast/expr.rs

@@ -65,10 +65,6 @@ impl Expr {
         }
     }
 
-    pub fn can_be_const_value(&self) -> bool {
-        self.data.can_be_const_value()
-    }
-
     pub fn can_be_moved(&self) -> bool {
         self.data.can_be_moved()
     }
@@ -2255,24 +2251,6 @@ impl Data {
 }
 
 impl Data {
-    /// "const values" here refers to expressions that can participate in constant
-    /// inlining, as they have no side effects on instantiation, and there would be
-    /// no observable difference if duplicated. This is a subset of canBeMoved()
-    pub(crate) fn can_be_const_value(&self) -> bool {
-        match self {
-            Data::ENumber(_)
-            | Data::EBoolean(_)
-            | Data::EBranchBoolean(_)
-            | Data::ENull(_)
-            | Data::EUndefined(_)
-            | Data::EInlinedEnum(_) => true,
-            Data::EString(str) => str.next.is_none(),
-            Data::EArray(array) => array.was_originally_macro,
-            Data::EObject(object) => object.was_originally_macro,
-            _ => false,
-        }
-    }
-
     /// Expressions that can be moved are those that do not have side
     /// effects on their own. This is used to determine what can be moved
     /// outside of a module wrapper (__esm/__commonJS).

src/ast/lib.rs

@@ -2987,8 +2987,8 @@ pub mod loader;
 pub mod target;
 
 pub use ast_result::{
-    Ast, CommonJSNamedExport, CommonJSNamedExports, ConstValuesMap, NamedExports, NamedImports,
-    TopLevelSymbolToParts, TsEnumsMap,
+    Ast, CommonJSNamedExport, CommonJSNamedExports, ConstValue, ConstValuesMap, NamedExports,
+    NamedImports, TopLevelSymbolToParts, TsEnumsMap,
 };
 pub use import_record::{Flags as ImportRecordFlags, ImportRecord, Tag as ImportRecordTag};
 pub use loader::{Loader, LoaderHashTable, SideEffects};

src/js_parser/p.rs

@@ -2188,7 +2188,7 @@ impl<'a, const TYPESCRIPT: bool, const SCAN_ONLY: bool> P<'a, TYPESCRIPT, SCAN_O
 
         if self.options.features.inlining {
             if let Some(replacement) = self.const_values.get(&ref_) {
-                let replacement = *replacement;
+                let replacement = replacement.expr();
                 self.ignore_usage(ref_);
                 return replacement;
             }

src/js_parser/parser.rs

@@ -711,7 +711,7 @@ pub struct VisitArgsOpts<'a> {
 #[derive(Clone, Copy)]
 pub struct VisitDeclOpts {
     pub(crate) was_anonymous_named_expr: bool,
-    pub(crate) could_be_const_value: bool,
+    pub(crate) kind: js_ast::LocalKind,
     pub(crate) could_be_macro: bool,
 }
 

src/js_parser/visit/mod.rs

@@ -279,7 +279,6 @@ impl<'a, const TYPESCRIPT: bool, const SCAN_ONLY: bool> P<'a, TYPESCRIPT, SCAN_O
         kind: LocalKind,
         is_export: bool,
     ) -> usize {
-        let was_const = kind == LocalKind::KConst;
         // A namespace member isn't a module export.
         let can_replace_exports =
             IS_POSSIBLY_DECL_TO_REMOVE && self.enclosing_namespace_arg_ref.is_none();
@@ -592,12 +591,11 @@ impl<'a, const TYPESCRIPT: bool, const SCAN_ONLY: bool> P<'a, TYPESCRIPT, SCAN_O
                     }
                 }
 
-                let is_after = self.vis_scope().is_after_const_local_prefix;
                 self.visit_decl(
                     decl,
                     VisitDeclOpts {
                         was_anonymous_named_expr,
-                        could_be_const_value: was_const && !is_after,
+                        kind,
                         could_be_macro: if Self::ALLOW_MACROS {
                             prev_macro_call_count != self.macro_call_count
                         } else {
@@ -622,12 +620,11 @@ impl<'a, const TYPESCRIPT: bool, const SCAN_ONLY: bool> P<'a, TYPESCRIPT, SCAN_O
                         if !self.replace_decl_and_possibly_remove(decl, replacer) {
                             continue 'outer;
                         }
-                        let is_after = self.vis_scope().is_after_const_local_prefix;
                         self.visit_decl(
                             decl,
                             VisitDeclOpts {
                                 was_anonymous_named_expr: false,
-                                could_be_const_value: was_const && !is_after,
+                                kind,
                                 could_be_macro: false,
                             },
                         );
@@ -652,98 +649,140 @@ impl<'a, const TYPESCRIPT: bool, const SCAN_ONLY: bool> P<'a, TYPESCRIPT, SCAN_O
         j
     }
 
-    pub(crate) fn visit_binding_and_expr_for_macro(&mut self, binding: Binding, expr: Expr) {
-        match binding.data {
-            BData::BObject(bound_object) => {
-                let bound_object = bound_object.get();
-                if let ExprData::EObject(mut object) = expr.data {
-                    if object.was_originally_macro {
-                        for property in bound_object.properties() {
-                            if property.flags.contains(flags::Property::IsSpread) {
-                                return;
-                            }
-                        }
-                        let mut end: u32 = 0;
-                        for property in bound_object.properties() {
-                            if let Some(name) = property.key.as_string_literal(self.arena) {
-                                if let Some(query) = object.as_property(name) {
-                                    match query.expr.data {
-                                        ExprData::EObject(_) | ExprData::EArray(_) => {
-                                            self.visit_binding_and_expr_for_macro(
-                                                property.value,
-                                                query.expr,
-                                            );
-                                        }
-                                        _ => {
-                                            if self.options.features.inlining {
-                                                if let BData::BIdentifier(id) = property.value.data
-                                                {
-                                                    self.const_values
-                                                        .put(id.r#ref, query.expr)
-                                                        .expect("oom");
-                                                }
-                                            }
-                                        }
-                                    }
-                                    let i = query.i as usize;
-                                    if i >= end as usize {
-                                        object.properties.slice_mut().swap(i, end as usize);
-                                        end += 1;
-                                    }
-                                }
-                            }
-                        }
-                        object.properties.truncate(end as usize);
+    /// Records the `const` leaves of a destructured macro result and narrows it to the bound parts.
+    pub(crate) fn visit_binding_and_expr_for_macro(
+        &mut self,
+        binding: Binding,
+        expr: Expr,
+        kind: LocalKind,
+        has_default: bool,
+    ) -> Expr {
+        match (binding.data, expr.data) {
+            (BData::BObject(bound), ExprData::EObject(object)) if object.was_originally_macro => {
+                let bound = bound.get();
+                let is_flat =
+                    |key: &Expr| matches!(key.data, ExprData::EString(key) if key.next.is_none());
+                let mut narrow = true;
+                for property in bound.properties() {
+                    if property.flags.contains(flags::Property::IsSpread) {
+                        return expr;
                     }
+                    // A key that is not one flat string can read any property.
+                    narrow &= is_flat(&property.key);
                 }
-            }
-            BData::BArray(bound_array) => {
-                let bound_array = bound_array.get();
-                if let ExprData::EArray(mut array) = expr.data {
-                    if array.was_originally_macro && !bound_array.has_spread {
-                        let bound_items = bound_array.items();
-                        let trunc_n = array.items.len().min(bound_items.len());
-                        array.items.truncate(trunc_n);
-                        let n = array.items.len_u32() as usize;
-                        for (item, child_expr) in bound_items[..n]
-                            .iter()
-                            .zip(array.items.slice_mut().iter_mut())
-                        {
-                            if matches!(item.binding.data, BData::BMissing(_)) {
-                                *child_expr = self.new_expr(E::Missing {}, expr.loc);
-                                continue;
-                            }
-                            self.visit_binding_and_expr_for_macro(item.binding, *child_expr);
+                let mut kept = G::PropertyList::init_capacity(bound.properties().len());
+                let mut kept_from: BumpVec<'a, u32> = BumpVec::new_in(self.arena);
+                for property in bound.properties() {
+                    if !is_flat(&property.key) {
+                        continue;
+                    }
+                    let Some(name) = property.key.as_string_literal(self.arena) else {
+                        continue;
+                    };
+                    let Some(query) = object.as_property(name) else {
+                        continue;
+                    };
+                    let value = self.visit_binding_and_expr_for_macro(
+                        property.value,
+                        query.expr,
+                        kind,
+                        property.default_value.is_some(),
+                    );
+                    if !narrow {
+                        continue;
+                    }
+                    match kept_from.iter().position(|&from| from == query.i) {
+                        // The pattern binds this key again. Keep the whole value.
+                        Some(at) => kept[at].value = Some(query.expr),
+                        None => {
+                            let mut copy =
+                                object.properties.slice()[query.i as usize].shallow_clone();
+                            copy.value = Some(value);
+                            kept_from.push(query.i);
+                            kept.push(copy);
                         }
                     }
                 }
+                if !narrow {
+                    return expr;
+                }
+                self.new_expr(
+                    E::Object {
+                        properties: kept,
+                        is_single_line: object.is_single_line,
+                        was_originally_macro: true,
+                        close_brace_loc: object.close_brace_loc,
+                        ..Default::default()
+                    },
+                    expr.loc,
+                )
             }
-            BData::BIdentifier(id) => {
+            (BData::BArray(bound), ExprData::EArray(array))
+                if array.was_originally_macro && !bound.has_spread =>
+            {
+                let bound_items = bound.get().items();
+                let n = array.items.len().min(bound_items.len());
+                let mut items = ExprNodeList::init_capacity(n);
+                for (item, child) in bound_items[..n].iter().zip(&array.items.slice()[..n]) {
+                    items.push(if matches!(item.binding.data, BData::BMissing(_)) {
+                        self.new_expr(E::Missing {}, expr.loc)
+                    } else {
+                        self.visit_binding_and_expr_for_macro(
+                            item.binding,
+                            *child,
+                            kind,
+                            item.default_value.is_some(),
+                        )
+                    });
+                }
+                self.new_expr(
+                    E::Array {
+                        items,
+                        is_single_line: array.is_single_line,
+                        was_originally_macro: true,
+                        close_bracket_loc: array.close_bracket_loc,
+                        ..Default::default()
+                    },
+                    expr.loc,
+                )
+            }
+            (BData::BIdentifier(id), _) => {
                 if self.options.features.inlining {
-                    self.const_values.put(id.r#ref, expr).expect("oom");
+                    self.admit_const_value(id.r#ref, kind, has_default, expr);
                 }
+                expr
             }
-            BData::BMissing(_) => {}
+            _ => expr,
+        }
+    }
+
+    /// The only writer of `const_values`.
+    #[inline]
+    fn admit_const_value(&mut self, ref_: Ref, kind: LocalKind, has_default: bool, value: Expr) {
+        if let Some(value) = js_ast::ConstValue::new(kind, has_default, value) {
+            self.const_values.put(ref_, value).expect("oom");
         }
     }
 
     pub(crate) fn visit_decl(&mut self, decl: &mut G::Decl, opts: VisitDeclOpts) {
         let VisitDeclOpts {
             was_anonymous_named_expr,
-            could_be_const_value,
+            kind,
             could_be_macro,
         } = opts;
         // Optionally preserve the name
         match decl.binding.data {
             BData::BIdentifier(id) => {
                 let id_ref = id.r#ref;
-                if could_be_const_value || (Self::ALLOW_MACROS && could_be_macro) {
+                // A macro result is exempt from the const-local prefix and does not end it.
+                let is_macro = Self::ALLOW_MACROS && could_be_macro;
+                if kind == LocalKind::KConst
+                    && (is_macro || !self.vis_scope().is_after_const_local_prefix)
+                {
                     if let Some(val) = decl.value {
-                        if val.can_be_const_value() {
-                            self.const_values.put(id_ref, val).expect("oom");
-                        }
+                        self.admit_const_value(id_ref, kind, false, val);
                     }
-                } else {
+                } else if !is_macro {
                     self.vis_scope().is_after_const_local_prefix = true;
                 }
                 // SAFETY: original_name is arena-owned, valid for 'a.
@@ -759,7 +798,12 @@ impl<'a, const TYPESCRIPT: bool, const SCAN_ONLY: bool> P<'a, TYPESCRIPT, SCAN_O
             BData::BObject(_) | BData::BArray(_) => {
                 if Self::ALLOW_MACROS {
                     if could_be_macro && let Some(value) = decl.value {
-                        self.visit_binding_and_expr_for_macro(decl.binding, value);
+                        decl.value = Some(self.visit_binding_and_expr_for_macro(
+                            decl.binding,
+                            value,
+                            kind,
+                            false,
+                        ));
                     }
                 }
             }
@@ -1852,11 +1896,7 @@ impl<'a, const TYPESCRIPT: bool, const SCAN_ONLY: bool> P<'a, TYPESCRIPT, SCAN_O
                         | StmtData::SDebugger(_)
                         | StmtData::STypeScript(_) => continue,
                         StmtData::SLocal(mut local) => {
-                            // "using" / "await using" declarations have disposal
-                            // side-effects on scope exit. Their refs can end up in
-                            // `const_values` via the macro path in `visitDecl`
-                            // (`could_be_macro`), so skip them here to avoid
-                            // silently dropping the declaration.
+                            // `using` has disposal side effects on scope exit and is never in `const_values`.
                             if local.kind.is_using() {
                                 continue;
                             }