fix(hir): a zero-arg call to a global builtin is runtime semantics, not a compile error (#6366)

[proggeramlug]

Part 1 of #6366.

Problem

globals.rs rejects a zero-argument call to several JS globals at compile time:

$ perry compile a.js
Error: isNaN requires one argument

A missing argument isn't an error in JS — the parameter is simply undefined, and each of these has well-defined behavior for it. Perry was refusing to compile legal JS. Verified against Node:

call	Node	perry (before)
isNaN()	true	compile error
isFinite()	false	compile error
encodeURI()	"undefined"	compile error
decodeURI()	"undefined"	compile error
encodeURIComponent()	"undefined"	compile error
decodeURIComponent()	"undefined"	compile error

Fix

Lower the omitted argument to Expr::Undefined and let the existing intrinsic produce Node's answer — the padding idiom globals.rs already uses for parseInt / parseFloat / BigInt / Object. Output is byte-for-byte identical to Node.

Deliberately NOT included: atob / btoa / structuredClone

These are WebIDL required-argument throws and cannot be modelled by undefined-padding, because f() and f(undefined) genuinely differ:

atob()            // TypeError
atob(undefined)   // InvalidCharacterError
btoa()            // TypeError
btoa(undefined)   // "dW5kZWZpbmVk"   <-- doesn't even throw

The runtime can't tell them apart today (global thunks have fixed arity and no arg count; Expr::Atob(Box<Expr>) has no room for "omitted"). Distinguishing them needs real arity plumbing — analysed in #6366 and left as follow-up. They keep the current (incorrect) diagnostic rather than silently returning a wrong value.

Test change

unshadowed_global_isfinite_zero_arg_call_keeps_builtin_arity_error asserted the old diagnostic, using it as a proxy for "the unshadowed global took the BUILTIN path". That intent is preserved and now asserted directly: the call must lower to the IsFinite intrinsic with an Expr::Undefined argument. Renamed to ..._lowers_to_builtin_with_undefined. Its shadowed sibling is untouched and still passes.

Found by

Running Bun's own test suite through perry — test/js/web/util/atob.test.js dies at the compile tier on try { atob(); } catch (e) { ... }, a legal catchable runtime TypeError.

Testing

• test-files/test_gap_6366_global_zero_arg_coercion.ts — passes the parity harness byte-for-byte vs node --experimental-strip-types (Parity Pass: 1).
• global_builtin_shadowing 2/2, node_named_export_hygiene 9/9.
• All lint gates green (fmt / file-size / gc-store / addr-class).

Summary by CodeRabbit

• Bug Fixes
  • JavaScript global functions now correctly treat omitted arguments as undefined for isNaN, isFinite, and URI encoding/decoding functions.
  • Zero-argument calls now compile and behave consistently with explicit undefined arguments.
  • Existing one-argument behavior remains unchanged.

[coderabbitai]

📝 Walkthrough

Walkthrough

Global numeric and URI built-ins now treat omitted arguments as undefined during lowering. HIR coverage verifies the intrinsic representation, and a runtime test covers zero-argument, explicit-undefined, and existing one-argument calls.

Changes

Global built-in argument handling

Layer / File(s)	Summary
Lower omitted global arguments crates/perry-hir/src/lower/expr_call/globals.rs	Adds shared argument padding and applies it to isNaN, isFinite, and URI global built-ins.
Validate zero-argument behavior crates/perry-hir/tests/global_builtin_shadowing.rs, test-files/test_gap_6366_global_zero_arg_coercion.ts	Updates HIR assertions and adds runtime coverage for omitted, explicit-undefined, and one-argument calls.

Estimated code review effort: 2 (Simple) | ~10 minutes

Possibly related issues

• hir: zero-arg calls to global builtins are a COMPILE error instead of JS's runtime semantics #6366 — Directly tracks lowering omitted arguments as undefined for the six affected JavaScript globals.

🚥 Pre-merge checks | ✅ 5

✅ Passed checks (5 passed)

Check name	Status	Explanation
Title check	✅ Passed	The title matches the main change: zero-arg global builtins now follow runtime semantics instead of compile-time errors.
Description check	✅ Passed	It covers the problem, fix, related issue, and testing details, though it doesn't follow the template's exact section headings.
Docstring Coverage	✅ Passed	No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Linked Issues check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check	✅ Passed	Check skipped because no linked issues were found for this pull request.

✨ Finishing Touches

🧪 Generate unit tests (beta)

• Create PR with unit tests

---

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands.}

[coderabbitai]

🧹 Nitpick comments (1)

crates/perry-hir/src/lower/expr_call/globals.rs (1)

954-963: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low value

Consider applying arg_or_undefined to existing inline patterns for DRY consistency.

The new helper is clean and well-documented. Several existing arms in this function (parseInt string arg at L31-35, parseFloat at L47-51, BigInt at L62-70, Object at L104-108, queueMicrotask at L205-209) use the same if args.is_empty() { Expr::Undefined } else { args.remove(0) } inline pattern. Applying arg_or_undefined to those sites would reduce duplication and centralize the padding behavior. This is purely optional and can be deferred.

♻️ Example refactor for parseFloat (representative)

             "parseFloat" => {
-                if !args.is_empty() {
-                    return Ok(Ok(Expr::ParseFloat(Box::new(args.remove(0)))));
-                } else {
-                    return Ok(Ok(Expr::ParseFloat(Box::new(Expr::Undefined))));
-                }
+                let arg = arg_or_undefined(&mut args);
+                return Ok(Ok(Expr::ParseFloat(Box::new(arg))));
             }

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro Plus

Run ID: 70ea600c-ec3e-4449-b7dd-af4a389cd8b3

📥 Commits

Reviewing files that changed from the base of the PR and between 42cf924 and 1a4c098.

📒 Files selected for processing (3)

• crates/perry-hir/src/lower/expr_call/globals.rs
• crates/perry-hir/tests/global_builtin_shadowing.rs
• test-files/test_gap_6366_global_zero_arg_coercion.ts

[proggeramlug]

conformance-smoke (1) is the known flaky shard — it cannot be caused by this diff.

The shard fails on five tests:

test	triaged in known_failures.json?
test_gap_iterator_helpers_2874	✅ known
test_gap_module_const_local_shadow	✅ known
test_gap_string_locale_2781_2845_2897	✅ known
test_gap_yieldstar_inherited_iterator_this	✅ known
test_gap_node_fs	untriaged

This PR changes exactly one thing: how a zero-argument call to isNaN / isFinite / encodeURI / decodeURI / encodeURIComponent / decodeURIComponent is lowered (previously a compile error, now Expr::Undefined padding). Grepping all five files for such a call:

test_gap_iterator_helpers_2874              zero-arg calls: 0
test_gap_module_const_local_shadow          zero-arg calls: 0
test_gap_node_fs                            zero-arg calls: 0
test_gap_string_locale_2781_2845_2897       zero-arg calls: 0
test_gap_yieldstar_inherited_iterator_this  zero-arg calls: 0

None of them contain one, so the changed lowering is a no-op for every one of them — the diff cannot affect these tests. (test_gap_string_locale_... in particular is the Node-version reference drift already seen on #6260 and #6362; it passes locally byte-for-byte against Node with this branch's binary.)

Per CLAUDE.md ("conformance-smoke shards are flaky — before believing a red shard, re-run it and A/B the named tests against a pristine main build; several are already in test-parity/known_failures.json"). I tried to re-run the failed job but the run isn't retriable yet; happy to re-trigger, or this is a candidate for the same admin-merge path as #6238/#6244.

The targeted gates for this change are all green: test_gap_6366_global_zero_arg_coercion passes the parity harness byte-for-byte vs Node, global_builtin_shadowing 2/2, and all lint gates (fmt / file-size / gc-store / addr-class).