Repository navigation
Array#index is Array's, not Enumerable's - #122
Open
jiro-by-nori[bot] wants to merge 1 commit into
Open
jiro-by-nori[bot] wants to merge 1 commit into
jiro-by-nori[bot] wants to merge 1 commit into
Conversation
The prelude aliased `index` to `find_index` inside `module Enumerable`, so
every Enumerable answered a call CRuby refuses: `(1..5).index(3)` returned 2,
`{a: 1}.index(1)` returned nil, and a user class including the module searched
its elements -- all where CRuby 4.0.5 raises `NoMethodError: undefined method
'index' for an instance of Range`. A `NoMethodError` sits in the family the
validator claims to exclude, so those were wrong answers, not gates, and the
reverse of the gated-miss work in #27/#29/#31.
CRuby defines `#find_index` on `Enumerable` and `#index` only on `Array`, as an
alias of `Array#find_index`. The name now lives on `Array`:
* the block form still searches (`[1,2,3].index { |x| x > 1 }` is 1);
* the arity error is CRuby's, `given 2, expected 0..1`, where the blockless
builtin said `expected 1`;
* a call with no argument and no block gates (it would need an Enumerator)
instead of raising that `ArgumentError`;
* `Enumerable#find_index` is untouched, and `Range#index` and any user
Enumerable now reach the ordinary miss path and raise CRuby's message.
`rb_ary_index` walks the array's own storage, never `each`. This body is Ruby
and reads through `each`, so a receiver that defines `each` (a subclass or a
singleton method) would search something CRuby never looks at; it gates rather
than answering. No program in either corpus is affected: conformance is
unchanged at 1087 agree / 194 declined.
`difftest/corpus/regressions/array-index-is-array-only` pins the surface
against CRuby 4.0.5.
Closes #116.
Co-authored-by: Jiro <contact@tilework.tech>
Co-authored-by: Nori <contact@tilework.tech>
Array#index is Array's, not Enumerable'sArray#index is Array's, not Enumerable's
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What was wrong
The prelude installed
indexonEnumerable:CRuby defines
#find_indexonEnumerableand#indexonly onArray(asArray#find_index), so every including class answered a call CRuby refuses:(1..5).index(3)2NoMethodError: undefined method 'index' for an instance of Range{a: 1}.index(1)nilNoMethodError: … for an instance of Hashclass C; include Enumerable; def each; yield 7; end; endthenC.new.index(7)0NoMethodError: … for an instance of CNoMethodErroris inside the family the validator claims to exclude, so thesewere wrong answers rather than gates — the inverse of the gated-miss work in
#27/#29/#31.
The change
ruby-lean/prelude/prelude.rb: the name moves ontoclass Arrayand theEnumerablealias is deleted.Array#indexis written out rather than aliased,because the blockless forms have to keep answering:
[1,2,3].index(2)is1,[1,2,3].index { |x| x > 1 }is1;ArgumentError: wrong number of arguments (given 2, expected 0..1). The blockless builtin saidexpected 1, and the aliaswould have turned the same call into a gate;
Enumerator), asEnumerable#find_indexalready does, instead of raising thatArgumentError;==on the element, and the array is read live, asrb_ary_indexdoes;Enumerable#find_indexis untouched, andRange#indexand any userEnumerable now reach the ordinary miss path and raise CRuby's message.
One deliberate narrowing:
rb_ary_indexwalks the array's own storage and nevercalls
each, but this body is Ruby and reads througheach. A receiver thatdefines
each(a subclass or a singleton method) would therefore searchsomething CRuby never looks at, so it gates with
Array#index on a receiver that defines #eachinstead of answering. No program in either corpus is affected.Making
Array#indexa prelude definition also shadows the blocklessArray#indexbuiltin (ruby-lean/RubyCore/Builtins/Collections.lean), which isnow unreachable; this PR leaves it in place so the model's boot heap, its guards
and the proofs over it are untouched. Removing the dead arm is a follow-up.
difftest/corpus/regressions/array-index-is-array-only.{rb,json}pins the wholesurface against CRuby 4.0.5, status
fixed: the Array forms, the 0..1 aritymessage,
Range/Hash/user-Enumerable misses,Enumerable#find_index,==comparison and live reading.
Test plan
Run in this session against CRuby 4.0.5 (ruby 4.0.5p0) with Lean
v4.32.2. Nothing was weakened: no baseline file needed a new number.make gen-checkPrelude.lean/PreludeJson.leanare in this PR)make booksBuild completed successfully (1399 jobs)— every proof book, including the type-soundness theorem and its controlsmake metatheorypropext,Classical.choice,Quot.sound; the booted heap satisfies the assumptions the proofs make about itmake soundnessSOUNDNESS CHECK PASSEDmake conformanceagree 1087(baseline ≥1087),sut_unsupported 194(≤194),not_compared 6(≤6),disagree 0; regressions and tier 3 agreemake difftest-testCOVERAGE OKmake desugar-testagree: 1257 disagree: 0 out-of-fragment: 76, desugaring-rule coverage 76/76make desugar-coveragebaseline: in-fragment=1211 (of 1282) current=1211 (of 1282)COVERAGE OKCOVERAGE OKmake book-checks72/72 inputs: CRuby, the model and the theorem agreemake feature-loadingAGREElines (requirescope, re-entry, cache; throw/ensure; failed-load dependency)make check-not-weakenedCHECKS NOT WEAKENEDmake ci-sync,make prereqsmake checktarget; every prerequisite presentruff check ./ruff format --check .Ad-hoc, before/after, with
bin/ruby-lean --compare:(1..5).index(3)printed2,{a: 1}.index(1)printednil—DIFFERagainst CRuby;Array#indexshapes(
[[1],[2]].index([2]),[1].index(1.0),[].index(1), a block that matchesnothing, …) print identical output and exit
AGREE.Environment notes, so the numbers above are reproducible:
json, whose native extension does notbuild against this box's
zig cc(ld.lld: relocation R_X86_64_PC32 cannot be used against symbol '__cpu_model'), sobundle installran withBUNDLE_WITHOUT=developmentand the two minitest suites were run with plainruby -e 'Dir.glob(…)'plus the samescripts/check-coverage.pycheck;rubocopwas installed outside the bundle and run over the whole tree(
43 files inspected, no offenses detected). Neitherruby-lean/prelude/nor
difftest/corpus/is in RuboCop'sIncludelist, so this change adds nolint surface;
make comparator(fetches and rebuildsleanprover/comparator, and the theorem statement is unchanged) andmake docs(mkdocs is not installed on this box and no documentationchanged).
Authored by Jiro, an autonomous coding agent, on behalf of @sam-xif. The
code, the regression case and this description were written by an agent; every
result above was produced by running the commands shown in this session.