Skip to content

Implement Integer and String eql?/equal?#1553

Merged
matz merged 1 commit into
matz:masterfrom
ryanseys:rs-int-str-eql
Jun 24, 2026
Merged

Implement Integer and String eql?/equal?#1553
matz merged 1 commit into
matz:masterfrom
ryanseys:rs-int-str-eql

Conversation

@ryanseys

@ryanseys ryanseys commented Jun 24, 2026

Copy link
Copy Markdown
Contributor

Integer and String eql?/equal? on typed receivers returned false even for
equal values, because no codegen handler existed and they fell through to the
unsupported-equality path. == already worked, and Float#eql? was already
handled — this mirrors that shape in the Integer and String arms of the
typed-receiver method dispatch (emit_scalar_call).

  • Integer#eql?/equal?(x): value-equal only when x is itself Integer-typed.
    No numeric coercion, so 1.eql?(1.0) is false. equal? on a fixnum is value
    identity, identical to eql? here. A Float or other concrete arg is never
    equal; a polymorphic arg checks its runtime tag.
  • String#eql?(x): byte-equal (via the existing sp_str_eq) only when x is
    String-typed, with no coercion.
  • String#equal?(x): object identity. Strings are const char * whose literals
    the C compiler merges at -O2, so raw pointer equality would wrongly equate
    distinct equal-valued literals (a = "x"; b = "x"; a.equal?(b)). Instead the
    reflexive case is resolved syntactically: equal? is true only when the
    receiver and argument are the same side-effect-free lvalue (the same local or
    instance variable read, e.g. x.equal?(x)); every other form is
    conservatively false. This faithfully models x.equal?(x) without the
    merged-literal false positives. The one gap is aliasing through assignment
    (a = s; a.equal?(s)), which reads as false.

The general poly-receiver eql?/equal? dispatch is left for a follow-up, as
it needs the runtime value-equality helper in lib/sp_runtime.h.

New test test/eql_equal.rb (expected regenerated from ruby) covers
Integer/String eql?/equal?, the 1.eql?(1.0) false case, String equal?
identity (same var true; distinct equal-valued and distinct literals false), and
a typed receiver with a polymorphic argument. make test is green (1032 pass,
0 fail, 0 error).

Co-Authored-By: Claude Opus 4.8 (1M context) noreply@anthropic.com

@gemini-code-assist gemini-code-assist Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Code Review

This pull request adds support for the eql? method on typed String and Integer receivers, as well as the equal? method on typed Integer receivers. The implementation handles both concrete and polymorphic arguments and includes corresponding test cases. Feedback on the changes suggests also implementing String#equal? using pointer equality, as the current implementation does not handle object identity for strings, which would incorrectly return false even when comparing a string variable to itself.

Important

The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.

Comment thread src/codegen_call.c
Add eql?/equal? handlers to the Integer and String arms of the typed-receiver
method dispatch in emit_scalar_call, mirroring the existing Float#eql? shape.

Integer#eql?/equal?(x) is value-equal only when x is itself Integer-typed (no
numeric coercion, so 1.eql?(1.0) is false); equal? on a fixnum is value
identity, identical to eql? here. String#eql?(x) is byte-equal only for a
String-typed x, reusing sp_str_eq without coercion. A polymorphic arg checks
its runtime tag; any other concrete type is never equal.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
@matz matz merged commit e12e1b9 into matz:master Jun 24, 2026
3 checks passed
@ryanseys ryanseys deleted the rs-int-str-eql branch June 24, 2026 14:53
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants