An each over a fresh String Array whose answer is read stores each mutated element back

[amatsuda]

Spinel refused three common shapes where a String yielded from a fresh Array is mutated in place, reporting the String as "not yet shared by reference". In all three, CRuby's answer is reproduced by mutating a copy, or by writing the element back:

• actionview TextHelper#highlight: text.scan(/<[^>]*|[^<]+/).each do |segment| … segment.gsub!(…) … end.join (the each answer is read).
• actionview split_paragraphs: text.to_str.gsub(…).split(/\n\n+/).map! { |t| t.gsub!(…) || t }.
• rack MediaType.parse_http_accept_header: parts = header.to_s.split(','); parts.map! { |part| part.strip!; … }.

A "fresh Array" here is the block-less result of split/scan/lines/chars on a String, typed as a String Array: nothing else holds it or its Strings.

1. A String mutated through a fresh Array nothing reads back is not refused (promote_shared_stored_strings's two share_route_defer sites):

   • A fresh temporary is accepted when the iterator's answer can't carry the mutated elements back: its value is dropped, or the iterator is map/collect or map!/collect!.
   • A local bound to a fresh Array is accepted for map!/collect! when all of these hold:
     • it's only ever bound to fresh Arrays, with every write before the map!;
     • there's no ||=, += or multiple-assignment write;
     • nothing reads it before the map! ends;
     • no loop encloses it, and it runs in the local's own scope.

   each with its answer used, select, find, each_with_object and the rest stay refused.

2. An each over a fresh String Array whose answer is read stores each mutated element back: an in-fixpoint desugar rewrites FRESH.each { |x| BODY } to FRESH.map! { |x| BODY; x } when the answer is used, the block has one plain parameter it mutates in place, and the block has no next/redo/retry and never rebinds the parameter. map! also answers the Array, so it's the same program, and commit 1 accepts it.

The test test/fresh_string_array_element_mutation.rb covers the three app shapes, plus:

• an each with its answer dropped;
• a mutating map;
• an each whose answer is printed;
• lines.each(&:chomp!).

Its expected output comes from CRuby. Three new refusal tests in make reject-test keep the observable cases refused:

• a = s.split; a.each(&:strip!); p a;
• a select;
• a read of the local before map!.

The 801 corpus tests matching strbuf/shared/string/mutat/bang/strip/each/map/scan/split all pass, as do make share-strings-test and make reject-test.

This textually conflicts with #8014, since both insert code just before promote_shared_stored_strings; keep both blocks.

🤖 Generated with Claude Code

Summary by CodeRabbit

• New Features
  • String mutations within supported iteration patterns over newly created string arrays are now handled correctly.
• Bug Fixes
  • Unsafe mutations are rejected when a local array is read before mutation, modified in a loop, or used in unsupported iteration patterns.
  • Added coverage for additional cases that must be rejected.

[coderabbitai]

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: cf84a488-d21f-46d3-aaa3-a397e344f200

📥 Commits

Reviewing files that changed from the base of the PR and between 70ff7a3 and e87cb0b.

📒 Files selected for processing (7)

• Makefile
• src/analyze.c
• test/fresh_string_array_element_mutation.rb
• test/fresh_string_array_element_mutation.rb.expected
• test/reject/string_split_local_each_mutation.rb
• test/reject/string_split_map_bang_read_before.rb
• test/reject/string_split_select_mutation.rb

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

---

📝 Walkthrough

Walkthrough

Compiler analysis now handles eligible in-place mutations of strings from fresh String arrays. The change adds conditions for rewriting iterator blocks and for allowing local map! mutations, with executable examples and rejection tests.

Changes

Fresh String Array Mutation

Layer / File(s)	Summary
Compiler rewrite and mutation checks src/analyze.c	Compiler analysis recognizes fresh split, scan, lines, and chars arrays. Eligible each blocks are rewritten to map!; local map! and collect! cases are checked for fresh-array writes, reads, loops, and scope.
Mutation examples and rejection tests test/fresh_string_array_element_mutation.rb, test/fresh_string_array_element_mutation.rb.expected, test/reject/string_split_local_each_mutation.rb, test/reject/string_split_map_bang_read_before.rb, test/reject/string_split_select_mutation.rb, Makefile	Executable examples cover mutations of strings yielded from fresh arrays. Rejection fixtures cover split-array mutation cases, and reject-test includes the listed fixtures in its compilation-refusal checks.

Priority: ⬇️ Low

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

Change: Bug fix

Merge Risk: ⚪ Minimal · up to e87cb

The change lets the compiler accept a constrained set of in-place mutations to strings from freshly split or scanned arrays. Other cases are still refused, and new example and rejection tests cover both outcomes. No blocking issues remain.

Security Architecture Review

Security architecture risk: 🔵 Low · up to e87cb

The change does not add an exposed service endpoint. An ownership gap remains: an iteration block can retain another reference to an element before mutating it, while the new rewrite preserves the array result without establishing equivalent behavior for that reference.

Retained concerns

• Low · architecture · inferred: The new ownership exception proves freshness before iteration but does not exclude aliases created inside the block. A block can assign its parameter to another local before mutating it; alias promotion excludes unshared block parameters, while the rewritten map! updates the array slot after block completion. Accepted programs may therefore expose inconsistent String contents through the retained reference, including during abrupt termination.

Security review details

Security Blast Radius

• inferred — The identified exposure is within programs compiled using the newly accepted mutation shapes. The evidence does not establish a deployed consumer, tenant boundary, privileged sink, or independently attackable environment for the ownership concern.

Trust Boundaries and Controls

• observed — The rewrite’s exclusion predicate checks early-iteration controls and writes to the parameter itself, but not assignment of that parameter to another local. Existing alias promotion explicitly excludes unshared block parameters, so it does not supply the missing ownership proof for that path.

Hardening Proposals

• proposed — Make the exception conditional on a proof that the block does not expose aliases to the mutated parameter, or preserve shared-handle semantics for escaping elements and retain refusal where that cannot be established.

🚥 Pre-merge checks | ✅ 5

✅ Passed checks (5 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title accurately describes the main change: qualifying each operations over fresh String Arrays store each mutated element back when the result is read. It is specific and related to the pull re…
Docstring Coverage	✅ Passed	Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 3 functions across 4 files. (3 skipped: 2 …
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.

✨ Finishing Touches 💡 1

⚔️ Resolve merge conflicts 💡

• Resolve merge conflict in branch pr/share-refusal-unobservable-element-mutation

• Autopilot · 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.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

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