diff --git a/src/ast/e.rs b/src/ast/e.rs index 7ae43d52aae1..88edbc752911 100644 --- a/src/ast/e.rs +++ b/src/ast/e.rs @@ -2036,7 +2036,7 @@ impl EString { } } - /// Link `other` onto this string's rope tail. + /// Link `other` onto this string's rope tail. Mutates both ropes: neither may have another owner. /// /// `other` MUST be Store/arena-allocated (callers pass /// `Expr::init(EString, ...).data.e_string_mut()` or a freshly diff --git a/src/ast/fold_string_addition.rs b/src/ast/fold_string_addition.rs index be74632ff71c..e096e0a3f02a 100644 --- a/src/ast/fold_string_addition.rs +++ b/src/ast/fold_string_addition.rs @@ -1,111 +1,14 @@ use crate::expr::{Data, PrimitiveType, data}; -use crate::{E, Expr, StoreRef, e}; +use crate::{E, Expr, e}; use bun_alloc::Arena; // bumpalo::Bump re-export -// ── local rope helpers ───────────────────────────────────────────────────── -// `EString` has no `push` / `clone_rope_nodes` inherent methods yet; -// provide the minimal surface here. +/// Links both ropes into the result (see `EString::push`); sound because shared strings are never ropes. +fn join_strings(left: &E::EString, right: &E::EString) -> E::EString { + let mut new = left.shallow_clone(); + let mut rhs = data::Store::append(right.shallow_clone()); -#[inline] -fn store_append_string(s: E::EString) -> StoreRef { - data::Store::append(s) -} - -/// Link `other` onto `lhs`'s rope tail. -fn estring_push(lhs: &mut E::EString, mut other: StoreRef) { - debug_assert!(lhs.is_utf8()); - debug_assert!(other.is_utf8()); - - // `other` is a freshly Store-appended node; mutate via `StoreRef::DerefMut`. - if other.rope_len == 0 { - other.rope_len = other.data.len() as u32; - } - if lhs.rope_len == 0 { - lhs.rope_len = lhs.data.len() as u32; - } - lhs.rope_len += other.rope_len; - - if lhs.next.is_none() { - lhs.next = Some(other); - lhs.end = Some(other); - } else { - let mut end = lhs.end.unwrap(); - while end.get().next.is_some() { - end = end.get().end.unwrap(); - } - // `end` points into the live Store; rope nodes are mutated in place - // via `StoreRef::DerefMut` (single-threaded visitor). - end.next = Some(other); - lhs.end = Some(other); - } -} - -/// Deep-copy the `next` chain into fresh Store nodes so mutating the result -/// can't alias an inlined-enum's string. -fn clone_rope_nodes(s: &E::EString) -> E::EString { - let mut root = s.shallow_clone(); - if let Some(first) = root.next { - // Clone the first link, then walk the freshly-cloned chain via - // `StoreRef` (safe `Deref`/`DerefMut`) instead of a raw `*mut` - // cursor. Each cloned node's `next` still points at the original - // chain (shallow clone), so re-clone link-by-link. - let mut tail: StoreRef = store_append_string(first.get().shallow_clone()); - root.next = Some(tail); - while let Some(next) = tail.next { - let cloned = store_append_string(next.get().shallow_clone()); - tail.next = Some(cloned); - tail = cloned; - } - root.end = Some(tail); - } - root -} - -/// Concatenate two `E::String`s, mutating BOTH inputs -/// unless `has_inlined_enum_poison` is set. -/// -/// Currently inlined enum poison refers to where mutation would cause output -/// bugs due to inlined enum values sharing `E::String`s. If a new use case -/// besides inlined enums comes up to set this to true, please rename the -/// variable and document it. -fn join_strings( - left: &E::EString, - right: &E::EString, - has_inlined_enum_poison: bool, -) -> E::EString { - let mut new = if has_inlined_enum_poison { - // Inlined enums can be shared by multiple call sites. In - // this case, we need to ensure that the ENTIRE rope is - // cloned. In other situations, the lhs doesn't have any - // other owner, so it is fine to mutate `lhs.data.end.next`. - // - // Consider the following case: - // const enum A { - // B = "a" + "b", - // D = B + "d", - // }; - // console.log(A.B, A.D); - clone_rope_nodes(left) - } else { - left.shallow_clone() - }; - - // Similarly, the right side has to be cloned for an enum rope too. - // - // Consider the following case: - // const enum A { - // B = "1" + "2", - // C = ("3" + B) + "4", - // }; - // console.log(A.B, A.C); - let rhs_clone = store_append_string(if has_inlined_enum_poison { - clone_rope_nodes(right) - } else { - right.shallow_clone() - }); - - estring_push(&mut new, rhs_clone); - new.prefer_template = new.prefer_template || rhs_clone.get().prefer_template; + new.push(&mut *rhs); + new.prefer_template = new.prefer_template || right.prefer_template; new } @@ -179,11 +82,8 @@ pub fn fold_string_addition( // "bar" + "baz" => "barbaz" Data::EString(right) => { if right.is_utf8() { - let has_inlined_enum_poison = matches!(l.data, Data::EInlinedEnum(_)) - || matches!(r.data, Data::EInlinedEnum(_)); - return Some(Expr::init( - join_strings(left.get(), right.get(), has_inlined_enum_poison), + join_strings(left.get(), right.get()), lhs.loc, )); } @@ -198,7 +98,6 @@ pub fn fold_string_addition( head: e::TemplateContents::Cooked(join_strings( left.get(), right.head.cooked(), - matches!(l.data, Data::EInlinedEnum(_)), )), }, l.loc, @@ -250,17 +149,12 @@ pub fn fold_string_addition( let new_tail = e::TemplateContents::Cooked(join_strings( last_tail.cooked(), right.get(), - matches!(r.data, Data::EInlinedEnum(_)), )); left.parts_mut()[i].tail = new_tail; return Some(lhs); } } else if left.head.is_utf8() { - let new_head = join_strings( - left.head.cooked(), - right.get(), - matches!(r.data, Data::EInlinedEnum(_)), - ); + let new_head = join_strings(left.head.cooked(), right.get()); left.head = e::TemplateContents::Cooked(new_head); return Some(lhs); } @@ -276,7 +170,6 @@ pub fn fold_string_addition( let new_tail = e::TemplateContents::Cooked(join_strings( last_tail.cooked(), right.head.cooked(), - matches!(r.data, Data::EInlinedEnum(_)), )); left.parts_mut()[i].tail = new_tail; @@ -289,11 +182,8 @@ pub fn fold_string_addition( return Some(lhs); } } else if left.head.is_utf8() && right.head.is_utf8() { - let new_head = join_strings( - left.head.cooked(), - right.head.cooked(), - matches!(r.data, Data::EInlinedEnum(_)), - ); + let new_head = + join_strings(left.head.cooked(), right.head.cooked()); left.head = e::TemplateContents::Cooked(new_head); left.parts = right.parts; return Some(lhs); diff --git a/src/js_parser/p.rs b/src/js_parser/p.rs index 7aba6f7d8d38..31ffcc1c6a07 100644 --- a/src/js_parser/p.rs +++ b/src/js_parser/p.rs @@ -7083,6 +7083,13 @@ impl<'a, const TYPESCRIPT: bool, const SCAN_ONLY: bool> P<'a, TYPESCRIPT, SCAN_O #[cold] #[inline(never)] pub(crate) fn wrap_inlined_enum(&mut self, value: Expr, comment: &'a [u8]) -> Expr { + debug_assert!( + value + .data + .as_e_string() + .is_none_or(|str_| str_.next.is_none()), + "inlined enum strings must be flat, string folding appends to rope chains in place" + ); if strings::contains(comment, b"*/") { // Don't wrap with a comment return value; diff --git a/src/js_parser/visit/visit_stmt.rs b/src/js_parser/visit/visit_stmt.rs index 5de468c08a2e..f399550709e3 100644 --- a/src/js_parser/visit/visit_stmt.rs +++ b/src/js_parser/visit/visit_stmt.rs @@ -2268,9 +2268,12 @@ impl<'a, const TYPESCRIPT: bool, const SCAN_ONLY: bool> P<'a, TYPESCRIPT, SCAN_O next_numeric_value = Some(num.value() + 1.0); } - js_ast::ExprData::EString(str_) => { + js_ast::ExprData::EString(mut str_) => { has_string_value = true; + // Inlined uses share this node's rope and folds append to ropes in place: store it flat. + str_.resolve_rope_if_needed(p.arena); + exported_members.get_ptr_mut(name).unwrap().data = js_ast::ts::Data::EnumString(str_); diff --git a/src/jsc/RuntimeTranspilerCache.rs b/src/jsc/RuntimeTranspilerCache.rs index a37b8714fbba..c3e5b5698f81 100644 --- a/src/jsc/RuntimeTranspilerCache.rs +++ b/src/jsc/RuntimeTranspilerCache.rs @@ -57,7 +57,8 @@ bun_core::declare_scope!(cache, visible); /// Version 27: ModuleInfo string table holds Latin-1 / UTF-16 bodies, not WTF-8. /// Version 28: the define table and `--drop` entries participate in the features hash. /// Version 29: `new Array(x, ...spread)` is no longer folded into an array literal. -const EXPECTED_VERSION: u32 = 29; +/// Version 30: String enum members are stored flat, so folds no longer append onto an inlined member. +const EXPECTED_VERSION: u32 = 30; /// Source files smaller than this are not written to / read from the on-disk /// transpiler cache. Originally 50 KiB, which excluded almost every file in a diff --git a/test/bundler/bundler_edgecase.test.ts b/test/bundler/bundler_edgecase.test.ts index a1100fb73a3b..42ffd8c80016 100644 --- a/test/bundler/bundler_edgecase.test.ts +++ b/test/bundler/bundler_edgecase.test.ts @@ -1435,6 +1435,49 @@ describe("bundler", () => { stdout: "12 312\n12 3124", }, }); + // A member initialized with a concatenation is stored as a rope. Folding a + // template literal that inlines it used to append the template's text onto + // that rope, changing the member everywhere: its declaration, every inlined + // use in the same file and every cross-module use. + itBundled("edgecase/EnumInliningRopeStringTemplateLiteral", { + files: { + "/entry.ts": /* ts */ ` + import { Routes, users } from "./routes"; + enum Local { + X = "x" + "y", + } + function tail(prefix: string) { + return \`\${prefix}-\${Local.X}-z\`; + } + console.log(users); + console.log(Routes.Base); + console.log(\`\${Routes.Base}/posts\`); + console.log(JSON.stringify(Routes)); + console.log(tail("p")); + console.log(Local.X); + console.log(JSON.stringify(Local)); + `, + "/routes.ts": /* ts */ ` + export enum Routes { + Base = "/api" + "/v1", + Health = "/health", + } + export const users = \`\${Routes.Base}/users\`; + `, + }, + minifySyntax: true, + run: { + stdout: ` + /api/v1/users + /api/v1 + /api/v1/posts + {"Base":"/api/v1","Health":"/health"} + p-xy-z + xy + {"X":"xy"} + `, + }, + }); itBundled("edgecase/ProtoNullProtoInlining", { files: { "/entry.ts": ` diff --git a/test/bundler/transpiler/transpiler.test.js b/test/bundler/transpiler/transpiler.test.js index 6ef742958718..50943c5be341 100644 --- a/test/bundler/transpiler/transpiler.test.js +++ b/test/bundler/transpiler/transpiler.test.js @@ -267,6 +267,85 @@ describe("Bun.Transpiler", () => { expect(lastLine(ts.parsedMin(pre + 'export let y = Foo?.["A"];', false))).toBe("export let y = Foo?.A;"); expect(lastLine(ts.parsedMin(pre + 'export let y = Bar?.["a-b"];', false))).toBe('export let y = Bar?.["a-b"];'); }); + + // `B = "a" + "b"` is folded into a rope, and every inlined `A.B` pointed at + // that rope. Folding a template literal around it used to append the + // template's text onto the rope itself, so the member's declaration and all + // of its other uses changed too. + describe("template literal around an inlined string enum member", () => { + const pre = 'enum A { B = "a" + "b", C = B + "c" }\n'; + const decl = 'var A;\n((A) => {\n A.B = "ab";\n A.C = "abc";\n})(A ||= {});\n'; + + it("member as the first part of the literal", () => { + expect(ts.parsedMin(pre + "console.log(`${A.B}-x`, A.B);", false)).toBe( + decl + 'console.log("ab-x", "ab" /* B */);\n', + ); + }); + it("member after a part that cannot be folded", () => { + expect(ts.parsedMin(pre + "console.log(`${y}${A.B}-x`, A.B);", false)).toBe( + decl + 'console.log(`${y}ab-x`, "ab" /* B */);\n', + ); + }); + it("member derived from another rope member", () => { + expect(ts.parsedMin(pre + "console.log(`${A.C}-x`, A.C);", false)).toBe( + decl + 'console.log("abc-x", "abc" /* C */);\n', + ); + }); + it("template inside the enum body, which folds even without minification", () => { + expect(ts.parsed('enum A { B = "a" + "b", C = `${B}-c`, D = `${B}` }\nconsole.log(A.B);', false)).toBe( + 'var A;\n((A) => {\n A["B"] = "ab";\n A["C"] = "ab-c";\n A["D"] = "ab";\n})(A ||= {});\nconsole.log("ab" /* B */);\n', + ); + }); + it("at runtime, including the same member in several templates", async () => { + // Not concurrent: before the fix the second template crashed the + // transpiler and the fourth one made it loop forever, and the test + // runner only kills a dangling child of a non-concurrent test. + await using proc = Bun.spawn({ + cmd: [ + bunExe(), + "-e", + `enum Routes { + Base = "/api" + "/v1", + Users = Base + "/users", + Health = "/health", + } + function tail(prefix: string) { + return \`\${prefix}\${Routes.Base}/y\`; + } + console.log( + [ + \`\${Routes.Base}/posts\`, + tail("q"), + \`\${Routes.Users}!\`, + \`\${Routes.Base}\${Routes.Base}\`, + Routes.Base, + Routes.Users, + Routes.Health, + JSON.stringify(Routes), + ].join("\\n"), + );`, + ], + env: bunEnv, + stderr: "pipe", + }); + const [stdout, stderr, exitCode] = await Promise.all([proc.stdout.text(), proc.stderr.text(), proc.exited]); + expect({ stdout, stderr, exitCode }).toEqual({ + stdout: [ + "/api/v1/posts", + "q/api/v1/y", + "/api/v1/users!", + "/api/v1/api/v1", + "/api/v1", + "/api/v1/users", + "/health", + '{"Base":"/api/v1","Users":"/api/v1/users","Health":"/health"}', + "", + ].join("\n"), + stderr: "", + exitCode: 0, + }); + }); + }); }); describe("TypeScript", () => { diff --git a/test/js/bun/transpiler/transpiler-enum-concat-chain-oom.test.ts b/test/js/bun/transpiler/transpiler-enum-concat-chain-oom.test.ts new file mode 100644 index 000000000000..dad1f4f7cf9a --- /dev/null +++ b/test/js/bun/transpiler/transpiler-enum-concat-chain-oom.test.ts @@ -0,0 +1,107 @@ +// bun-fuzz: folding a long `+` chain of inlined string-enum members was +// quadratic in memory. String folding builds a rope of AST nodes instead of +// copying bytes, and an enum member's rope used to be shared by every +// reference to it. To avoid appending onto that shared rope, each `+` with an +// enum member operand deep-cloned BOTH operands' ropes, so the growing +// left-hand accumulator of `S.A + "k" + S.A + "k" + ...` was re-cloned in full +// at every enum term: 8k terms (49 KB of source) took 1.4 GB. The same chain +// over plain literals stayed flat. +import { expect, test } from "bun:test"; +import { bunEnv, bunExe } from "harness"; + +test.concurrent("long `+` chain of inlined enum members folds in linear memory", async () => { + const fixture = /* js */ ` + const rss = process.platform === "darwin" && typeof Bun.unsafe.memoryFootprint === "function" ? Bun.unsafe.memoryFootprint : process.memoryUsage.rss; + const n = 4096; + // The same chain twice: at top level (folds because target is "bun") and + // as an enum member initializer (always folds). + const src = + 'enum S { A = "value", B = "" + ' + '"k" + A + '.repeat(n) + '"" }\\n' + + 'capture1(' + 'S.A + "k" + '.repeat(n) + '"");\\n' + + 'capture2(S.B);\\n' + + 'export {};\\n'; + const before = rss(); + const out = new Bun.Transpiler({ loader: "ts", target: "bun" }).transformSync(src); + const after = rss(); + const m1 = out.match(/capture1\\("([^"]*)"\\)/); + const m2 = out.match(/capture2\\("([^"]*)" \\/\\* B \\*\\/\\)/); + if (!m1 || m1[1] !== "valuek".repeat(n)) throw new Error("capture1 folded wrong: " + JSON.stringify(out.slice(0, 200))); + if (!m2 || m2[1] !== "kvalue".repeat(n)) throw new Error("capture2 folded wrong: " + JSON.stringify(out.slice(0, 200))); + console.log(JSON.stringify({ delta_mb: (after - before) / 1024 / 1024 })); + `; + + await using proc = Bun.spawn({ + cmd: [bunExe(), "-e", fixture], + env: bunEnv, + stdout: "pipe", + stderr: "pipe", + }); + const [stdout, stderr, exitCode] = await Promise.all([proc.stdout.text(), proc.stderr.text(), proc.exited]); + + expect({ stdout: stdout.trim(), stderr, exitCode }).toEqual({ + stdout: expect.stringMatching(/^\{"delta_mb":/), + stderr: "", + exitCode: 0, + }); + const { delta_mb } = JSON.parse(stdout); + // Before the fix: ~1.5 GB in release for n=4096 (two chains). After: a few MB. + expect(delta_mb).toBeLessThan(150); +}); + +test.concurrent("folding onto an inlined enum member leaves the member's own value intact", async () => { + // B below folds to "12", and every later A.B reference is inlined from the + // stored member. No way of appending more string to such a reference may + // reach the member itself: member on the left of "+", on the right, on both + // sides, inside a template literal, and a member whose name cannot be + // printed as a trailing /* comment */ because it contains "*/". Before the + // fix the last two appended onto the member's shared rope, changed its value + // everywhere and then panicked the transpiler, so this runs out of process. + const src = /* ts */ ` + enum A { + B = "1" + "2", + L = B + "x", + R = "x" + B, + LR = B + B, + M = ("<" + B + ">") + B + "!", + T = \`t\${B}t\`, + "*/" = "s" + "t", + S2 = A["*/"] + "u", + } + capture(A.B, A.L, A.R, A.LR, A.M, A.T, A["*/"], A.S2); + capture(A.B + "y", "y" + A.B, A.B + A.B, \`a\${x}b\` + A.B + "c", A.B + \`b\${x}a\`, \`(\${A.B})\`, A.B); + capture(A["*/"] + "v", "v" + A["*/"], \`(\${A["*/"]})\`, A["*/"]); + capture(A); + `; + const fixture = /* js */ ` + const out = new Bun.Transpiler({ loader: "ts", target: "bun" }).transformSync(${JSON.stringify(src)}); + const captured = []; + new Function("capture", "x", out)((...args) => captured.push(args), "X"); + console.log(JSON.stringify({ out, captured })); + `; + await using proc = Bun.spawn({ + cmd: [bunExe(), "-e", fixture], + env: bunEnv, + stdout: "pipe", + stderr: "pipe", + }); + const [stdout, stderr, exitCode] = await Promise.all([proc.stdout.text(), proc.stderr.text(), proc.exited]); + expect({ stdout: stdout.trim(), stderr, exitCode }).toEqual({ + stdout: expect.stringMatching(/^\{"out":/), + stderr: "", + exitCode: 0, + }); + const { out, captured } = JSON.parse(stdout) as { out: string; captured: unknown[][] }; + + // The member references must actually have been inlined and folded for the + // values below to say anything about rope sharing. + const captureLines = out.split("\n").filter(l => l.startsWith("capture(") && l !== "capture(A);"); + expect(captureLines).toHaveLength(3); + for (const line of captureLines) expect(line).not.toMatch(/\bA\b/); + + expect(captured).toEqual([ + ["12", "12x", "x12", "1212", "<12>12!", "t12t", "st", "stu"], + ["12y", "y12", "1212", "aXb12c", "12bXa", "(12)", "12"], + ["stv", "vst", "(st)", "st"], + [{ B: "12", L: "12x", R: "x12", LR: "1212", M: "<12>12!", T: "t12t", "*/": "st", S2: "stu" }], + ]); +});