Boxed receivers dispatch user <, >, <= and >= without coerce

[FrancescoK]

Probed at master fc7ef4762800b37b70732e5910d1c1a05fa68382, macOS, Apple clang 21.

class Money
  attr_reader :c
  def initialize(c)
    @c = c
  end
  def <(o) = c < o.c
end
vals = [Money.new(1), 3]
p vals[0] < Money.new(5)

	CRuby 4.0	Spinel before	Spinel after
Result	true	ArgumentError: comparison of Money with Money failed	true

• A boxed comparison reaches a user's operator through sp_user_binop_hook. Its existing sp_user_binop_dispatch table already supports <, >, <= and >=.
• codegen_program enabled that table for these operators only when another operator or a coerce shape already required it. A class defining only its own ordering could fall through to sp_poly_cmp, which cannot supply that ordering.

Why. A class's own comparison must work when its receiver comes out of a mixed Array or a polymorphic parameter, including inherited operators and non-Boolean return values.

The fix (src/codegen.c). Extend the existing hook-detection scan with is_cmp_op and comp_method_in_chain. A class defining or inheriting <, >, <= or >= enables the existing dispatch table without requiring coerce. The other hook conditions, runtime comparison paths and String-sharing behavior stay unchanged. The source change adds four lines, including its comment.

Performance and C change. This restores comparison dispatch; no benchmark speedup is claimed. All 67 benchmarks and optcarrot emit byte-identical C against the master above, both by default and with SPINEL_SHARE_STRINGS=1 (-S --no-line-map). Neither mode has a benchmark or optcarrot candidate for callgrind. Numeric receivers still avoid the user hook through the existing numeric and receiver-kind checks.

In the eight related tests compared in both modes, only two programs change C:

• poly_user_relop_no_coerce: the new regression gains the operator table and hook installation for Money's four comparison methods and Euro's inherited methods.
• visibility_explicit_receiver: the existing test gains the table and hook installation for Account's > and Savings' inherited >.

The other six related tests emit identical C. All 76 samples compile in each mode; there are no refusal changes. The full corpus comparison is pending.

Corpus C diff. Against master 80e28dd29, of the 6,524 programs in test/, test/infer/, benchmark/, the packages' tests and optcarrot, 2 programs change: this PR's new test/poly_user_relop_no_coerce.rb and 1 existing ones: test/visibility_explicit_receiver.rb. The only other differences are the build stamp in RUBY_DESCRIPTION. The benchmarks and optcarrot emit the same C as before.

Callgrind. Not run: optcarrot and all 67 benchmarks emit byte-identical C in both builds, and lib/ is untouched.

Tests. Eight focused tests match their CRuby 4.0 frozen-literal output in 64 run configurations: default/share-strings, plain/promote, and normal/GC stress. The new regression covers all four operators, Symbol and nil results, inherited methods, mixed Array reads, a polymorphic parameter, a sort block and numeric neighbors. On current master it raises comparison of Money with Euro failed in both string modes.

make -j3, make infer-test (46 fixtures plus its C assertions), and make share-strings-test (179 programs, normal and GC stress: 358 runs) pass. Traits checks pass for 52 kinds in both integer modes; builtin arity checks pass for 494 Method rows and 914 operation rows. The 16 focused representation/plan-check compiles preserve generated C and show no representation or call-plan conflicts. The Linux aarch64 build in spinel-cg passes with GCC 13.3 (make -j3 all CC=gcc OPT=-O1); the new regression passes all eight string/integer/GC configurations there at -O1.

Not covered, also on master: --plan-check reports that open has a hand sharing row without an iterator row. Diagnostics for all 16 focused compiles, including the existing fallback and unrecorded-call messages, are identical to master's.

make gate (on this branch merged with current master)

Merged with master 9922a2c74:

scale-test: boxed Hash store work at 2x the methods is 1.95x (limit 2.20)
scale-test: boxed-receiver alias work at 2x the writes is 1.86x (limit 2.20)
scale-test: instance_eval forwarding work at 2x the wrappers is 1.71x (limit 2.50)
scale-test: work at 4x the program is 4.73x (linear 4.00, limit 5.20)
scale-test: work at 4x the program, compiled to C, is 6.14x (limit 6.90)
scale-test: call-shape work at 4x the units, compiled to C, is 4.13x (linear 4.00, limit 4.50)
Tests: 6440 pass, 0 fail, 0 error
gate: stamp for tree 6bddc26cf97d on master 9922a2c74ee1; git commit --amend --no-edit adds the Gate: trailer
gate: ALL GREEN

• New tests have .expected files that match CRuby 4.0 run with --enable-frozen-string-literal
• Values past 2^31 are marked # spinel: int64 (none in the new test)
• If optcarrot's generated C changed: callgrind numbers, checksum 59662 (its C is unchanged in both modes)
• Depends on: #

🤖 Generated with Claude Code

Summary by CodeRabbit

• Bug Fixes
  • Comparisons using user-defined <, >, <=, and >= operators now work for boxed values even when the class does not define coerce.
  • Sorting boxed values with user-defined comparison operators now works.

[coderabbitai]

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

⚙️ Run configuration

• Configuration used: Organization UI
• Review profile: CHILL
• Plan: Advanced
• Run ID: 01eecaea-d4ca-47af-bbb8-50d47720a825

📥 Commits

Reviewing files that changed from the base of the PR and between 9922a2c and ada0f34.

📒 Files selected for processing (4)

• Makefile
• src/codegen.c
• test/poly_user_relop_no_coerce.rb
• test/poly_user_relop_no_coerce.rb.expected

Included review availability: This review used your included allowance. Your plan provides up to 8 included reviews per hour; 7 remain after this review.

---

📝 Walkthrough

Walkthrough

The code generator now enables user binary-operator dispatch for comparison operators without requiring #coerce. A new test fixture covers comparisons involving Money and Euro, and the test target checks generated hook installation.

Changes

Comparison dispatch

Layer / File(s)	Summary
Enable comparison dispatch src/codegen.c	The feature scan enables user binary-operator dispatch when an instantiated class defines <, >, <=, or >=, even without #coerce.
Add regression coverage test/poly_user_relop_no_coerce.rb, test/poly_user_relop_no_coerce.rb.expected, Makefile	The fixture prints mixed-object and numeric comparisons and sorts Money instances. The expected output records ten results. The infer-test target checks that generated C installs sp_user_binop_dispatch through SP_INSTALL_HOOK.

Priority: ➖ Normal

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

Change: Bug fix

Merge Risk: ⚪ Minimal · up to ada0f

The change enables comparison dispatch for classes without coerce, and the regression fixture checks comparison results and hook installation. No actionable merge-blocking risk was identified in the supplied evidence.

Security Architecture Review

Security architecture risk: 🔵 Low · up to ada0f

The change restores comparisons for user-defined objects without demonstrating new external access or privilege. Initialization and concurrent-use guarantees remain incompletely verified, limiting assurance.

Retained concerns
No architecture-level concerns identified.

Security review details

Security Blast Radius

• inferred — The established exposure is execution of application-defined operators in generated programs newly eligible for the hook. The inspected fixture supplies no external input or privileged sink; tenant, service and deployment exposure are not established by the available evidence.

Trust Boundaries and Controls

• observed — Runtime dispatch retains boxed-receiver checks, and generated arms retain class and operand identity guards. Numeric receivers remain on existing non-user-hook paths. Removing the coerce eligibility prerequisite does not demonstrate bypass of an authentication or authorization control.

Resilience and Maintainability Implications

• inferred — When initialization is emitted, declaration, dispatcher generation and installation are ordered before program-body execution. Universal installation for newly eligible minimal programs remains unproven because the startup gate does not directly include the new eligibility flag. Multi-unit ownership, concurrent installation, repeated initialization and teardown of the global hook also remain unresolved. These are assurance gaps, not established vulnerabilities.

🚥 Pre-merge checks | ✅ 4 | ❓ 1

❌ Failed checks (1 inconclusive)

Check name	Status	Explanation	Resolution
Docstring Coverage	❓ Inconclusive	Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 6 functions across 1 files. (3 skipped: 2 …	Write docstrings for the functions missing them to satisfy the coverage threshold.

✅ Passed checks (4 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title clearly and concisely describes the main change: boxed receivers now dispatch user-defined relational operators without requiring coerce.
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.

Full details: Docstring Coverage

Explanation

Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 6 functions across 1 files. (3 skipped: 2 unsupported, 1 too large.)

• Autopilot · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

---

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