Skip to content

Stage 5 parallel round A: Ripper files, error_highlight through Prism, per-object ivar index - #81

Open
284km wants to merge 8 commits into
mainfrom
stage5-par
Open

284km wants to merge 8 commits into
mainfrom
stage5-par

Conversation

@284km

@284km 284km commented Nov 12, 2026

Copy link
Copy Markdown
Owner

Stage 5 work run in parallel with round 22 (stage 9): Ripper and error_highlight from note 545's remainder. LOOP.md (39) has the reasoning and the measurements.

What changes

  • Ripper's files tests (55). test/ripper's parse-files Parser answers Object.new for every event; the translation asked that value any? to decide whether a call is a command. lib/ripper_ext.rb now decides by the node, as the receiver branch already did.
  • Prism loads 2.2x faster: the Loader reads the serialized bytes with a bare reader instead of StringIO (lib/prism_ext.rb).
  • A per-object index of instance variable names (iv_idx / iv_put, m_state): every write to the ivar table goes through iv_put, so Object#inspect / #instance_variables no longer walk every object's ivars. MERE_RUBY_IVAR_INDEX_CHECK=1 compares the index with the walk on every listing.
  • error_highlight (1 -> 105 of 120 locally): RubyVM::AbstractSyntaxTree.of raises as ruby does for a Prism-compiled body, and node_id_for_backtrace_location finds the frame's call in the file's Prism tree, by the name of the call each frame was making (written on the Locations by #backtrace_locations).
  • Interpreter defects found on the way:
    • a rescued pair exception keeps its own frames (rescue => e binds an object; its #backtrace_locations used to follow the last raise);
    • a pair's #detailed_message / #full_message reach a prepended or own definition, and their super;
    • a Location read from a label with :: no longer loses its path and line;
    • +nil / -nil raise NoMethodError; nil[0] = 1 says "for nil";
    • A::NotDefined::Foo raises the head's NameError, and a head found through the nesting is named as found.
  • Ripper reports errors and warnings as parse.y does (warn vs warning, compile_error, and the tree built through the errors parse.y builds through): parser_events 165 -> 197.

Checked (Mere v0.1.731, rebased on round 22)

  • corpus 674/674 with the ivar index check on (witness 643 added); bracket depth 977/1024, dead defs, gc roots.
  • Locally against round 21's CI counts: ripper's six files 55/55 (lib 119 s, test_2 83 s, under 1 GB), error_highlight 274 s, test_ast 2,559 (313 s, 864 MB), and about 90 files the changes touch at or above round 21.

Left

test_ast's parse.y-shaped tree (needs Prism's load built natively first, see LOOP.md), a bare name with a builtin self raising NoMethodError instead of NameError, IO#lineno=, lex_compat details.

🤖 Generated with Claude Code

@284km 284km left a comment

Copy link
Copy Markdown
Owner Author

Choose a reason for hiding this comment

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

Reviewed efdb917d86200b9df04f5b7891382d4b0ede0e33 against 05f8a463065df05cc1b73ce3100d595f2aca348e. Two P2 issues are detailed inline: anonymous-class ivar index entries survive collection, and ambiguous same-line calls can produce the wrong backtrace node ID.

The opaque Ripper-event fix is effective: the new corpus witness fails on the exact base artifact and matches Ruby on the head. The generated Prism library also exactly matches regeneration from the checked source.

Independent validation:

  • Fresh O2 native build with pinned Mere b92d7335, SHA-256 b7d7806eed05a143b65fd743447426518d9c44fa8344536c01e95f0ac38efcf7.
  • Base comparisons use the published Darwin/arm64 O2 artifact with recorded source 05f8a463 and verified binary SHA-256 1ebe1ac159fd45e0c3bf2810a8c08d1d13c75910233ec64ca44feab590e4db3f.
  • New corpus 643 matches Ruby 4.0.9 in normal, hash-index-off and MRO-cache-off modes with GC100 and MERE_RUBY_IVAR_INDEX_CHECK=1, with no index diagnostics.
  • Both the memory scaling witness and the ambiguous-node witness were checked against reference/base behavior as applicable. The node error also reproduces with GC100 and MRO caching disabled.
  • Method-write, fresh generated-C GC-root, name-length, duplicate-definition and dead-definition guards pass.

The complete hosted native/resource jobs are still running. These focused results are not a claim that all upstream compatibility or resource gates pass.

Comment thread main.mere
let dIX = vec_new () in
let _ = map_iter iv_idx (fn (k: str) -> fn (v: str) ->
let owner = gc_owner_of k in
if owner < 0 then ()

Copy link
Copy Markdown
Owner Author

Choose a reason for hiding this comment

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

[Major / P2] Reclaim index entries owned by collected anonymous classes

iv_put indexes class ivars under prefixes such as cls:AnonClass_...@, but gc_owner_of only recognizes numeric object prefixes. Consequently this branch keeps every class-owned index entry forever. The existing class sweep removes the corresponding ivars_g rows, but iv_idx is not part of that sweep. This reintroduces retained metadata for discarded anonymous classes.

I compared the exact base artifact and fresh head with this bounded witness:

name = '@' + 'x' * 65536
ARGV[0].to_i.times do |i|
  klass = Class.new
  klass.instance_variable_set(name, i)
  klass = nil
  GC.start if i % 10 == 9
end
GC.start

For 40 / 400 / 1,200 classes, base peak RSS stays at approximately 66.0 / 66.1 / 66.5 MiB, while head reaches 71.2 / 148.0 / 209.4 MiB, despite the explicit collections. Long valid ivar names make the retained entries visible; the first insertion also bypasses the stated 4-KiB fallback. Either avoid indexing nonnumeric prefixes (the indexed reader currently queries numeric object IDs), or include class-owned entries in the same ownership sweep as ivars_g; apply the size bound to first insertions too.

Copy link
Copy Markdown
Owner Author

Choose a reason for hiding this comment

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

Fixed in 137cd7f. iv_idx_add now files only keys whose prefix is an object id (oid@) — the indexed reader asks by object id, and those are the owners the collector's sweep drops — so a class's ivars (cls:AnonClass_…@) are no longer indexed. The 4 KB bound now also applies to the first name filed for an object. Your witness is a tools/loop_rss_check.rb case ("a discarded class's ivar", 1,200 classes with a 64 KB name): 2.05x before, 1.06x after.

The same entries were behind this PR's first build-linux failure: class_module_check timed out test_complemented_method_entry_memory_leak (300,000 turns of Class.new and a refinement in 10 s). A key that is not an object's is now written without asking the table first, and that loop runs as fast as main does locally (5.8 s vs 5.6–5.8 s); test_module is 195/195 locally.

Comment thread lib/prism_ext.rb Outdated
cands.find { |n| n.type == :def_node && n.name == own } ||
cands.find { |n| n.type == :lambda_node || n.type == :block_node }
else
(want && cands.find { |n| __nid_call?(n) && __nid_names(n).include?(want) }) ||

Copy link
Copy Markdown
Owner Author

Choose a reason for hiding this comment

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

[Major / P2] Do not identify an ambiguous call by the first matching name

A line number and callee name do not identify a unique call. For example, run this from a file:

begin
  'first'.upcase; nil.upcase
rescue NoMethodError => error
  id = RubyVM::AbstractSyntaxTree.node_id_for_backtrace_location(
    error.backtrace_locations.first
  )
  # Look up id in Prism.parse_file(__FILE__).value.
end

Reference Ruby's ID selects nil.upcase. Head selects 'first'.upcase, because both candidates are named upcase and find chooses the first. I reproduced the same wrong slice with GC100 and MRO caching disabled. This supplies error_highlight with the position of a successful call instead of the failing one. Preserve execution/source-offset information to distinguish calls, or return an unknown node ID when the available metadata is ambiguous rather than presenting an arbitrary candidate as the executed node.

Copy link
Copy Markdown
Owner Author

Choose a reason for hiding this comment

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

Fixed in 137cd7f. Each way of choosing a node (the callee name, the frame's own name for a C method, the line's calls) now has to find exactly one candidate. If it finds more than one, the id is -1 (no node, so no snippet) instead of the first match. Your 'first'.upcase; nil.upcase case is now in corpus 643 as "no node, or nil.upcase": it never answers the call that succeeded. One exception: an operator write and the constant path it writes are one construct, so ::X += 1 still answers the write, as ruby's id does. error_highlight goes from 105 to 107/120 locally, because test_COLON2_5 and test_OP_CDECL_read_3 expect no snippet for exactly this kind of line.

284km added a commit that referenced this pull request Nov 12, 2026
…it, and the 4 KB bound skipped a first name; loop_rss_check case), a key that is no object's is written without the lookup (class_module_check's complemented_method_entry loop timed out); node_id_for_backtrace_location answers -1 when more than one node answers rather than the first (an operator write and its target are one); error_highlight 107/120

