A String pushed into an Array a Hash hands out is shared with its iteration

[amatsuda]

Strings in an Array held by a Hash value lost their in-place mutations (strip!, upcase!, <<). The block changed a copy, with no refusal: silent wrong output. CRuby strips every value below; Spinel left " 1".

h = { "k" => [+" 1"] }
h.each { |k, vs| vs.each(&:strip!) }

webrick's HTTPUtils.parse_header builds headers like this, then runs header.each { |key, values| values.each(&:strip!) }. A POST's Content-Length kept its leading space and failed webrick's digits check with 400 invalid content-length request header. Its continuation lines (header[field][-1] << " " << $1) were dropped too.

There are two root causes, one commit each, each with its test:

1. An Array read boxed out of a Hash value (promote_shared_stored_strings, element-iterator loop): stored Strings mutated by an iterator block were made shared handles only when the receiver was a local Array or a literal. Receivers like h.each { |k, vs| vs.each(&:strip!) }, each_value, h[k].each and vs = h[k]; vs.each were skipped. They now demand the stored Strings, as the poly-Array path already does: strbuf_demand_container_stores for a local, and strbuf_container_source_walk for a call.
   • Test: test/hash_value_array_element_bang.rb.
2. A String pushed into an Array that a Hash hands out (strbuf_demand_container_stores_here): the demand walk followed stores into the Hash itself, but not the << into the Array that h[k] returns. That covers header[field] = [] then header[field] << value, and (h[k] ||= []) << s (IndexOrWriteNode now joins sb_store_nodes).
   • Test: test/hash_element_push_each_bang.rb, a webrick-style parse_header with a continuation line.

Both tests' expected output comes from CRuby, and both print the unstripped values on master. Of the 1041 corpus tests matching strbuf/shared/string/mutat/bang/strip/each/hash_, all pass except array_callforms_strbuf_cycle, which passes when rerun alone (a load flake). make share-strings-test and make reject-test pass.

Not covered: store shapes this walk still doesn't follow, e.g. h.store(k, arr).

🤖 Generated with Claude Code

Summary by CodeRabbit

• Bug Fixes
  • Improved string-sharing analysis for strings stored in hash value arrays and for indexed ||= assignments, including cases involving boxed values and element iteration.
• Tests
  • Added examples covering string mutations in hash value arrays, continuation lines in parsed headers, and in-place whitespace removal.

[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: 0054ccd3-b7a1-42ba-a614-0de788ee3840

📥 Commits

Reviewing files that changed from the base of the PR and between 2773fd6 and 3b5a8ef.

📒 Files selected for processing (5)

• src/analyze.c
• test/hash_element_push_each_bang.rb
• test/hash_element_push_each_bang.rb.expected
• test/hash_value_array_element_bang.rb
• test/hash_value_array_element_bang.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

The string-sharing analysis now considers indexed ||= stores and traces values through polymorphic element iteration. Ruby examples cover in-place string mutations in hash value arrays and parsed header values.

Changes

Hash value mutation analysis

Layer / File(s)	Summary
Track indexed stores src/analyze.c	The store index now includes IndexOrWriteNode entries. Nested-store analysis checks assigned values and traces values stored in an inner container.
Trace polymorphic element iteration src/analyze.c	For nonliteral TY_POLY receivers with TY_POLY block parameters, analysis waits until inference is no longer optimistic, then traces values from local receivers or follows call receiver source expressions.
Exercise hash-stored string mutations test/hash_element_push_each_bang.rb, test/hash_element_push_each_bang.rb.expected, test/hash_value_array_element_bang.rb, test/hash_value_array_element_bang.rb.expected	Examples mutate strings in hash value arrays and parse header values into arrays. Expected output records the resulting values and hashes.

Priority: ➖ Normal

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

Change: Bug fix

Merge Risk: ⚪ Minimal · up to 3b5a8

The changed paths preserve the string mutations exercised by the new examples. No merge-blocking issue remains after normal checks.

🚥 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 describes the core string-sharing behavior involving a String pushed into an Array returned by a Hash and later observed during iteration. It is specific to the changeset, although the wordi…
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 1 functions across 2 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.

• 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.}

[amatsuda]

Found a compile-time regression in this branch on large programs (a 33k-line program goes from ~7 min to not finishing in 20 min). Working on a fix; please hold off merging until it's pushed here.

🤖 Generated with Claude Code

[amatsuda]

This merged before the fix for the compile-time regression landed here; the fix is in #8010 (a 33k-line program: 391 s with it, against not finishing in 1200 s on master now).

🤖 Generated with Claude Code