Skip to content

Commit e2eee40

Browse files
proggeramlugRalph Küpper
andauthored
fix(hir): share function and var declaration bindings (#8753)
Lands #8743. A same-named top-level `function` and `var` declaration created two separate bindings instead of the single hoisted one the spec requires. Module-var-scope names are now collected before top-level function declarations are lowered, and both forms seed one hoisted mutable binding, preserving source-position overwrites, inert bare `var`, nested `var`, and last-duplicate-function semantics. Test262 `language/reserved-words` goes 25/26 -> 26/26; `language/global-code` is unchanged at 26 pass / 2 runtime-fail. A changelog fragment was added; the PR had neither one nor a skip-changelog label. No version bump. Co-authored-by: Ralph Küpper <ralph@skelpo.com>
1 parent 9ec6250 commit e2eee40

3 files changed

Lines changed: 143 additions & 2 deletions

File tree

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1 @@
1+
Fixed a same-named top-level `function` and `var` declaration creating two separate bindings instead of one. Module-var-scope names are now collected before top-level function declarations are lowered, and both forms seed a single hoisted mutable binding, preserving source-position overwrites, inert bare `var`, nested `var`, and last-duplicate-function semantics. Takes Test262 `language/reserved-words` from 25/26 to 26/26.

crates/perry-hir/src/lower/lower_module_fn.rs

Lines changed: 44 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -539,6 +539,37 @@ fn collect_assigned_function_binding_candidates(ast_module: &ast::Module) -> Has
539539
out
540540
}
541541

542+
/// Names introduced by `var` declarations in the module's var scope.
543+
///
544+
/// A same-named top-level FunctionDeclaration and `var` declaration share one
545+
/// binding. Collect these before function lowering so the function's hoisted
546+
/// value can seed that binding instead of a later var pre-scan creating a
547+
/// separate, undefined local that shadows it. The statement walker deliberately
548+
/// descends through blocks/loops/try/switch but not nested functions or classes.
549+
fn collect_module_var_binding_names(ast_module: &ast::Module) -> HashSet<String> {
550+
let mut names = Vec::new();
551+
for item in &ast_module.body {
552+
match item {
553+
ast::ModuleItem::Stmt(stmt) => {
554+
crate::lower_decl::collect_var_binding_names_from_stmt(stmt, &mut names);
555+
}
556+
ast::ModuleItem::ModuleDecl(ast::ModuleDecl::ExportDecl(export_decl)) => {
557+
if let ast::Decl::Var(var_decl) = &export_decl.decl {
558+
if var_decl.kind == ast::VarDeclKind::Var {
559+
for decl in &var_decl.decls {
560+
crate::lower_decl::collect_var_binding_names_from_pat(
561+
&decl.name, &mut names,
562+
);
563+
}
564+
}
565+
}
566+
}
567+
_ => {}
568+
}
569+
}
570+
names.into_iter().collect()
571+
}
572+
542573
/// #5833: names assigned by a **direct top-level** `name = ...`/`name++`
543574
/// expression statement (module scope, not nested in any block/if/loop/
544575
/// function). Deliberately narrower than
@@ -932,6 +963,7 @@ pub fn lower_module_full(
932963
// Skip 'declare function' statements (functions with no body) - they are external FFI
933964
// BUT: also skip overload signatures if an implementation exists
934965
let reassigned_function_candidates = collect_assigned_function_binding_candidates(ast_module);
966+
let module_var_binding_names = collect_module_var_binding_names(ast_module);
935967
// #5833: a narrower, binding-aware-enough scan stashed on `ctx` so
936968
// `stmt.rs`'s top-level `Decl::Class` arm can gate its opt-in local-slot
937969
// seeding (see both `reassigned_top_level_identifiers`'s doc comment and
@@ -1008,11 +1040,21 @@ pub fn lower_module_full(
10081040
let func_id = ctx.fresh_func();
10091041
ctx.register_func(func_name.clone(), func_id);
10101042
if reassigned_function_candidates.contains(&func_name)
1011-
&& ctx.lookup_local(&func_name).is_none()
1043+
|| module_var_binding_names.contains(&func_name)
10121044
{
1013-
let local_id = ctx.define_local(func_name.clone(), Type::Any);
1045+
// FunctionDeclaration and `var` of the same name share one
1046+
// mutable binding. Seed it with every declaration in source
1047+
// order so duplicate function declarations leave the last
1048+
// function installed at entry, then let any source-position
1049+
// `var f = value` overwrite that same slot when it executes.
1050+
let local_id = ctx
1051+
.lookup_local(&func_name)
1052+
.unwrap_or_else(|| ctx.define_local(func_name.clone(), Type::Any));
10141053
ctx.record_local_source_span(local_id, fn_decl.ident.span);
10151054
ctx.function_valued_locals.insert(local_id);
1055+
if module_var_binding_names.contains(&func_name) {
1056+
ctx.var_hoisted_ids.insert(local_id);
1057+
}
10161058
module.init.push(Stmt::Let {
10171059
id: local_id,
10181060
name: func_name.clone(),
Lines changed: 98 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,98 @@
1+
//! Regression for #5894 / Test262 `language/reserved-words/unreserved-words.js`.
2+
//!
3+
//! A top-level FunctionDeclaration and a same-named `var` declaration share a
4+
//! single binding. The function value is installed during declaration
5+
//! instantiation; a bare `var f;` is inert, while `var f = value` overwrites it
6+
//! only when execution reaches the initializer. Perry used to pre-register the
7+
//! `var` as a separate undefined local, shadowing the function even before the
8+
//! declaration's source position.
9+
10+
use std::path::PathBuf;
11+
use std::process::Command;
12+
13+
fn perry_bin() -> PathBuf {
14+
PathBuf::from(env!("CARGO_BIN_EXE_perry"))
15+
}
16+
17+
#[test]
18+
fn function_and_var_declarations_share_the_hoisted_binding() {
19+
let dir = tempfile::tempdir().expect("tempdir");
20+
let entry = dir.path().join("main.ts");
21+
std::fs::write(
22+
&entry,
23+
r#"
24+
// The Test262 harness defines `assert` as a function, assigns helper
25+
// properties, and the test later declares `var assert = 1`.
26+
function assert() {}
27+
(assert as any)._isSameValue = "ready";
28+
console.log("helper-before-var:", (assert as any)._isSameValue);
29+
var assert = 1;
30+
console.log("helper-after-var:", assert);
31+
32+
// An uninitialised var redeclaration must not replace the hoisted function.
33+
console.log("bare-before:", bare());
34+
function bare() { return "bare-function"; }
35+
var bare;
36+
console.log("bare-after:", bare());
37+
38+
// A var nested in a block still belongs to the module var scope and shares
39+
// the function binding, even when its initializer never executes.
40+
console.log("nested-before:", nested());
41+
function nested() { return "nested-function"; }
42+
if (false) { var nested = 2; }
43+
console.log("nested-after:", nested());
44+
45+
// Duplicate function declarations install the last declaration at entry;
46+
// the same-named bare var remains inert.
47+
console.log("duplicate-before:", duplicate());
48+
function duplicate() { return "first"; }
49+
function duplicate() { return "second"; }
50+
var duplicate;
51+
console.log("duplicate-after:", duplicate());
52+
console.log("DONE");
53+
"#,
54+
)
55+
.expect("write entry");
56+
57+
let output = dir.path().join("main_bin");
58+
let compile = Command::new(perry_bin())
59+
.current_dir(dir.path())
60+
.arg("compile")
61+
.arg(&entry)
62+
.arg("-o")
63+
.arg(&output)
64+
.arg("--no-cache")
65+
.env("PERRY_NO_AUTO_OPTIMIZE", "1")
66+
.output()
67+
.expect("run perry compile");
68+
assert!(
69+
compile.status.success(),
70+
"perry compile failed\nstdout:\n{}\nstderr:\n{}",
71+
String::from_utf8_lossy(&compile.stdout),
72+
String::from_utf8_lossy(&compile.stderr)
73+
);
74+
75+
let run = Command::new(&output).output().expect("run compiled binary");
76+
let stdout = String::from_utf8_lossy(&run.stdout);
77+
assert!(
78+
run.status.success(),
79+
"compiled binary failed\nstdout:\n{stdout}\nstderr:\n{}",
80+
String::from_utf8_lossy(&run.stderr)
81+
);
82+
for expected in [
83+
"helper-before-var: ready",
84+
"helper-after-var: 1",
85+
"bare-before: bare-function",
86+
"bare-after: bare-function",
87+
"nested-before: nested-function",
88+
"nested-after: nested-function",
89+
"duplicate-before: second",
90+
"duplicate-after: second",
91+
"DONE",
92+
] {
93+
assert!(
94+
stdout.contains(expected),
95+
"missing {expected:?}\nstdout:\n{stdout}"
96+
);
97+
}
98+
}

0 commit comments

Comments
 (0)