@284km 284km left a comment

Copy link
Copy Markdown
Owner Author

Choose a reason for hiding this comment

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

Follow-up review of 137cd7f6 against unchanged base 05f8a463: both previously reported P2 issues are fixed. No new actionable findings in the update.

  • Anonymous-class index retention: non-object keys no longer enter the index, and the first ivar name obeys the size bound. Re-running the original 40 / 400 / 1,200-class witness with a fresh O2 build gives peak RSS of approximately 66.2 / 66.7 / 67.2 MiB, close to the exact base artifact's 66.0 / 66.7 / 66.5 MiB. The new loop RSS regression case passes at 1.06x.
  • Ambiguous node IDs: the original 'first'.upcase; nil.upcase witness now yields an unknown node rather than identifying the successful call. This is the conservative fallback requested in the earlier review.
  • Updated corpus 643, the standalone ambiguity witness, and long-name / aggregate-size ivar fallback checks all pass in normal, hash-index-off and MRO-cache-off modes with GC100 and the ivar index checker enabled (nine runs, no diagnostics). Corpus and ivar listing output match Ruby 4.0.9; the ambiguity witness intentionally returns unknown instead of Ruby's precise node.
  • Embedded Prism regeneration matches; method-write, fresh generated-C GC-root, name-length, duplicate-definition and dead-definition guards pass.

