fix(codegen): root comparison operands across calls

[proggeramlug]

Closes #7979.

Summary

• lower comparison operands through the shared selective-rooting scope, keeping pointer-capable left values rooted across an allocating right operand and through the consuming comparison dispatch
• add LLVM-IR regressions proving a heap-valued left operand is re-read below the right allocation while a proven numeric operand emits no extra root traffic
• restore the Object.defineProperty moving-GC witness to its natural inline observed() === expected() form and register it in the moving-collector corpus
• register the two moving-GC witnesses shipped by fix(runtime): root JS values retained in Rust containers across allocations (#7949) #7962 but omitted from the corpus, restoring a green test-registration gate

Root cause

Comparison lowering evaluated left, evaluated right, and then consumed the original left SSA register. If the right operand collected, roots were rewritten but that bare register was not. A heap string returned by the left call therefore still named retired from-space when js_eq / js_jsvalue_equals dereferenced it.

with_operands_rooted protects each pointer-capable operand before later operands run, re-reads it after evaluation, and owns release across every early-returning comparison arm. Its existing operand-protection analysis leaves proven primitives in their original registers.

Validation

• cargo test -p perry-codegen --lib: 937 passed
• post-rebase cargo test -p perry-codegen --lib compare_tests: 10 passed
• cargo build --release -p perry -p perry-runtime -p perry-stdlib -p perry-runtime-static -p perry-stdlib-static
• freshly compiled (--no-cache) runtime witness under PERRY_GC_SCHEDULE_SEED=1 PERRY_GC_SCHEDULE_RATE=1 PERRY_GC_PROTECT_FROMSPACE=1 PERRY_GC_PROTECT_FROMSPACE_DEPTH=800: exit 0, all three verdicts ok, 301 copying minors, 111,145 moved objects
• cargo fmt --all -- --check
• python3 scripts/check_test_registration.py: all 206 candidates accounted for
• ./scripts/check_file_size.sh

Summary by CodeRabbit

• Bug Fixes

  • Improved comparison reliability when garbage collection occurs during evaluation.
  • Preserved comparison values across allocating operations and runtime callbacks.
  • Strengthened handling for strings, numbers, BigInts, booleans, nullish values, and mixed-type comparisons.

• Tests

  • Added regression coverage for comparisons involving garbage collection and allocating operands.
  • Expanded validation for property operations and value retention during evaluation.

[coderabbitai]

📝 Walkthrough

Walkthrough

Comparison lowering now roots GC-capable operands across allocating evaluations and dispatch. It adds type-specific equality and relational paths, preserves numeric fast paths, and adds IR and moving-GC regression coverage.

Changes

Comparison operand rooting

Layer / File(s)	Summary
Root comparison operands crates/perry-codegen/src/expr/compare.rs	Comparison lowering roots operands across dispatch and uses rooted values for BigInt comparisons.
Update comparison dispatch crates/perry-codegen/src/expr/compare.rs	Boolean, nullish, string, generic equality, numeric, BigInt, and runtime relational paths now use type-specific handling.
Validate moving-GC comparisons crates/perry-codegen/src/expr/compare_tests.rs, test-files/test_gap_gc_define_property_descriptor_rooting.ts, test-parity/gc_repsel_corpus.txt, changelog.d/8011-root-comparison-operands.md	IR tests verify operand re-reading and numeric register preservation. Moving-GC witnesses and changelog entries cover the updated behavior.

Estimated code review effort: 4 (Complex) | ~45 minutes

Mergeability Score: ⚪ Minimal · up to 77de7

The PR keeps pointer-capable comparison operands valid across allocating calls and adds regression and corpus coverage. The remaining concerns are limited to trivial cleanup, so no actionable merge-blocking risk remains.

Sequence Diagram(s)

sequenceDiagram
  participant ComparisonLowering
  participant with_operands_rooted
  participant GarbageCollector
  participant ComparisonRuntime
  ComparisonLowering->>with_operands_rooted: lower left and right operands
  with_operands_rooted->>GarbageCollector: root GC-capable values
  GarbageCollector-->>with_operands_rooted: re-read moved operands
  with_operands_rooted->>ComparisonRuntime: dispatch non-fast-path comparison
  ComparisonRuntime-->>ComparisonLowering: return comparison result

Loading

Possibly related PRs

• PerryTS/perry#7978: Covers related Object.defineProperty rooting behavior and comparison operands across allocating calls.
• PerryTS/perry#6975: Addresses related GC-safe operand rooting in comparison lowering.
• PerryTS/perry#6983: Uses related temporary-rooting and operand re-reading patterns across allocating evaluations.

🚥 Pre-merge checks | ✅ 5

✅ Passed checks (5 passed)

Check name	Status	Explanation
Title check	✅ Passed	The title clearly summarizes the primary change: rooting comparison operands across calls.
Description check	✅ Passed	The description covers the change, root cause, linked issue, validation, and regression coverage, despite using non-template section names.
Linked Issues check	✅ Passed	The changes implement issue #7979 by rooting and re-reading GC-capable comparison operands before dispatch and adding regression coverage.
Out of Scope Changes check	✅ Passed	The code, tests, corpus registrations, and changelog entry directly support the comparison-rooting fix and its moving-GC coverage.
Docstring Coverage	✅ Passed	No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.

✨ Finishing Touches

📝 Generate docstrings

• Create stacked PR
• Commit on current branch

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Commit unit tests in branch fix/7979-root-binary-operands

---

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 (2)

crates/perry-codegen/src/expr/compare.rs (1)

817-832: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low value

Rename or remove either_non_numeric.

The binding name states "either", but the expression tests that neither operand is numeric. The condition at line 832 also makes the term redundant: unknown_l && unknown_r already implies !is_numeric_expr(left) && !is_numeric_expr(right), because unknown_l and unknown_r each include a !is_numeric_expr conjunct.

Behavior is correct today. The risk is a future edit that weakens unknown_l/unknown_r and then trusts the misleading name to still exclude numeric operands. Remove the redundant term, or rename it to neither_numeric.

♻️ Proposed simplification

-                let either_non_numeric =
-                    !is_numeric_expr(ctx, left) && !is_numeric_expr(ctx, right);
                 let only_eq = matches!(
                     op,
                     CompareOp::Eq | CompareOp::LooseEq | CompareOp::Ne | CompareOp::LooseNe
                 );
@@
                 let unknown_r = !is_numeric_expr(ctx, right)
                     && !is_string_expr(ctx, right)
                     && !is_bool_expr(ctx, right);
-                if either_non_numeric && only_eq && unknown_l && unknown_r {
+                if only_eq && unknown_l && unknown_r {

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@crates/perry-codegen/src/expr/compare.rs` around lines 817 - 832, In the
comparison fallback condition, remove the redundant either_non_numeric binding
and its condition because unknown_l && unknown_r already excludes numeric
operands; preserve the existing Eq/Ne and unknown operand checks.

crates/perry-codegen/src/expr/compare_tests.rs (1)

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

Extract the generic IR helpers into a shared test module.

call_operand_of and producer_line are used by compare_tests, issue7628_rooting_tests, and slice8_rooting_tests. Both helpers fail on missing matches, so the assertions are not vacuous.

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@crates/perry-codegen/src/expr/compare_tests.rs` at line 26, Extract
call_operand_of and producer_line from slice8_rooting_tests into a shared test
helper module, then update compare_tests, issue7628_rooting_tests, and
slice8_rooting_tests to import them from that module. Preserve their existing
behavior, including failing when no matching IR instruction is found.

🤖 Prompt for all review comments with AI agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Nitpick comments:
In `@crates/perry-codegen/src/expr/compare_tests.rs`:
- Line 26: Extract call_operand_of and producer_line from slice8_rooting_tests
into a shared test helper module, then update compare_tests,
issue7628_rooting_tests, and slice8_rooting_tests to import them from that
module. Preserve their existing behavior, including failing when no matching IR
instruction is found.

In `@crates/perry-codegen/src/expr/compare.rs`:
- Around line 817-832: In the comparison fallback condition, remove the
redundant either_non_numeric binding and its condition because unknown_l &&
unknown_r already excludes numeric operands; preserve the existing Eq/Ne and
unknown operand checks.

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro Plus

Run ID: f9267e7e-58d8-4eec-9203-681b1888b899

📥 Commits

Reviewing files that changed from the base of the PR and between 1b53332 and 77de72d.

📒 Files selected for processing (5)

• changelog.d/8011-root-comparison-operands.md
• crates/perry-codegen/src/expr/compare.rs
• crates/perry-codegen/src/expr/compare_tests.rs
• test-files/test_gap_gc_define_property_descriptor_rooting.ts
• test-parity/gc_repsel_corpus.txt