Skip to content

js_parser: fold +, - and ~ of a constant-folded string; stop folding non-ascii strings to NaN #39072

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
robobun wants to merge 1 commit into
base: main
Choose a base branch
from
Open

js_parser: fold +, - and ~ of a constant-folded string; stop folding non-ascii strings to NaN #39072

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
39 changes: 16 additions & 23 deletions src/ast/expr.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2581,36 +2581,26 @@ impl Data {
Tag::typeof_(self.tag())
}

pub fn to_number(&self) -> Option<f64> {
/// The value of `+expr` when it is known at compile time. `bump` is used to
/// flatten a string that constant folding left as a rope.
pub fn to_number(&self, bump: &Bump) -> Option<f64> {
match self {
Data::ENull(_) => Some(0.0),
Data::EUndefined(_) => Some(f64::NAN),
Data::EString(str) => {
if str.next.is_some() {
return None;
}
// `StoreRef<EString>` is a Copy pointer; rebind mutably so
// `DerefMut` gives `&mut EString` for in-place rope flattening.
let mut str = *str;
str.resolve_rope_if_needed(bump);
if !str.is_utf8() {
return None;
}
// +'1' => 1
Some(string_to_equivalent_number_value(str.slice8()))
string_to_equivalent_number_value(str.slice8())
}
Data::EBoolean(b) | Data::EBranchBoolean(b) => Some(if b.value { 1.0 } else { 0.0 }),
Data::ENumber(n) => Some(n.value()),
Data::EInlinedEnum(inlined) => match &inlined.value.data {
Data::ENumber(num) => Some(num.value()),
Data::EString(str) => {
if str.next.is_some() {
return None;
}
if !str.is_utf8() {
return None;
}
// +'1' => 1
Some(string_to_equivalent_number_value(str.slice8()))
}
_ => None,
},
Data::EInlinedEnum(inlined) => inlined.value.data.to_number(bump),
_ => None,
}
}
Expand Down Expand Up @@ -2914,13 +2904,16 @@ pub use data::Store;
// Helpers
// ───────────────────────────────────────────────────────────────────────────

fn string_to_equivalent_number_value(str: &[u8]) -> f64 {
fn string_to_equivalent_number_value(str: &[u8]) -> Option<f64> {
// +"" -> 0
if str.is_empty() {
return 0.0;
return Some(0.0);
}
// `JSC__jsToNumber` reads latin1, so utf-8 input must be ascii. A non-ascii
// string is not necessarily NaN (+"\u00a01" is 1: U+00A0 is whitespace to
// StringToNumber), so it is left unfolded rather than folded to NaN.
if !bun_core::is_all_ascii(str) {
return f64::NAN;
return None;
}
unsafe extern "C" {
// NOT `safe fn`: callee dereferences `ptr` for `len` bytes — caller must
Expand All @@ -2930,5 +2923,5 @@ fn string_to_equivalent_number_value(str: &[u8]) -> f64 {
// SAFETY: `str` is a live `&[u8]`, so `as_ptr()` is non-null and readable for
// exactly `str.len()` bytes for the duration of this call; the C++ side reads
// only (no mutation, no retention past return).
unsafe { JSC__jsToNumber(str.as_ptr(), str.len()) }
Some(unsafe { JSC__jsToNumber(str.as_ptr(), str.len()) })
}
4 changes: 2 additions & 2 deletions src/js_parser/scan/scan_side_effects.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -95,8 +95,8 @@ impl SideEffects {

// Re-exports of ExprData methods.
#[inline(always)]
pub(crate) fn to_number(data: &ExprData) -> Option<f64> {
data.to_number()
pub(crate) fn to_number(data: &ExprData, bump: &Bump) -> Option<f64> {
data.to_number(bump)
}
#[inline(always)]
pub(crate) fn typeof_(data: &ExprData) -> Option<&'static [u8]> {
Expand Down
6 changes: 3 additions & 3 deletions src/js_parser/visit/visit_expr.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1271,7 +1271,7 @@ impl<'a, const TYPESCRIPT: bool, const SCAN_ONLY: bool> P<'a, TYPESCRIPT, SCAN_O
}
Op::UnCpl => {
if p.should_fold_typescript_constant_expressions {
if let Some(value) = SideEffects::to_number(&e_.value.data) {
if let Some(value) = SideEffects::to_number(&e_.value.data, p.arena) {
*e = p.new_expr(
E::Number::new(f64::from(!float_to_int32(value))),
expr.loc,
Expand All @@ -1287,13 +1287,13 @@ impl<'a, const TYPESCRIPT: bool, const SCAN_ONLY: bool> P<'a, TYPESCRIPT, SCAN_O
}
}
Op::UnPos => {
if let Some(num) = SideEffects::to_number(&e_.value.data) {
if let Some(num) = SideEffects::to_number(&e_.value.data, p.arena) {
*e = p.new_expr(E::Number::new(num), expr.loc);
return;
}
}
Op::UnNeg => {
if let Some(num) = SideEffects::to_number(&e_.value.data) {
if let Some(num) = SideEffects::to_number(&e_.value.data, p.arena) {
*e = p.new_expr(E::Number::new(-num), expr.loc);
return;
}
Expand Down
42 changes: 42 additions & 0 deletions test/bundler/bundler_minify.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -630,6 +630,48 @@ describe("bundler", () => {
'+"æ"',
],
});
// "1" + "2" is folded into a rope (the pieces linked together, flattened when
// printed); +, - and ~ have to read the whole rope.
itBundled("minify/ConstantFoldingUnaryOnFoldedString", {
files: {
"/entry.ts": `
capture(+("1" + "2"));
capture(-("1" + "2"));
capture(~("1" + "2"));
capture(+("1" + "2" + "3"));
capture(+("" + ""));
capture(-(" 0x" + "10 "));
capture(+("1" + y));
enum E { A = "1" + "2", B = +("3" + "4"), C = -A, D = ~A }
capture(+E.A);
capture(-E.A);
capture(E.B + E.C + E.D);
`,
},
minifySyntax: true,
capture: ["12", "-12", "-13", "123", "0", "-16", '+("1" + y)', "12", "-12", "9"],
});
// Strings from --define are stored as UTF-8. U+00A0 is whitespace to
// StringToNumber (+"\u00a01" is 1), so a non-ascii string is left alone
// instead of being folded to NaN.
itBundled("minify/ConstantFoldingUnaryPlusNonAsciiDefine", {
files: {
"/entry.js": `
capture(+NBSP_ONE);
capture(-NBSP_ONE);
capture(~NBSP_ONE);
capture(+(NBSP_ONE + ""));
capture(+SPACE_ONE);
capture(+(SPACE_ONE + ""));
`,
},
define: {
NBSP_ONE: JSON.stringify("\u00a01"),
SPACE_ONE: JSON.stringify(" 1 "),
},
minifySyntax: true,
capture: ['+"\u00a01"', '-"\u00a01"', '~"\u00a01"', '+"\u00a01"', "1", "1"],
});
itBundled("minify/ImportMetaHotTreeShaking", {
files: {
"/entry.ts": `
Expand Down
72 changes: 72 additions & 0 deletions test/bundler/transpiler/transpiler.test.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3563,6 +3563,78 @@ class Foo {
expectPrinted("a = !(b, c)", "a = (b, !c)");
});

it("+, - and ~ fold a string that was itself constant folded", () => {
// "1" + "2" is folded into a rope (the pieces linked together, flattened
// when printed), not into one flat string literal.
// Asserts the output, and that transpiling the output again changes nothing.
const expectFixedPoint = (code, out) => {
const once = parsed(code, false);
expect(once).toBe(out);
expect(parsed(once, false)).toBe(out);
};

expectFixedPoint('x = +("1" + "2");', "x = 12;\n");
expectFixedPoint('x = -("1" + "2");', "x = -12;\n");
expectFixedPoint('x = ~("1" + "2");', "x = -13;\n");
expectFixedPoint('x = +("1" + "2" + "3");', "x = 123;\n");
expectFixedPoint('x = +("1" + ("2" + "3"));', "x = 123;\n");
expectFixedPoint('x = +("1" + `2`);', "x = 12;\n");
expectFixedPoint('x = +("1" + 2);', "x = 12;\n");
expectFixedPoint('x = +("" + "");', "x = 0;\n");
expectFixedPoint('x = +("1" + "e2");', "x = 100;\n");
expectFixedPoint('x = -(" 0x" + "10 ");', "x = -16;\n");
expectFixedPoint('x = -("1" + "2") * 2;', "x = -24;\n");
expectFixedPoint('x = +("1" + "2") === 12;', "x = !0;\n");

// Only a constant operand folds.
expectFixedPoint('x = +("1" + y);', 'x = +("1" + y);\n');
expectFixedPoint('x = -(y + "1");', 'x = -(y + "1");\n');
});

it("+, - and ~ fold an enum member whose value was constant folded", () => {
// Enum initializers are always folded, so A is stored as a rope and each
// use of A is inlined as that rope. This does not depend on minify.
const ts = new Bun.Transpiler({ loader: "ts" });
expect(ts.transformSync('enum E { A = "1" + "2", B = +("3" + "4"), C = -A, D = ~A }')).toBe(
'var E;\n((E) => {\n E["A"] = "12";\n E[E["B"] = 34] = "B";\n E[E["C"] = -12] = "C";\n E[E["D"] = -13] = "D";\n})(E ||= {});\n',
);
expect(ts.transformSync('const enum E { A = "1" + "2" }\nx = +E.A;\ny = -E.A;\n')).toBe(
'var E;\n((E) => {\n E["A"] = "12";\n})(E ||= {});\nx = 12;\ny = -12;\n',
);
});

it("+, - and ~ do not fold a string containing non-ascii characters", () => {
// Source literals with non-ascii characters are stored as UTF-16 and never
// folded; strings from `define` are stored as UTF-8 and used to fold to NaN.
// U+00A0 is whitespace to StringToNumber: +"\u00a01" is 1 and +"\u00a0" is 0.
const defined = new Bun.Transpiler({
loader: "js",
minify: { syntax: true },
define: {
NBSP: JSON.stringify("\u00a0"),
NBSP_ONE: JSON.stringify("\u00a01"),
SPACE_ONE: JSON.stringify(" 1 "),
},
});
const expectPrintedDefined = (code, out) => {
const once = defined.transformSync(code);
expect(once).toBe(out);
expect(defined.transformSync(once)).toBe(out);
};

expectPrintedDefined("x = +NBSP_ONE;", 'x = +"\u00a01";\n');
expectPrintedDefined("x = -NBSP_ONE;", 'x = -"\u00a01";\n');
expectPrintedDefined("x = ~NBSP_ONE;", 'x = ~"\u00a01";\n');
expectPrintedDefined("x = +NBSP;", 'x = +"\u00a0";\n');
expectPrintedDefined('x = +(NBSP + "1");', 'x = +"\u00a01";\n');
expectPrintedDefined('x = +("1" + NBSP);', 'x = +"1\u00a0";\n');

// An ascii string from `define` folds, on its own and as part of a rope.
expectPrintedDefined("x = +SPACE_ONE;", "x = 1;\n");
expectPrintedDefined('x = +(SPACE_ONE + "");', "x = 1;\n");
expectPrintedDefined('x = ~(SPACE_ONE + "");', "x = -2;\n");
});

it.todo("const inlining", () => {
var transpiler = new Bun.Transpiler({
inline: true,
Expand Down
Loading