Validation used pinned Mere b92d7335; new native binary SHA-256: 48e2bf1aa7be151e99565b93214c3e008181ee512de1831786914ed00eb65e40. Hosted reference tests pass; Linux/resource and corpus jobs are still pending, so this does not establish that the complete gates pass.

@284km 284km left a comment

Copy link
Copy Markdown
Owner Author

Choose a reason for hiding this comment

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

Follow-up review of f8c4b027 against 05f8a463. One new P2 finding is inline: the pair/object equality bridge can invoke an unrelated right-hand object's ==, changing both the result and side effects of array comparisons and membership checks.

The added TracePoint corpus case itself matches Ruby 4.0.9 in normal, GC100, hash-index-off and MRO-cache-off modes. The earlier anonymous-class index and ambiguous-node fixes are retained; long-name ivar fallback and the ambiguity witness still pass in all four modes with the ivar index checker enabled. No duplicate findings are posted for those fixes.

Validation used a fresh O2 native build with pinned Mere b92d7335, binary SHA-256 c050329f72ac68c8778a3c2f7ddfbc0b7da7910d34e4827285d1fe60ca704c17. The new regression was compared with both the exact base artifact and the previous reviewed binary. Method-write, fresh generated-C GC-root, name-length, duplicate-definition and dead-definition guards pass. Hosted reference tests pass; Linux/resource and corpus jobs remain pending.

