Skip to content

A String Range's % answers an Enumerator that inspects as %(n) - #8468

Merged
matz merged 1 commit into
matz:masterfrom
FrancescoK:range-mod-label
Oct 11, 2026
Merged

matz merged 1 commit into
matz:masterfrom
FrancescoK:range-mod-label

Conversation

@FrancescoK

@FrancescoK FrancescoK commented Oct 11, 2026 •

Copy link
Copy Markdown
Contributor

Probed at master 8aeee4bb5, macOS, Apple clang 21.

r = ("a".."e")
p r % 2
p r.step(2)
p r % 1.5
p((r % 1.5).to_a)
CRuby 4.0 Spinel
p r % 2 #<Enumerator: "a".."e":%(2)> #<Enumerator: "a".."e":step(2)>
p r.step(2) #<Enumerator: "a".."e":step(2)> the same
p r % 1.5 #<Enumerator: "a".."e":%(1.5)> #<Enumerator: "a".."e":step(1)>
(r % 1.5).to_a TypeError (no implicit conversion of Float into String) ["a", "b", "c", "d", "e"]
  • step with a Float (r.step(1.5)) answers step(1) and the members of a stride of 1, the same way: the row read the stride as an Integer, truncating it. CRuby builds the Enumerator without looking at the stride and raises when its walk adds the stride to a String.
  • desugar_str_range_methods (src/analyze.c) renamed every range % n on a String Range to step, because the arithmetic emitter had no arm for it. That left the builtin-op row for % (whose label is %(n)) unreachable.
  • Found next to the String Range step Enumerator fix, whose row text carries both labels.

The fix. The blockless % keeps its name, and emit_array_arith_call (src/codegen_call.c) hands a String Range to the row ahead of its numeric arms, which would read a Float stride as Float arithmetic. The poly-operand arm of src/codegen_call_operator.c skips a String Range (as it does a Time): under --int-overflow=promote a stride variable is boxed, and the arm took range % n for Integer arithmetic. The block form, range % n { }, is still renamed to step: it is the step walk and has no row of its own.

A Float stride. Both rows (src/builtin_ops.c) call sp_srange_step_enum (lib/spinel_rt.h), which takes the stride boxed, so a Float held in a boxed slot is seen as well as a Float-typed one. An Integer stride materializes the stepped members as the rows did. A Float stride answers an Enumerator labeled with the call as written (%(1.5), step(0.5)) whose walk is a generator raising CRuby's TypeError, so to_a, next, first, each and the rest raise it, and its size is nil. sp_enum_inspect prints the receiver of a generator Enumerator whose creator recorded one, where it printed a Generator placeholder for every generator; that is a change to an existing line of lib/spinel_rt.h, whose natural home is that check, not a helper beside it. The block forms, r.step(1.5) { } and r.%(1.5) { }, raise the TypeError at the call, as CRuby does.

The corpus runs come from the local make gate below (this branch merged with master 0fc287c2f, macOS arm64, clang 21); the corpus-wide C diff was not run.

One existing runtime line changes. sp_enum_inspect (lib/spinel_rt.h) printed a placeholder for any generator Enumerator (if (e->gen || e->gen_label)); it now prints the receiver a generator Enumerator recorded, so the new step and % Enumerators inspect as CRuby's #<Enumerator: "a".."e":%(1.5)>. Adding a second inspect path for the same object would duplicate the function; the existing branch is where the receiver belongs.

Tests. test/srange_percent_label.rb covers inspect, an exclusive Range, a variable stride, map, first, next, the block forms ((r % 2).each, r.%(2) { }, r.step(2) { }), a Range passed through a method, and a Float stride (literal, variable and boxed) through inspect, size, to_a, next, first, each, each_with_index and the block forms. It is marked # spinel: share and # spinel: gc-stress; it fails on master and passes in both builds, plain, with --int-overflow=promote, and under SPINEL_GC_STRESS=1 and 2, on macOS arm64 and with gcc -O1 on Linux. In that gate make test passes (6844 pass, 0 fail, 0 error) and the corpus with sharing on has 6843 pass, 1 known failure (already listed in test/share/known-failures.txt) and 0 new failures.

