Repository navigation
reverse_each with a block walks a boxed user Enumerable last to first - #10177
Conversation
A user object that includes Enumerable, read back out of a container (a
TY_POLY receiver), ran the block of `reverse_each { }` zero times and
answered the object: the reverse walk read the length and elements of the
object itself, which has neither.
The boxed reverse walk now reads the receiver's elements first, as its
to_a answers them (sp_poly_enum_chk, in iter_ewi_zip_poly_arms,
src/codegen_iter.c), as the blockless reverse_each already does, then walks
them from the end. A
class that defines its own reverse_each keeps it; an object with no #each
still raises NoMethodError naming reverse_each.
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
|
No actionable comments were generated in the recent review.
 |
| Layer / File(s) | Summary |
|---|---|
Boxed receiver iteration and coverage src/codegen_iter.c, test/poly_user_enum_reverse_each.rb, test/poly_user_enum_reverse_each.rb.expected |
The reverse_each path checks boxed receivers with sp_poly_enum_chk before obtaining the iteration subject. The gc-stress script exercises custom and built-in receivers, and the expected output records the results. |
Priority:
Estimated code review effort: 2 (Simple) | ~10 minutes
Change: Bug fix
Possibly related PRs
- matz/spinel#10164: Adds polymorphic block-taking Enumerable arms that obtain elements from Enumerable-including classes.
Merge Risk: 
5f45f
-
The change appears ready to merge after normal checks; no unresolved behavior issue was identified.
Pre-merge checks: 
-
Check name Status Explanation Resolution Docstring Coverage Docstring coverage is 25.00% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 8 functions across 2 files. (1 skipped: 1… Write docstrings for the functions missing them to satisfy the coverage threshold. -
Check name Status Explanation Description Check Check skipped - CodeRabbit’s high-level summary is enabled. Title check The title clearly and concisely describes the main change: boxed user Enumerables now support block-form reverse_each iteration from last to first. Linked Issues check Check skipped because no linked issues were found for this pull request. Out of Scope Changes check Check skipped because no linked issues were found for this pull request.
Full details: Docstring Coverage
-
Explanation
Docstring coverage is 25.00% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 8 functions across 2 files. (1 skipped: 1 unsupported.)
Resolution
Write docstrings for the functions missing them to satisfy the coverage threshold.
-
- Autofix · 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.
Comment @coderabbitai help to get the list of available commands.
What this changes
A user object that includes Enumerable, read back out of a container, ran
the block of
reverse_each { }zero times: on master,[Nums.new, 0][0].reverse_each { |x| print x }prints nothing where CRubyprints
321, and abreakinside it answers the object instead of thebreak value. The same holds for a class whose #each forwards its block
(
["C", "B", "A"]expected,[]measured). The blocklessreverse_each.to_aalready answers[3, 2, 1]. This came up in ourcompatibility testing against CRuby.
The boxed block walk of
reverse_each(iter_ewi_zip_poly_arms,src/codegen_iter.c) now reads the receiver's elements through
sp_poly_enum_chk, the helper the blockless form uses, beforesp_poly_iter_subject, so a user Enumerable is walked as the Array itsto_aanswers (what its #each yields, unless the class has ato_aofits own). Every other walk keeps
sp_poly_iter_walkand the check line isunchanged. A class that defines its own reverse_each keeps it; an object
with no #each still raises NoMethodError naming reverse_each.
The new test is
test/poly_user_enum_reverse_each.rb; on master:make gate(on this branch merged with current master).expectedfiles that match CRuby 4.0 run with--enable-frozen-string-literal# spinel: int64(not applicable: none)Validation
Branch
5f45f24e6on38313140d, merged for the make gate withorigin/master38313140d; expectations from CRuby 4.0.7 with--enable-frozen-string-literal. The generated C of 17 of the 8,675 suite programs and benchmarks changes: the new test; six whose boxedreverse_eachblock walk now reads the receiver throughsp_poly_enum_chk(test/boxed_hash_each_rest_param.rb,test/boxed_rev_chain.rb,test/hash_reverse_each_answers_receiver.rb,test/poly_each_returns_receiver.rb,test/poly_iter_non_enumerable_object.rb,test/safe_nav_block_calls.rb), with the same output; and ten whose only change is the revision stringRUBY_DESCRIPTIONembeds or a source path. The new test passes at-O0,-O1and-O2, and underSPINEL_GC_STRESS=1.Left alone
to_ahas a boxedreverse_each { }walk whatto_aanswers where CRuby walks whateachyields; master does the same.reverse_each, a boxed Enumerable without one raises NoMethodError where CRuby walks it, and a class witheachbut noinclude Enumerablewalks underreverse_each { }where CRuby raises NoMethodError; master does the same.reverse_each { }raises NoMethodError, a boxed(1..)raises RangeError where CRuby raises TypeError, andredoin a boxedreverse_eachblock is refused at compile time; master does the same.each_with_index,map,each_with_objectandinjectbind them unlike CRuby (map { |x| x }answers[[1, 2], [3, 4]], CRuby[1, 3]); master does the same.Summary by CodeRabbit
reverse_eachbehavior for custom Enumerable objects and string ranges. Custom objects are iterated through their array results when applicable, while objects with their ownreverse_eachmethod continue to use it.