Skip to content
Closed
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
3 changes: 3 additions & 0 deletions changelog.d/10996-string-add-default-coercion.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
### Fixed

- Folded string `+` chains now use default-hint primitive conversion in operator order, so object `valueOf` methods and `Symbol.toPrimitive` match pairwise addition ([#10996](https://github.com/PerryTS/perry/pull/10996)).
61 changes: 61 additions & 0 deletions crates/perry-codegen/src/lower_string_concat.rs
Original file line number Diff line number Diff line change
Expand Up @@ -817,15 +817,76 @@ fn chain_part_without_redundant_coerce<'a>(ctx: &FnCtx<'_>, part: &'a Expr) -> &
}
}

/// A template substitution already performs ToString (string hint) while it
/// is evaluated. An ordinary `+` operand instead needs ToPrimitive(default),
/// unless its runtime value is already known to be primitive.
fn chain_part_needs_default_primitive(ctx: &FnCtx<'_>, part: &Expr) -> bool {
!matches!(part, Expr::StringCoerce(_))
&& !crate::type_analysis::string_value_is_runtime_guaranteed(ctx, part)
&& !crate::expr::expr_produces_non_pointer_bits_by_construction(ctx, part)
}

pub(crate) fn lower_string_concat_chain(ctx: &mut FnCtx<'_>, parts: &[&Expr]) -> Result<String> {
debug_assert!(parts.len() >= 2);
debug_assert!(parts.len() <= CONCAT_CHAIN_MAX_PARTS);
let needs_primitive: Vec<bool> = parts
.iter()
.map(|part| chain_part_needs_default_primitive(ctx, part))
.collect();
let parts: Vec<&Expr> = parts
.iter()
.map(|part| chain_part_without_redundant_coerce(ctx, part))
.collect();
let parts = parts.as_slice();

if needs_primitive.iter().any(|needed| *needed) {
return with_rooted_group(ctx, parts.len() + 2, |ctx, group| {
// For the first Add, both operands are evaluated before either is
// coerced. Later parts are evaluated and coerced one at a time,
// before the next Add's right operand is evaluated.
let mut first = Vec::with_capacity(2);
for part in parts.iter().take(2) {
let raw = lower_expr(ctx, part)?;
first.push(group.adopt_emitted(ctx, Repr::Boxed, &raw, true));
}

let mut converted = Vec::with_capacity(parts.len());
for i in 0..parts.len() {
let raw = if i < 2 {
group.reread_emitted(ctx, first[i])
} else {
lower_expr(ctx, parts[i])?
};
let value = if needs_primitive[i] {
ctx.block()
.call(DOUBLE, "js_to_primitive_default_for_add", &[(DOUBLE, &raw)])
} else {
raw
};
converted.push(group.adopt_emitted(ctx, Repr::Boxed, &value, true));
// A Symbol throws only after both operands of this Add have
// been converted. Do that check after the head pair, then
// after each later right operand and before the next one.
if i == 1 {
for part in converted.iter().take(2) {
let value = group.reread_emitted(ctx, *part);
ctx.block()
.call_void("js_add_throw_if_symbol", &[(DOUBLE, &value)]);
}
} else if i > 1 {
let value = group.reread_emitted(ctx, converted[i]);
ctx.block()
.call_void("js_add_throw_if_symbol", &[(DOUBLE, &value)]);
}
}
let lowered: Vec<String> = converted
.into_iter()
.map(|part| group.reread_emitted(ctx, part))
.collect();
Ok(emit_string_concat_chain(ctx, &lowered))
});
}

// Lower each part first (in source order); side effects must fire
// left-to-right per JS spec. #6951: that ordering is exactly what makes
// every earlier part a heap value in an SSA register across every later
Expand Down
2 changes: 2 additions & 0 deletions crates/perry-codegen/src/runtime_decls/strings.rs
Original file line number Diff line number Diff line change
Expand Up @@ -59,6 +59,8 @@ pub fn declare_phase_b_strings(module: &mut LlModule) {
// second arg is the count. Returns a raw string handle.
// (`crates/perry-runtime/src/string.rs::js_string_concat_chain`)
module.declare_function("js_string_concat_chain", I64, &[I64, I32]);
module.declare_function("js_to_primitive_default_for_add", DOUBLE, &[DOUBLE]);
module.declare_function("js_add_throw_if_symbol", VOID, &[DOUBLE]);
// Self-append variant of the N-way chain. The first part is the binding's
// current owner value; the runtime may extend it in place when unique and
// otherwise writes the complete result in one allocation.
Expand Down
16 changes: 16 additions & 0 deletions crates/perry-runtime/src/value/dynamic_arith.rs
Original file line number Diff line number Diff line change
Expand Up @@ -333,6 +333,22 @@ unsafe fn to_primitive_default_for_add(value: f64) -> f64 {
}
}

/// The N-way `+` concat fold needs the same default-hint conversion as the
/// pairwise dynamic-add path, before it hands each part to the string builder.
#[no_mangle]
pub unsafe extern "C" fn js_to_primitive_default_for_add(value: f64) -> f64 {
let scope = crate::gc::RuntimeHandleScope::new();
let rooted = scope.root_nanbox_f64(value);
to_primitive_default_for_add(rooted.get_nanbox_f64())
}

#[no_mangle]
pub unsafe extern "C" fn js_add_throw_if_symbol(value: f64) {
if is_symbol_value(value) {
throw_add_type_error(b"Cannot convert a Symbol value to a string");
}
}

type BigIntBinaryOp = extern "C" fn(
*const crate::bigint::BigIntHeader,
*const crate::bigint::BigIntHeader,
Expand Down
8 changes: 4 additions & 4 deletions crates/perry-runtime/src/value/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -112,10 +112,10 @@ pub use nanbox::{

// ----- Dynamic arithmetic dispatch (BigInt vs float) -----
pub use dynamic_arith::{
js_dynamic_add, js_dynamic_bitand, js_dynamic_bitor, js_dynamic_bitxor, js_dynamic_div,
js_dynamic_mod, js_dynamic_mul, js_dynamic_neg, js_dynamic_pos, js_dynamic_pow, js_dynamic_shl,
js_dynamic_shr, js_dynamic_string_or_number_add, js_dynamic_sub, js_dynamic_ushr,
js_numeric_step, js_to_numeric,
js_add_throw_if_symbol, js_dynamic_add, js_dynamic_bitand, js_dynamic_bitor, js_dynamic_bitxor,
js_dynamic_div, js_dynamic_mod, js_dynamic_mul, js_dynamic_neg, js_dynamic_pos, js_dynamic_pow,
js_dynamic_shl, js_dynamic_shr, js_dynamic_string_or_number_add, js_dynamic_sub,
js_dynamic_ushr, js_numeric_step, js_to_numeric, js_to_primitive_default_for_add,
};

// ----- Dynamic index get/set + bare-NaN check -----
Expand Down
50 changes: 50 additions & 0 deletions test-files/test_issue_10775_string_add_default_hint.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,50 @@
// A folded + chain must use ToPrimitive(default), then ToString, at each Add.
// Template substitutions instead use ToString (the "string" hint).
const valueOfOnly: any = { valueOf: () => 5 };
console.log("a" + valueOfOnly + "b");

const both: any = { valueOf: () => 1, toString: () => "T" };
console.log("a" + both + "b");

const hints: string[] = [];
const exotic: any = {
[Symbol.toPrimitive](hint: string) {
hints.push(hint);
return "P";
},
};
console.log("a" + exotic + "b", hints.join(","));
console.log(`a${exotic}b`, hints.join(","));

const order: string[] = [];
function tracked(label: string): any {
order.push("eval-" + label);
return {
[Symbol.toPrimitive](hint: string) {
order.push("coerce-" + label + "-" + hint);
return label;
},
};
}
console.log(tracked("A") + ":" + tracked("B") + ":");
console.log(order.join(","));

// The left operand's coercion may mutate an object already evaluated as the
// right operand of that first Add. Both values must be kept alive in order.
const right: any = { valueOf: () => 2 };
const left: any = {
valueOf() {
right.valueOf = () => 3;
return 1;
},
};
console.log(left + ":" + right + "x");

// An object may return a Symbol from ToPrimitive. The + operator throws;
// explicit String() and template substitutions have different rules.
const symbolObject: any = { [Symbol.toPrimitive]: () => Symbol("s") };
try {
console.log("a" + symbolObject + "b");
} catch (error) {
console.log(error instanceof TypeError);
}
Loading