From 41d2ceb8d3de24738f9bd3ea3ef9ca3a2e72e92a Mon Sep 17 00:00:00 2001 From: Claude Bot Date: Thu, 11 Dec 2025 01:39:16 +0000 Subject: [PATCH] refactor: move bun:bundle handling to parse time and optimize MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Per Jarred's review feedback: - Move bun:bundle import handling from visit time (visitStmt.zig) to parse time (processImportStatement in P.zig), similar to how macros are handled - Move bundler_feature_flag_ref.isValid() check before the function call to avoid extra stack memory usage from copying values 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude --- src/ast/P.zig | 29 +++++++++++++++++++++++++++++ src/ast/visitExpr.zig | 14 +++++++------- src/ast/visitStmt.zig | 34 ++-------------------------------- 3 files changed, 38 insertions(+), 39 deletions(-) diff --git a/src/ast/P.zig b/src/ast/P.zig index 0ea8bbb5ab..7aff168548 100644 --- a/src/ast/P.zig +++ b/src/ast/P.zig @@ -2658,6 +2658,35 @@ pub fn NewParser_( return p.s(S.Empty{}, loc); } + // Handle `import { feature } from "bun:bundle"` - this is a special import + // that provides static feature flag checking at bundle time. + // We handle it here at parse time (similar to macros) rather than at visit time. + if (strings.eqlComptime(path.text, "bun:bundle")) { + // Look for the "feature" import and validate specifiers + for (stmt.items) |item| { + // In ClauseItem from parseImportClause: + // - alias is the name from the source module ("feature") + // - original_name is the local binding name + // - name.ref is the ref for the local binding + if (strings.eqlComptime(item.alias, "feature")) { + // Check for duplicate imports of feature + if (p.bundler_feature_flag_ref.isValid()) { + try p.log.addError(p.source, item.alias_loc, "`feature` from \"bun:bundle\" may only be imported once"); + continue; + } + // Declare the symbol and store the ref + const name = p.loadNameFromRef(item.name.ref.?); + const ref = try p.declareSymbol(.other, item.name.loc, name); + p.bundler_feature_flag_ref = ref; + } else { + // Warn about unknown specifiers + try p.log.addWarningFmt(p.source, item.alias_loc, p.allocator, "\"bun:bundle\" only exports \"feature\"; \"{s}\" will be undefined", .{item.alias}); + } + } + // Return empty statement - the import is completely removed + return p.s(S.Empty{}, loc); + } + const macro_remap = if (comptime allow_macros) p.options.macro_context.getRemap(path.text) else diff --git a/src/ast/visitExpr.zig b/src/ast/visitExpr.zig index 4d6def0f51..92ff7c7089 100644 --- a/src/ast/visitExpr.zig +++ b/src/ast/visitExpr.zig @@ -1278,8 +1278,12 @@ pub fn VisitExpr( } // Handle `feature("FLAG_NAME")` calls from `import { feature } from "bun:bundle"` - if (maybeReplaceBundlerFeatureCall(p, e_, expr.loc)) |result| { - return result; + // Check if the bundler_feature_flag_ref is set before calling the function + // to avoid stack memory usage from copying values back and forth. + if (p.bundler_feature_flag_ref.isValid()) { + if (maybeReplaceBundlerFeatureCall(p, e_, expr.loc)) |result| { + return result; + } } if (e_.target.data == .e_require_call_target) { @@ -1645,12 +1649,8 @@ pub fn VisitExpr( /// bundlers to eliminate dead code branches at build time. /// /// Returns the replacement expression if this is a feature() call, or null otherwise. + /// Note: Caller must check `p.bundler_feature_flag_ref.isValid()` before calling. fn maybeReplaceBundlerFeatureCall(p: *P, e_: *E.Call, loc: logger.Loc) ?Expr { - // Quick check: is the bundler_feature_flag_ref even set? - if (!p.bundler_feature_flag_ref.isValid()) { - return null; - } - // Check if the target is the `feature` function from "bun:bundle" // It could be e_identifier (for unbound) or e_import_identifier (for imports) const target_ref: ?Ref = switch (e_.target.data) { diff --git a/src/ast/visitStmt.zig b/src/ast/visitStmt.zig index 778aeb4619..f85544a71d 100644 --- a/src/ast/visitStmt.zig +++ b/src/ast/visitStmt.zig @@ -40,38 +40,8 @@ pub fn VisitStmt( const visitors = struct { pub fn s_import(noalias p: *P, noalias stmts: *ListManaged(Stmt), noalias stmt: *Stmt, noalias data: *S.Import) !void { - // Handle `import { feature } from "bun:bundle"` - this is a special import - // that provides static feature flag checking at bundle time. - const import_record = &p.import_records.items[data.import_record_index]; - if (strings.eqlComptime(import_record.path.text, "bun:bundle")) { - // Look for the "feature" import and validate specifiers - for (data.items) |*item| { - // In ClauseItem from parseImportClause: - // - alias is the name from the source module ("feature" in both cases) - // - original_name is the local binding name (empty for `import { feature }`, - // "checkFeature" for `import { feature as checkFeature }`) - // - name.ref is the ref for the local binding - if (strings.eqlComptime(item.alias, "feature")) { - // Check for duplicate imports of feature - if (p.bundler_feature_flag_ref.isValid()) { - p.log.addError(p.source, item.alias_loc, "`feature` from \"bun:bundle\" may only be imported once") catch unreachable; - continue; - } - // Record the symbol so it's properly tracked - try p.recordDeclaredSymbol(item.name.ref.?); - // Assign the ref to bundler_feature_flag_ref so we can detect - // feature() calls in e_call visitor - p.bundler_feature_flag_ref = item.name.ref.?; - } else { - // Warn about unknown specifiers - p.log.addWarningFmt(p.source, item.alias_loc, p.allocator, "\"bun:bundle\" only exports \"feature\"; \"{s}\" will be undefined", .{item.alias}) catch unreachable; - } - } - // Mark this import as unused so it gets removed from the output - import_record.is_unused = true; - // Return early without appending the statement (effectively deleting it) - return; - } + // Note: `import { feature } from "bun:bundle"` is handled at parse time + // in processImportStatement(), not here at visit time. try p.recordDeclaredSymbol(data.namespace_ref);