Not covered, also on master:

  • r % n and r.step(n) on an Integer or Float Range answer an Array where CRuby answers an ArithmeticSequence (((1..10).%(3))).
  • A stride of 0 or less (("a".."e") % 0, step(-1)) raises ArgumentError when called; CRuby answers an Enumerator whose walk yields only the first member (and loops for step(0) { }).
  • r.%(2, &blk) raises NoMethodError for step; a String Range held in a boxed slot (z = [r, 1][0]; z % 2) raises NoMethodError for %; the % and step of an endless String Range (("a"..) % 3) print as a Generator.
  • r + x, r - x and r * x on a String Range with a boxed operand raise TypeError naming an Array where CRuby raises NoMethodError for the Range.

make gate (on this branch merged with current master)

Local make gate on this branch merged with master 0fc287c2f (macOS arm64, clang 21); the head commit carries its Gate trailer.

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 5.00x (linear 4.00, limit 5.20)
scale-test: work at 4x the program, compiled to C, is 6.27x (limit 6.90)
scale-test: call-shape work at 4x the units, compiled to C, is 4.15x (linear 4.00, limit 4.50)
Tests:     6844 pass,        0 fail,        0 error
gate-test-shared: known ERR: yield_string_mutator_tail
gate-test-shared: 6843 pass, 1 known failures, 0 new failures
gate: stamp for tree bed77af5c031 on master 108d210f1445; 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
  • If optcarrot's generated C changed: callgrind numbers, checksum 59662
  • Depends on: #

🤖 Generated with Claude Code

Summary by CodeRabbit

  • New Features
    • String range % and step operations now return enumerators that yield every nth member, with support for runtime-provided strides.
    • String range enumerators provide clearer inspection labels and support standard iteration and collection operations.
    • Float strides produce lazy enumerators; advancing them raises a TypeError.
    • Nonpositive integer strides raise an ArgumentError.

`("a".."e") % 2` inspected as `#<Enumerator: "a".."e":step(2)>` where CRuby
prints `%(2)`. desugar_str_range_methods renamed every `range % n` on a
String Range to `step` because the arithmetic emitter had no arm for it,
which left the builtin-op row for `%` (whose label is `%(n)`) unreachable.

The blockless `%` keeps its name now, and the arithmetic emitter hands a
String Range to that row, ahead of the poly-operand arm, which took a stride
of boxed type (any Integer variable under --int-overflow=promote) for
Integer arithmetic. The block form, `range % n { }`, is still renamed: it is
the step walk, and has no row of its own.

A Float stride (`r % 1.5`, `r.step(0.5)`) was truncated to an Integer, so the
Enumerator inspected as `step(1)` and walked every member. Both rows now call
sp_srange_step_enum, which takes the stride boxed: an Integer stride
materializes the members as before, and a Float one answers an Enumerator
labeled `%(1.5)` whose walk raises CRuby's TypeError and whose size is nil.
sp_enum_inspect prints the receiver of a generator Enumerator that recorded
one.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Gate: green tree bed77af5c031 master 0fc287c (darwin-arm64 clang-21.0.0) tests 6844/0
@coderabbitai

coderabbitai Bot commented Oct 11, 2026 •

Copy link
Copy Markdown

Review in Change Stack →

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: b73f28fb-48ee-4cfe-bd47-9d4a34b9dab3

📥 Commits

Reviewing files that changed from the base of the PR and between 108d210 and dc0cfed.


📒 Files selected for processing (7)
  • lib/spinel_rt.h
  • src/analyze.c
  • src/builtin_ops.c
  • src/codegen_call.c
  • src/codegen_call_operator.c
  • test/srange_percent_label.rb
  • test/srange_percent_label.rb.expected

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



📝 Walkthrough

Walkthrough

String Range step(n) and blockless % (n) calls now use dedicated enumerator construction. Integer strides produce stepped members; Float strides defer a TypeError until iteration. Enumerator inspection and tests also change.

Changes

String Range step enumerators

Layer / File(s) Summary
Route blockless String Range calls
src/analyze.c, src/codegen_call.c, src/codegen_call_operator.c
Blockless % calls retain their form, and String Range percent operations route through builtin handling rather than numeric or poly-arithmetic paths.
Construct and inspect step enumerators
lib/spinel_rt.h, src/builtin_ops.c, test/srange_percent_label.rb, test/srange_percent_label.rb.expected
step and % use shared enumerator construction. Integer strides at or below zero raise ArgumentError; Float strides defer TypeError until iteration. Inspection uses the recorded source and method when available. Tests cover integer and Float strides, inspection, and block forms.

