fix(codegen): stop typing a symbol-keyed element read as a number (#7796)

[jdalton]

Closes #7796.

A symbol-keyed property read off an array produced a value that every test agreed was a function — typeof said "function", Boolean() said true, === undefined said false — but that a plain if treated as falsy. Only when the value was stored in a local first, which is what made it look so strange.

const a = [1];
console.log((a as any)[Symbol.iterator] ? "truthy" : "FALSY");  // truthy  ✅
const f = (a as any)[Symbol.iterator];
console.log(f ? "truthy" : "FALSY");                            // FALSY   ❌ (node: truthy)

Why it happened

Perry types an element read of a number[] as a number, and it was doing that regardless of what the index actually was. Two places do this inference — refine_type_from_init (which decides the type of const f = ...) and is_numeric_expr (which answers "is this expression a number?") — and both destructured the index and then ignored it.

But a[Symbol.iterator] is not an element read. It reads a property of the array object, and the answer is Array.prototype[Symbol.iterator], a function. So the local holding it was recorded as a number.

That is where it turns from imprecise into wrong. A value believed to be a number gets its truthiness tested with a floating-point comparison:

%r75 = fcmp one double %r74, 0.0

Perry stores objects, strings and functions as NaN-boxed doubles, which really are NaN — and every comparison against a NaN is false. So the test answered "falsy" for every function, object and string that reached it. The inline form was correct only because it never went through a local, so the bad type was never recorded.

The fix, and why it asks for proof

Both inference sites now require the index to be provably numeric before taking the array's element type.

Requiring proof, rather than just checking for a known-bad index like a symbol, is deliberate. An index the compiler cannot type may hold anything at runtime, so "I have no evidence this is a symbol" is not evidence that it is a number. The two possible mistakes are not equal either: answering "not a number" costs one missed fast path, while answering "number" costs a branch that silently goes the wrong way.

The masked-window fast-copy case keeps its early exit, since that fact already proves the index is an integer.

Tests

Two unit tests in crates/perry-codegen/src/type_analysis/numeric/tests.rs, both asserting on the emitted IR for the function under test rather than the whole module:

Test	Asserts
a_symbol_indexed_element_is_tested_with_js_is_truthy	The symbol-keyed read reaches the general truthiness helper and does not get the fcmp one fast path
a_numeric_index_keeps_the_inline_fast_path	The guard against over-correcting: ordinary a[i] still gets its inline comparison

Reverting just the two source changes and keeping the tests turns the first one red, so it fails for the reason it claims.

Also checked by hand that a hot loop is unaffected: for (let i = 0; ...) { const v = a[i]; if (v) acc += v; } still emits two fcmp one and zero js_is_truthy calls in the loop body, and returns the right answer.

cargo test -p perry-codegen --lib: 853 passing. cargo test -p perry-hir --lib: 293 passing. The issue's reproducer now matches node --experimental-strip-types line for line.

Summary by CodeRabbit

• Bug Fixes
  • Fixed incorrect truthiness results when accessing arrays with symbol or other non-numeric keys.
  • Preserved optimized behavior for standard numeric array and typed-array indexing.
  • Non-numeric property accesses are now evaluated using general truthiness handling instead of numeric comparisons.
• Tests
  • Added regression coverage for numeric and symbol-keyed array accesses.

[coderabbitai]

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro Plus

Run ID: d41bb45a-2ffe-46b7-89b7-5b2b23d851fe

📥 Commits

Reviewing files that changed from the base of the PR and between 1804991 and f7762af.

📒 Files selected for processing (4)

• changelog.d/7796-symbol-keyed-element-truthiness.md
• crates/perry-codegen/src/type_analysis/numeric.rs
• crates/perry-codegen/src/type_analysis/numeric/tests.rs
• crates/perry-codegen/src/type_analysis/refine.rs

---

📝 Walkthrough

Walkthrough

IndexGet analysis now requires numeric indices for array and typed-array element inference. Symbol-keyed reads use general truthiness handling, while numeric-indexed reads retain the numeric fast path. Regression tests and a changelog entry document the fix.

Changes

Index truthiness correction

Layer / File(s)	Summary
Restrict element refinement to numeric indices crates/perry-codegen/src/type_analysis/refine.rs	IndexGet refinement no longer assigns array or string element types to nonnumeric property accesses.
Correct numeric lowering and regression coverage crates/perry-codegen/src/type_analysis/numeric.rs, crates/perry-codegen/src/type_analysis/numeric/tests.rs, changelog.d/7796-symbol-keyed-element-truthiness.md	Numeric classification preserves masked-window proofs, rejects nonnumeric indices before numeric fast paths, and tests general truthiness for symbol-keyed reads while retaining fcmp one for numeric indices. The changelog records the corrected behavior.

Estimated code review effort: 3 (Moderate) | ~20 minutes

Possibly related PRs

• PerryTS/perry#6850: Both changes restrict IndexGet numeric classification for nonnumeric accesses.
• PerryTS/perry#6997: Both changes update typed-array index proof logic for symbol and nonnumeric indices.
• PerryTS/perry#7746: Both changes require statically numeric indices before numeric type analysis.

Suggested labels: bug

Suggested reviewers: proggeramlug, thehypnoo

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Linked Issues check	⚠️ Warning	The PR fixes the array case and preserves numeric fast paths, but it does not demonstrate coverage for plain objects, user-defined symbols, or explicit any locals required by #7796.	Extend the implementation or tests to verify symbol-keyed reads on plain objects, user-defined symbols, and explicit any locals.

✅ Passed checks (4 passed)

Check name	Status	Explanation
Title check	✅ Passed	The title clearly and concisely describes the main code-generation fix for symbol-keyed element reads.
Description check	✅ Passed	The description explains the issue, fix, related issue, implementation, tests, and verification results, although it does not follow every template heading.
Out of Scope Changes check	✅ Passed	The changelog entry, codegen changes, regression tests, and type-refinement changes all support the linked issue and stated fix.
Docstring Coverage	✅ Passed	No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.

✨ 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.}