Skip to content

fix(hir): parenthesized / cast assignment target must honour const immutability (#6300) #6308

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

Merged
proggeramlug merged 1 commit into from
Jul 13, 2026
+334 −127
Merged

fix(hir): parenthesized / cast assignment target must honour const immutability (#6300) #6308

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
162 changes: 93 additions & 69 deletions crates/perry-hir/src/lower/expr_assign.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -70,7 +70,7 @@ fn lower_rhs_with_assignment_name(
result
}

fn throw_type_error_const_assignment(name: &str) -> Expr {
pub(crate) fn throw_type_error_const_assignment(name: &str) -> Expr {
Expr::Call {
callee: Box::new(Expr::ExternFuncRef {
name: "js_throw_type_error_const_assignment".to_string(),
Expand Down Expand Up @@ -413,81 +413,105 @@ pub(super) fn lower_assign(ctx: &mut LoweringContext, assign: &ast::AssignExpr)
lower_assignment_target(ctx, &assign.left, value)
}

/// Lower `<name> = value` where `<name>` is an identifier assignment target.
///
/// #6300: this is the ONE place identifier stores are resolved, so the
/// `const`-immutability (and class-inner-name) checks can't be routed around.
/// It is shared by the bare-`Ident` `AssignTarget` arm below and by
/// `lower_expr_assignment`'s `Ident` arm, which is what the parenthesized /
/// TS-cast targets (`(c) = 9`, `(c as any) = 9`, `(c satisfies T) = 9`,
/// `(c!) = 9`) unwrap into. Before the extraction, only the bare-`Ident` arm
/// checked immutability, so any wrapper on the LHS silently mutated a `const`.
pub(crate) fn lower_ident_assignment(
ctx: &mut LoweringContext,
name: String,
value: Box<Expr>,
) -> Result<Expr> {
if let Some(env_id) = ctx.active_with_envs_for_ident(&name).into_iter().next() {
let fallback = with_set_fallback_for_ident(ctx, &name);
return Ok(Expr::WithSet {
object: Box::new(Expr::LocalGet(env_id)),
property: name,
value,
fallback,
strict: ctx.current_strict,
});
}
if let Some(id) = ctx.lookup_local(&name) {
if ctx.is_local_immutable(id) {
// `const c = 1; c = 9` (and every wrapped spelling of the same
// target) evaluates the RHS for side effects, then throws
// `TypeError: Assignment to constant variable.`
return Ok(Expr::Sequence(vec![
*value,
throw_type_error_const_assignment(&name),
]));
}
Ok(Expr::LocalSet(id, value))
} else if ctx.current_class_inner_name.as_deref() == Some(name.as_str()) {
// Assigning to the class own-name binding from inside the class
// body targets the immutable inner `const` binding -> TypeError
// (test262 language/statements/class/name-binding/const). Evaluate
// the RHS for side effects first, then throw. A local/param that
// shadows the name was already handled by the `lookup_local` arm
// above, so this only fires for the genuine class binding.
Ok(Expr::Sequence(vec![
*value,
throw_type_error_const_assignment(&name),
]))
} else if ctx.lookup_class(&name).is_some() || ctx.lookup_func(&name).is_some() {
// v0.5.757: don't shadow a class/function binding with an
// implicit local for `<Name> = X` patterns. Drizzle's
// sql.js uses `((sql2) => { ... })(sql || (sql = {}))`
// (and the same for SQL) — since the binding exists
// (truthy), the OR short-circuits and the assignment is
// dead. Pre-fix the implicit local hid the original
// binding from later reads. Just evaluate the RHS for
// side effects. Refs #420.
Ok(*value)
} else {
if ctx.current_strict {
// #5989: strict-mode assignment to an existing global
// builtin is a property write, not a ReferenceError. See
// `strict_global_assign_existing_or_throw` for the full
// rationale.
return Ok(strict_global_assign_existing_or_throw(name, value));
}
eprintln!(
" Warning: Assignment to undeclared variable '{}', creating sloppy global",
name
);
// Sloppy implicit global: the binding IS a property of globalThis
// (spec CreateGlobalVarBinding on the global object), so `foo = 1`
// must be visible as `globalThis.foo`, write through to a
// pre-existing global property, and observe a later
// `delete globalThis.foo`. Reads of the name resolve through the
// `js_global_get_or_throw_unresolved` fallback, so no module-local
// shadow may be created here (a stale local would keep serving
// deleted/overwritten values).
// NOTE: `GlobalGet(0)` alone is a by-name routing SENTINEL in
// codegen (bare reads lower to 0.0) — the write must target
// the VALUE globalThis, which the `PropertyGet { GlobalGet(0),
// "globalThis" }` shape resolves to the real global object.
Ok(Expr::PropertySet {
object: Box::new(Expr::PropertyGet {
object: Box::new(Expr::GlobalGet(0)),
property: "globalThis".to_string(),
}),
property: name,
value,
})
}
}

fn lower_assignment_target(
ctx: &mut LoweringContext,
target: &ast::AssignTarget,
value: Box<Expr>,
) -> Result<Expr> {
match target {
ast::AssignTarget::Simple(ast::SimpleAssignTarget::Ident(ident)) => {
let name = ident.id.sym.to_string();
if let Some(env_id) = ctx.active_with_envs_for_ident(&name).into_iter().next() {
let fallback = with_set_fallback_for_ident(ctx, &name);
return Ok(Expr::WithSet {
object: Box::new(Expr::LocalGet(env_id)),
property: name,
value,
fallback,
strict: ctx.current_strict,
});
}
if let Some(id) = ctx.lookup_local(&name) {
if ctx.is_local_immutable(id) {
return Ok(Expr::Sequence(vec![
*value,
throw_type_error_const_assignment(&name),
]));
}
Ok(Expr::LocalSet(id, value))
} else if ctx.current_class_inner_name.as_deref() == Some(name.as_str()) {
// Assigning to the class own-name binding from inside the class
// body targets the immutable inner `const` binding -> TypeError
// (test262 language/statements/class/name-binding/const). Evaluate
// the RHS for side effects first, then throw. A local/param that
// shadows the name was already handled by the `lookup_local` arm
// above, so this only fires for the genuine class binding.
Ok(Expr::Sequence(vec![
*value,
throw_type_error_const_assignment(&name),
]))
} else if ctx.lookup_class(&name).is_some() || ctx.lookup_func(&name).is_some() {
// v0.5.757: don't shadow a class/function binding with an
// implicit local for `<Name> = X` patterns. Drizzle's
// sql.js uses `((sql2) => { ... })(sql || (sql = {}))`
// (and the same for SQL) — since the binding exists
// (truthy), the OR short-circuits and the assignment is
// dead. Pre-fix the implicit local hid the original
// binding from later reads. Just evaluate the RHS for
// side effects. Refs #420.
Ok(*value)
} else {
if ctx.current_strict {
// #5989: strict-mode assignment to an existing global
// builtin is a property write, not a ReferenceError. See
// `strict_global_assign_existing_or_throw` for the full
// rationale (shared with the sibling arm in
// lower_expr/assignment.rs).
return Ok(strict_global_assign_existing_or_throw(name, value));
}
eprintln!(
" Warning: Assignment to undeclared variable '{}', creating sloppy global",
name
);
// Sloppy implicit global — a real globalThis property, not
// a module local (see the sibling arm in lower_expr.rs).
// NOTE: `GlobalGet(0)` alone is a by-name routing SENTINEL in
// codegen (bare reads lower to 0.0) — the write must target
// the VALUE globalThis, which the `PropertyGet { GlobalGet(0),
// "globalThis" }` shape resolves to the real global object.
Ok(Expr::PropertySet {
object: Box::new(Expr::PropertyGet {
object: Box::new(Expr::GlobalGet(0)),
property: "globalThis".to_string(),
}),
property: name,
value,
})
}
lower_ident_assignment(ctx, ident.id.sym.to_string(), value)
}
ast::AssignTarget::Simple(ast::SimpleAssignTarget::Member(member)) => {
// Proxy set: `proxy.foo = v` / `proxy[k] = v`
Expand Down
41 changes: 39 additions & 2 deletions crates/perry-hir/src/lower/expr_misc.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,7 @@ use anyhow::{anyhow, Result};
use swc_ecma_ast as ast;

use crate::ir::{BinaryOp, Expr, UpdateOp};
use crate::lower::expr_assign::throw_type_error_const_assignment;
use crate::lower_patterns::unescape_template;

use super::{lower_expr, LoweringContext};
Expand Down Expand Up @@ -130,13 +131,19 @@ pub(super) fn lower_update(ctx: &mut LoweringContext, update: &ast::UpdateExpr)
ast::UpdateOp::MinusMinus => BinaryOp::Sub,
};

// Unwrap compile-time-only wrappers: `obj.x!++` (TS non-null assertion) and
// `(obj.x)++` (parenthesized) are transparent to update-expression lowering.
// Unwrap compile-time-only wrappers: `obj.x!++` (TS non-null assertion),
// `(obj.x)++` (parenthesized) and the TS casts (`(x as any)++`,
// `(<any>x)++`, `(x satisfies number)++`) are all transparent to
// update-expression lowering (#6300 — the cast forms previously fell
// through to the "only identifiers and member expressions" error).
let mut arg = update.arg.as_ref();
loop {
match arg {
ast::Expr::TsNonNull(inner) => arg = inner.expr.as_ref(),
ast::Expr::Paren(inner) => arg = inner.expr.as_ref(),
ast::Expr::TsAs(inner) => arg = inner.expr.as_ref(),
ast::Expr::TsTypeAssertion(inner) => arg = inner.expr.as_ref(),
ast::Expr::TsSatisfies(inner) => arg = inner.expr.as_ref(),
_ => break,
}
}
Expand Down Expand Up @@ -174,6 +181,36 @@ pub(super) fn lower_update(ctx: &mut LoweringContext, update: &ast::UpdateExpr)
byte_offset: 0,
});
};
if ctx.is_local_immutable(id) {
// #6300: `const c = 1; c++` (and `(c as any)--`, `(c!)++`, …) is
// a PutValue on an immutable binding →
// `TypeError: Assignment to constant variable.` Emitting
// `Expr::Update` here silently mutated the `const` instead.
//
// The throw is NOT unconditional-first: per ES `++`/`--`
// (13.4.2.1 / 13.4.3.1) the operand is run through
// `ToNumeric(GetValue(lhs))` *before* `PutValue` rejects the
// immutable binding, and that coercion is itself observable —
// `const s = Symbol(); s++` throws "Cannot convert a Symbol
// value to a number", not the const TypeError (a BigInt, by
// contrast, passes ToNumeric cleanly and does get the const
// TypeError). Sequence the same `js_to_numeric` the normal
// update path uses in front of the throw so both orders match
// V8.
return Ok(Expr::Sequence(vec![
Expr::Call {
callee: Box::new(Expr::ExternFuncRef {
name: "js_to_numeric".to_string(),
param_types: vec![perry_types::Type::Any],
return_type: perry_types::Type::Any,
}),
args: vec![Expr::LocalGet(id)],
type_args: vec![],
byte_offset: 0,
},
throw_type_error_const_assignment(&name),
]));
}
let op = match update.op {
ast::UpdateOp::PlusPlus => UpdateOp::Increment,
ast::UpdateOp::MinusMinus => UpdateOp::Decrement,
Expand Down
61 changes: 5 additions & 56 deletions crates/perry-hir/src/lower/lower_expr/assignment.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -17,63 +17,12 @@ pub(crate) fn lower_expr_assignment(
value: Box<Expr>,
) -> Result<Expr> {
match expr {
// #6300: identifier stores — including the ones reached by unwrapping a
// parenthesized / TS-cast assignment target below — go through the one
// shared helper, so the `const`-immutability check can't be bypassed by
// writing `(c as any) = 9` instead of `c = 9`.
ast::Expr::Ident(ident) => {
let name = ident.sym.to_string();
if let Some(env_id) = ctx.active_with_envs_for_ident(&name).into_iter().next() {
let fallback = with_set_fallback_for_ident(ctx, &name);
return Ok(Expr::WithSet {
object: Box::new(Expr::LocalGet(env_id)),
property: name,
value,
fallback,
strict: ctx.current_strict,
});
}
if let Some(id) = ctx.lookup_local(&name) {
Ok(Expr::LocalSet(id, value))
} else if ctx.lookup_class(&name).is_some() || ctx.lookup_func(&name).is_some() {
// v0.5.757: don't shadow a class/function binding with an
// implicit local for `<Name> = X` patterns. Drizzle's
// sql.js uses `((sql2) => { ... })(sql || (sql = {}))` —
// the binding exists (truthy), the OR short-circuits, and
// the assignment is dead. Pre-fix the implicit local hid
// the original binding from later reads. Just evaluate
// the RHS for side effects. Refs #420.
Ok(*value)
} else {
if ctx.current_strict {
// #5989: strict-mode assignment to an existing global
// builtin is a property write, not a ReferenceError. See
// `strict_global_assign_existing_or_throw` for the full
// rationale (shared with the sibling arm in expr_assign.rs).
return Ok(strict_global_assign_existing_or_throw(name, value));
}
eprintln!(
" Warning: Assignment to undeclared variable '{}', creating sloppy global",
name
);
// Sloppy implicit global: the binding IS a property of
// globalThis (spec CreateGlobalVarBinding on the global
// object), so `foo = 1` must be visible as
// `globalThis.foo`, write through to a pre-existing global
// property, and observe a later `delete globalThis.foo`.
// Reads of the name resolve through the
// `js_global_get_or_throw_unresolved` fallback, so no
// module-local shadow may be created here (a stale local
// would keep serving deleted/overwritten values).
// NOTE: `GlobalGet(0)` alone is a by-name routing SENTINEL in
// codegen (bare reads lower to 0.0) — the write must target
// the VALUE globalThis, which the `PropertyGet { GlobalGet(0),
// "globalThis" }` shape resolves to the real global object.
Ok(Expr::PropertySet {
object: Box::new(Expr::PropertyGet {
object: Box::new(Expr::GlobalGet(0)),
property: "globalThis".to_string(),
}),
property: name,
value,
})
}
crate::lower::expr_assign::lower_ident_assignment(ctx, ident.sym.to_string(), value)
}
ast::Expr::Member(member) => {
if let ast::Expr::Ident(obj_ident) = member.obj.as_ref() {
Expand Down
Loading
Loading