Priority: ⬇️ Low

Estimated code review effort: 3 (Moderate) | ~20 minutes

Change: Bug fix

Sequence Diagram(s)

sequenceDiagram
  participant Caller
  participant desugar_str_range_methods
  participant emit_array_arith_call
  participant builtin_ops
  participant sp_srange_step_enum
  participant Enumerator
  Caller->>desugar_str_range_methods: Preserve blockless percent call
  desugar_str_range_methods->>emit_array_arith_call: Pass String Range percent call
  emit_array_arith_call->>builtin_ops: Route to String Range builtin
  builtin_ops->>sp_srange_step_enum: Pass range, stride, and method
  sp_srange_step_enum->>Enumerator: Construct labeled Enumerator
Loading

Suggested reviewers: matz


Merge Risk | ⚪ Minimal · up to dc0cf

Merge Risk: ⚪ Minimal · up to dc0cf

This change makes blockless String Range percent calls keep their label and defers Float-stride errors until iteration. No actionable merge-blocking risk was identified.

Security Architecture Review

Security architecture risk: 🟡 Moderate · up to dc0cf

This change alters when Range operations report errors and introduces a potential memory-safety regression while constructing their results. Exposure is limited to affected running programs in the available evidence; a remotely exploitable path has not been established.

Retained concerns

  • High · security · inferred: The new shared helper registers its fresh Enumerator as a GC root only after formatting the stride label. Both integer and Float formatting allocate strings, whose allocator can initiate object collection. The Enumerator is not retained by the already-rooted inputs, so collection at this point can make the later source/label writes access reclaimed memory. The base templates protected the Enumerator before formatting. This introduces a native object-lifetime hazard in ordinary blockless String Range step and percent calls; exploitation and application-level reachability remain unverified.
Security review details

Security Blast Radius

  • inferred — The identified lifetime hazard applies to native programs invoking blockless String Range step or percent, including integer strides, when label allocation initiates collection. Its potential impact is memory safety within the executing process. No additional privilege is required by these language operations, but network reachability, tenant exposure and practical exploitability depend on application use that was not supplied.

Security Findings and Attack Paths

  • inferred — A caller reaches the new helper through ordinary Range operations. After allocating an Enumerator, stride formatting can cross the string-heap collection threshold while that object is unrooted. Collection can reclaim an unreachable object, after which source and method-label attachment writes through the retained pointer. The base step template rooted the object before formatting. This supports the introduced lifetime concern, without establishing a remote attack path or code-execution exploit.

Trust Boundaries and Controls

  • observed — The routed attempt and stride declarations are test helpers, not production authority-bearing entrypoints. Production changes route language operands through receiver-specific builtin handling and boxed type checks. The concrete concern is object-lifetime protection, rather than a demonstrated authentication, tenant or privileged-resource boundary bypass.

Resilience and Maintainability Implications

  • observed — Once an Enumerator is reachable, its GC scan retains items, fiber, capture, source and method label. The new helper also roots its receiver, stride and temporary arrays. These are relevant protections for dependencies and subsequent iteration, but none provides a reverse reference retaining the fresh Enumerator during pre-root label formatting.

Pre-merge checks | Passed 4 | Inconclusive 1

❌ Failed checks (1 inconclusive)

Check name Status Explanation Resolution
Docstring Coverage Inconclusive Docstring coverage is 66.67% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 3 functions across 3 files. (4 skipped: 1… 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 summarizes the main change: blockless String Range % returns an Enumerator that inspects with the %(n) label.
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 66.67% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 3 functions across 3 files. (4 skipped: 1 unsupported, 3 too large.)


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

Warning

Some tools did not complete. Review the errors below.

🔧 ast-grep (0.45.3)
src/builtin_ops.c

ast-grep timed out on this file



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

Comment @coderabbitai help to get the list of available commands.

@github-actions github-actions Bot added the gate: verified The head commit's Gate trailer names the tree its merge with master gives label Oct 11, 2026
@matz
matz merged commit b8db3ca into matz:master Oct 11, 2026
6 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

gate: verified The head commit's Gate trailer names the tree its merge with master gives

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants