Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion src/ast/e.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
132 changes: 11 additions & 121 deletions src/ast/fold_string_addition.rs
Original file line number Diff line number Diff line change
@@ -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<E::EString> {
data::Store::append(s)
}

/// Link `other` onto `lhs`'s rope tail.
fn estring_push(lhs: &mut E::EString, mut other: StoreRef<E::EString>) {
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<E::EString> = 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
}
Expand Down Expand Up @@ -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,
));
}
Expand All @@ -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,
Expand Down Expand Up @@ -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);
}
Expand All @@ -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;

Expand All @@ -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);
Expand Down
7 changes: 7 additions & 0 deletions src/js_parser/p.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down
5 changes: 4 additions & 1 deletion src/js_parser/visit/visit_stmt.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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_);

Expand Down
3 changes: 2 additions & 1 deletion src/jsc/RuntimeTranspilerCache.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
43 changes: 43 additions & 0 deletions test/bundler/bundler_edgecase.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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": `
Expand Down
79 changes: 79 additions & 0 deletions test/bundler/transpiler/transpiler.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -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", () => {
Expand Down
Loading
Loading