Comment thread main.mere Outdated
| _ -> rb_eq a b)
| VExc (_, _) ->
(match b with
| VObj _ -> truthy (invoke_val_c env world (false, (Nil : str list), (Nil : Stmt list), env) b "==" (Cons (a, Nil)) false)

Copy link
Copy Markdown
Owner Author

Choose a reason for hiding this comment

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

[Major / P2] Preserve the exception receiver when comparing a pair with an arbitrary object

This VObj arm reverses == for every object, although the bridge is intended for another representation of an exception. An exception does not generally delegate equality to an arbitrary right-hand operand. The new Array routing makes this observable with the same TracePoint representation used by the added corpus case:

class EqualAnything
  def ==(other)
    $calls += 1
    true
  end
end
seen = nil
tp = TracePoint.new(:c_call) do |t|
  seen = t.self if t.method_id == :initialize && t.defined_class == Exception
end
tp.enable
begin; raise RuntimeError; rescue RuntimeError => bound; end
tp.disable
$calls = 0
p [seen] == [EqualAnything.new]
p $calls

Ruby 4.0.9, the exact base artifact (05f8a463), and the previously reviewed head (137cd7f6) all print false then 0. The current head prints true then 1: it invokes the unrelated object's method instead of answering the exception's comparison. Nested arrays reproduce the same result, and [seen].include?(other) also invokes that reversed comparison. The array witness reproduces in normal mode, GC100, hash-index-off and MRO-cache-off.

Restrict this representation bridge to compatible exception objects, or perform Exception#== on the exception side with its normal conversion rules, rather than invoking b.==(a) for any VObj.

Copy link
Copy Markdown
Owner Author

Choose a reason for hiding this comment

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

Fixed in 5416d9b. A pair now hands its comparison only to an object that is an exception and answers the built-in #== (exc_bridge_peer). Any other object on the right gets the exception's own answer (rb_eq, false), and its #== is not called. The same condition now guards the binary == arm added earlier.

Your witness is now in corpus 643. It also covers a nested Array, include?, and the reverse direction. The output matches Ruby 4.0.9: seen == other, [seen] == [other], [[1, seen]] == [[1, other]] and [seen].include?(other) are all false with 0 calls, and other == seen is true through other's own #== (1 call). The case this bridge exists for is unchanged: the rescued exception still equals the TracePoint's self.

Locally: corpus 677/677 with MERE_RUBY_IVAR_INDEX_CHECK=1. settracefunc, exception, backtrace, array, hash, set, object, struct, comparable, range, module, delegate, error_highlight and json are all at or above the previous counts.

…the node, not by what a handler returned (test/ripper's parse-files, 55); Prism's Loader reads bytes without StringIO (2.2x); a per-object index of instance variable names (inspect no longer walks every object's); RubyVM::AbstractSyntaxTree.of and node_id_for_backtrace_location through the file's Prism tree, for error_highlight (1 -> 105); a rescued pair exception keeps its own frames; a pair's detailed_message reaches the program's; Locations of labels with '::'; +nil / -nil refuse; a constant path's head names its own NameError; Ripper reports errors and warnings as parse.y does (parser_events 165 -> 197) (witness 643)
…it, and the 4 KB bound skipped a first name; loop_rss_check case), a key that is no object's is written without the lookup (class_module_check's complemented_method_entry loop timed out); node_id_for_backtrace_location answers -1 when more than one node answers rather than the first (an operator write and its target are one); error_highlight 107/120
… (Array#== looked only at the left array for an element with an == of its own): test_settracefunc's test_tracepoint compares the self a TracePoint saw in Exception#initialize with the object the rescue bound
…tion answering the built-in == (exc_bridge_peer), not to any object on the right: [pair] == [obj] called obj's == and answered true where ruby says false (witness in 643: ==, Array, nested Array, include?)

@284km 284km left a comment

Copy link
Copy Markdown
Owner Author

Choose a reason for hiding this comment

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

Follow-up review of 5416d9bb against 140575da: the previously reported arbitrary-object == calls are fixed, including arrays, nested arrays and include?. The expanded corpus 643 also matches Ruby 4.0.9 in all four tested modes.

One P2 remains in the revised bridge, detailed inline: a compatible right-hand exception with a singleton == override now prevents the left exception's built-in value comparison. The new witness is true on Ruby and the exact current base, but false on head; no right-hand override should run in either case.

Independent validation used a fresh O2 native build with pinned Mere b92d7335, SHA-256 f837bbdc8622f46e31150cc253968e4ead80f269c5bae32d62b9e2ed8ebb3f73. The original reproductions, new exception-peer witness and updated corpus were run in normal, GC100, hash-index-off and MRO-cache-off modes with the ivar index checker enabled. Method-write, fresh generated-C GC-root, name-length, duplicate-definition and dead-definition guards pass. Hosted full gates are still running.

Comment thread main.mere Outdated
// question the exception never asks (PR #81's review).
and exc_bridge_peer = fn world -> fn (v: Val) ->
match v with
| VObj _ -> exc_is_a world (dispatch_cls_of world v) "Exception" && (if has_own_eq world v then false else true)

Copy link
Copy Markdown
Owner Author

Choose a reason for hiding this comment

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

[Major / P2] Keep the left exception's value comparison when the right exception overrides ==

The original unrelated-object witness is fixed, but rejecting every exception with its own == also skips valid Exception#== comparisons. The right operand's override must not decide whether the left operand can compare its class, message and backtrace. With this predicate false, the pair/object fallback is rb_eq, which simply answers false for the two representations.

seen = nil
tp = TracePoint.new(:c_call) do |t|
  seen = t.self if t.method_id == :initialize && t.defined_class == Exception
end
tp.enable
begin; raise RuntimeError, 'same'; rescue RuntimeError => bound; end
tp.disable
other = bound.dup
$calls = 0
def other.==(x)
  $calls += 1
  false
end
p seen == other
p [seen] == [other]
p [seen].include?(other)
p $calls

Ruby 4.0.9 and the exact current-base O2 artifact (140575da) print true, true, true, 0. The head prints false, false, false, 0, in normal, GC100, hash-index-off and MRO-cache-off modes. Not calling the right override is correct, but dropping the left exception's comparison changes all three results. Please perform the exception-side built-in comparison for a compatible exception peer even when that peer overrides ==, while retaining the refusal to call an arbitrary right-hand override.

Copy link
Copy Markdown
Owner Author

Choose a reason for hiding this comment

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

Fixed in a9e6041. A pair facing an exception object no longer asks that object's #==, and no longer gives up on the comparison. It now runs its own Exception#== (exc_pair_equal): same class, then the other's #message and #backtrace compared with the pair's. The class is read through the singleton that def twin.== creates. An object that is not an exception is still never equal and is never asked.

Your witness is now in corpus 643, next to the previous round's. Both match Ruby 4.0.9: seen == twin, [seen] == [twin] and [seen].include?(twin) are true with 0 calls to the override, and the EqualAnything case is false with 0 calls.

Locally: corpus 677/677 with MERE_RUBY_IVAR_INDEX_CHECK=1. settracefunc, exception, backtrace, array, hash, set, object, struct, comparable, range, module, delegate, error_highlight and json match the previous counts. test_thread's test_priority differed once locally; it is the 0.5 s counter race (1,318,602 vs 1,320,061).

… the pair's own Exception#== -- the class, the other's #message and #backtrace -- and the other's #== is never called, so an exception that overrides == still equals the pair as in ruby (its class read through the singleton a def gave it; witness in 643)

@284km 284km left a comment

Copy link
Copy Markdown
Owner Author

Choose a reason for hiding this comment

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

Follow-up review of a9e60417 against 140575da: both previously reported equality witnesses now pass. In particular, the right-hand exception's singleton == is no longer called and no longer suppresses a valid left-hand comparison. The expanded corpus 643 matches Ruby 4.0.9 in all four tested modes.

One new P2 regression is detailed inline: the new helper dispatches the right-hand exception's overridden message during a same-class comparison, whereas Ruby reads the stored exception message. The witness passes on Ruby, the exact current base and the previous head, but changes from true to false on this head and executes the override. I also checked the pinned Ruby source's exc_equal implementation to verify the intended semantics.

Independent validation used a fresh O2 native build with pinned Mere b92d7335, SHA-256 4a97d491c26bb3ed5c7cf9ec22cce2916d09811dc08aa333558c5fddc122f75e. Prior reproductions, the new message witness and the expanded corpus were run in normal, GC100, hash-index-off and MRO-cache-off modes with the ivar index checker enabled. Method-write, fresh generated-C GC-root, name-length, duplicate-definition and dead-definition guards pass. Hosted full gates are still running.

Comment thread main.mere Outdated
else
(let nb9 = (false, (Nil : str list), (Nil : Stmt list), env) in
let pair9 = VExc (c, m) in
rb_eq (invoke_val_c env world nb9 o "message" Nil false) (vexc_text world c m)

Copy link
Copy Markdown
Owner Author

Choose a reason for hiding this comment

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

[Major / P2] Read the stored message when comparing same-class exceptions

This new helper invokes the right-hand exception's public message method. Ruby's same-class Exception#== instead reads the stored message and backtrace; an overridden presentation method must not change equality or run during the comparison. The pinned Ruby source makes this distinction explicitly: exc_equal in error.c uses rb_attr_get(obj, id_mesg) for the same-class branch, reserving public message/backtrace calls for conversion from a different class.

seen = nil
tp = TracePoint.new(:c_call) do |t|
  seen = t.self if t.method_id == :initialize && t.defined_class == Exception
end
tp.enable
begin; raise RuntimeError, 'same'; rescue RuntimeError => bound; end
tp.disable
other = bound.dup
$calls = 0
def other.message
  $calls += 1
  'different'
end
p seen == other
p [seen] == [other]
p $calls

Ruby 4.0.9, the exact current base (140575da) and the previous head (5416d9bb) print true, true, 0. Current head prints false, false, 2, in normal, GC100, hash-index-off and MRO-cache-off modes. Thus this update introduces both wrong comparison results and unexpected user-code execution; an override that raises would also make equality raise. Please use the exception's stored message and backtrace in this same-class path, as the built-in comparison does, instead of dispatching public presentation methods.

Copy link
Copy Markdown
Owner Author

Choose a reason for hiding this comment

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

Fixed in ceb084a. In the same-class branch, exc_pair_equal now reads the other exception's stored message (@__exception_message, or the class name when there is none) and its stored backtrace (@__bt), as exc_equal's rb_attr_get does. It reads the pair's frames from the raise record (pair_bt_raw). It no longer calls #message or #backtrace, so an override cannot change the answer, and it does not run or raise during ==.

While checking this I found that Array#include?, #index and #rindex used the structural path for these operands, because they only take the semantic path for an element or argument with its own ==. They now take the semantic path when the argument is an exception.

corpus 643 now has your witness, plus a variant whose #message and #backtrace raise. Both match Ruby 4.0.9: seen == shown, [seen] == [shown] and [seen].include?(shown) are true with 0 calls, and the raising variant compares equal without raising. The two earlier rounds' witnesses are unchanged.

Locally: corpus 677/677 with MERE_RUBY_IVAR_INDEX_CHECK=1. settracefunc, exception, backtrace, array, hash, set, object, struct, comparable, range, module, delegate, error_highlight and json are at the previous counts.

…the other's stored message and backtrace, as exc_equal's rb_attr_get does, and the pair's frames from the raise record -- an overridden #message / #backtrace neither decides the answer nor runs; Array#include? / #index ask the semantic comparison for an exception argument (witness in 643)

@284km 284km left a comment

Copy link
Copy Markdown
Owner Author

Choose a reason for hiding this comment

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

Follow-up review of ceb084ab against 140575da: the previously reported public-message/backtrace dispatch is fixed. All earlier equality witnesses and the expanded corpus 643 match Ruby 4.0.9 in the four tested modes, including presentation-method overrides that raise.

One remaining P2 compatibility issue is detailed inline: Array#count and Hash value lookup/equality can still select structural comparison for the pair/object representations of the same exception. This gives zero matches, misses hash values and makes Hash#== asymmetric. I compared the related container operations as a group rather than posting separate findings for each method; the exact current base matches Ruby on this matrix.

Independent validation used a fresh O2 native build with pinned Mere b92d7335, SHA-256 05b1c7212cbbef0b2aa6a72d344d9572906012de3e2af541986461201934ea60. Prior reproductions, presentation-method checks, the container matrix and expanded corpus were run in normal, GC100, hash-index-off and MRO-cache-off modes with the ivar index checker enabled. Method-write, fresh generated-C GC-root, name-length, duplicate-definition and dead-definition guards pass. Hosted full gates remain pending.

Comment thread main.mere Outdated
@@ -17414,6 +17486,12 @@ and invoke_val_c0 = fn env -> fn world -> fn blk -> fn (recv0: Val) -> fn (name0
// (...or the ARGUMENT's: an element 5 asks `5 == obj`,
// and Integer#== asks obj -- num_equal)
|| (match args with Cons (a9, _) -> has_own_eq world a9 | _ -> false)
// (...or an exception, which a pair and an object of
// it compare equal across: exc_pair_equal)
|| (match args with

Copy link
Copy Markdown
Owner Author

Choose a reason for hiding this comment

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

[Major / P2] Route the remaining value-based container operations through exception equality too

The new exception guard fixes these Array searches, but the sibling Array#count and Hash value operations still choose their structural paths when neither operand defines a custom ==. That is no longer sufficient after rescue materializes a pair as an object. Hash's binary == also checks only its left-hand values before choosing the semantic path, so it can become asymmetric.

seen = nil
tp = TracePoint.new(:c_call) do |t|
  seen = t.self if t.method_id == :initialize && t.defined_class == Exception
end
tp.enable
begin; raise RuntimeError, 'same'; rescue RuntimeError => bound; end
tp.disable
twin = bound.dup
p [seen].count(twin)
h = {key: seen}
p h.value?(twin), h.key(twin)
p h == {key: twin}, {key: twin} == h

Ruby 4.0.9 and the exact current-base artifact (140575da) answer 1, true, :key, true, true. Head answers 0, false, nil, false, true. The inverse Array#count and Hash#has_value? fail too. All reproduce in normal, GC100, hash-index-off and MRO-cache-off modes.

Please apply the exception-aware semantic routing to Array#count and Hash's value-search methods as well, and account for either hash's values in the binary Hash#== fast-path decision. A shared predicate plus a container-API regression matrix would keep these representations from comparing differently depending on the entry point. I checked the related paths together: Array include?/index/find_index/rindex/delete/assoc/rassoc, Struct#== and the Enumerator checks now match reference; the failures above remain.

Copy link
Copy Markdown
Owner Author

Choose a reason for hiding this comment

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

Fixed in 0739117. As you suggested, there is now one shared predicate, exc_cmp_val: a pair, or an object that is an exception. Every container entry point that chooses between the structural and the semantic comparison now consults it:

  • the elements' check elem_has_user_eq, which include?, count, delete, value?/has_value? and key all read;
  • the argument's check in count, value?/has_value? and key;
  • the Array#== and Hash#== fast-path decision (arr_has_obj_d, which hash_has_obj uses). The binary Hash#== now looks at the values of both hashes, so h == g and g == h agree.

corpus 643 now has your matrix with a plain bound.dup: [seen].count(x), [x].count(seen), value?, has_value?, key, the inverse has_value?, Hash#== from both sides, index, include? and nested Array#== both ways. All of it matches Ruby 4.0.9: 1, 1, true, true, :key, true, true, true, 0, true, true, true. It also checks that an exception on the left whose == is overridden still answers through its own ==.

Locally: corpus matches except one run of 394 (handle_interrupt and signal timing under a load average of about 5), which passed 20/20 when rerun. array, hash, set, struct, comparable, range, enumerator, enum, marshal, string, object, module, exception, backtrace, settracefunc, delegate, error_highlight, json and psych are at the previous counts. The dead-defs and dup-defs gates pass.

Copy link
Copy Markdown
Owner Author

Choose a reason for hiding this comment

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

Follow-up for the object/object part (review 5878981676), fixed in d3c88ae.

The built-in Exception#== now follows exc_equal:

  • an identical object is equal;
  • if the class is the same (rb_obj_class, so read through any singleton), it compares what both exceptions hold: the stored message and the stored backtrace. This is exc_same_class_equal, one helper shared by pairs and objects in every combination, and it calls no method;
  • otherwise it converts the other object with #exception and asks that object's #message and #backtrace, as rb_check_funcall does.

An exception object now also answers == on every container path: obj_answers_eq counts it, and rb_eq_w0 dispatches its built-in == instead of falling back to identity. A user-defined left-hand == still wins.

corpus 643 now has:

  • your witness: bound == twin, include?, count, index/rindex, value?/has_value?, key, Hash#== both ways, and Array#==;
  • the def odd.message = 'different' case, both directions and in containers;
  • pair/pair, pair/object, object/pair and object/object.

All of it matches Ruby 4.0.9 (true, true, 1, true, :key, true, …).

Locally: corpus 677/677 with MERE_RUBY_IVAR_INDEX_CHECK=1. The dead-defs and dup-defs gates pass. exception, backtrace, settracefunc, array, hash, set, object, struct, comparable, range, module, enumerator, enum, marshal, string, thread, fiber, method, delegate, error_highlight, json, psych and did_you_mean are at the last full CI's counts. keyword is unmeasurable locally: the local reference lacks the -test- extension.

…n exception object) for every container entry point that chooses between structural and semantic comparison -- the elements' check (include? / count / delete / value? / key), the argument's of count / value? / key, Array#== and Hash#== (now looking at both hashes' values); witness matrix in 643

@284km 284km left a comment

Copy link
Copy Markdown
Owner Author

Choose a reason for hiding this comment

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

Follow-up review of 07391179 against 140575da: the shared predicate fixes the previously posted pair/object container matrix. All earlier witnesses and expanded corpus 643 match Ruby 4.0.9 in normal, GC100, hash-index-off and MRO-cache-off modes.

[Major / P2 — remaining part of the container-equality finding] Apply the built-in exception comparison when both operands are materialized objects.

The selected semantic helpers still fall back to identity for two exception objects without a user-defined ==. The new routing therefore fixes pair/object combinations but leaves ordinary rescued exceptions inconsistent with their direct comparison:

begin
  raise RuntimeError, 'same'
rescue RuntimeError => bound
end
twin = bound.dup
p bound == twin
p [bound].include?(twin), [bound].count(twin)
h = {key: bound}
p h.value?(twin), h.key(twin), h == {key: twin}

Ruby 4.0.9 and the exact current-base artifact (140575da) print true, true, 1, true, :key, true. Head prints true, false, 0, false, nil, false. Array#index/rindex, Hash#has_value? and the inverse Hash#== fail too. This reproduces in all four modes. The ordinary rescue materialization introduced in this PR exposes the difference; the witness requires no TracePoint or custom equality method.

The shared predicate now chooses the semantic path, but rb_eq_w0 still uses identity for the plain VObj/VObj case, and member comparison does not consistently dispatch the inherited built-in Exception#==. Please carry the existing exception-aware comparison through those helpers for object/object operands, while preserving user-defined left-hand == behavior. Extending the matrix to pair/pair, pair/object, object/pair and object/object would cover the remaining representation combination.

An additional object/object check is worth retaining in that matrix: adding def twin.message = 'different' to the duplicate makes even bound == twin false on head, while Ruby/base keep it true. The scalar object's Exception#== arm still compares raw singleton class names in ours; the underlying-class normalization and stored-field semantics already implemented for pair/object comparison need to hold for two objects too.

I am reporting this as a continuation of the existing P2 rather than duplicating its inline finding. The earlier pair/object matrix is fixed; the object/object portion remains unresolved.

Validation: fresh O2 build with pinned Mere b92d7335, binary SHA-256 33f4eb8e649a342785e0db06ab8eca9e2857541767f5c2b2e3958a1ccdd977d8; reference/base controls; prior reproductions, both container matrices and corpus 643 in all four modes with the ivar index checker enabled. Method-write, fresh generated-C GC-root, name-length, duplicate-definition and dead-definition guards pass. Full hosted gates are still pending.

…equal -- identical; the same class (through the singleton) compares what both hold, stored message and backtrace, pairs and objects alike (exc_same_class_equal); another class through #exception -- and an exception object answers == in every container path (obj_answers_eq, rb_eq_w0), so a rescued exception and its dup are == inside Arrays and Hashes too (pair/object matrix in 643)

@284km 284km left a comment

Copy link
Copy Markdown
Owner Author

Choose a reason for hiding this comment

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

Follow-up review of d3c88aed against unchanged base 140575da: the remaining exception-equality P2 is fixed. No new actionable findings in this update.

The shared stored-field comparison now works for pair/pair, pair/object, object/pair and object/object operands. The ordinary rescued exception and its duplicate compare equally both directly and through Array/Hash operations, including include?, count, index/rindex, value?/has_value?, key and equality in both directions. Singleton presentation-method overrides preserve the underlying class and do not run during same-class comparisons; user-defined left-hand == still takes precedence.

Independent validation:

  • Fresh O2 native build with pinned Mere b92d7335; binary SHA-256 fdbd6458a2e87fdfae7f15d3f431c7982691a3dd05b8a9676d98f8ed2862ff4b.
  • 40 focused runs all match Ruby 4.0.9: earlier reproductions, the container matrices, ordinary rescued object/object comparisons, singleton-message overrides, differing class/message/backtrace checks, the separate #exception conversion path and expanded corpus 643.
  • Every case ran in normal, GC100, hash-index-off and MRO-cache-off modes with the ivar index checker enabled, with no diagnostics.
  • Method-write, fresh generated-C GC-root, name-length, duplicate-definition and dead-definition guards pass. Base comparisons retain the verified exact 140575da CI artifact.

There is no remaining code-review blocker from my reported findings. The current-head Linux/resource, corpus and reference CI jobs are still running; these focused checks do not establish that the full hosted gates pass. Please wait for the required current-head CI to finish successfully before merging.

This branch has not been deployed

No deployments
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.

1 participant