Ignore empty native candidates in boxed return freshness

[dchuk]

What this changes

Loading a native class such as StringIO adds empty rows to the polymorphic candidate index. Freshness analysis treats those placeholders as possible method_missing implementations and native method arms, so an otherwise fresh user-method result can be refused under --share-strings.

Skip a native candidate only when it has no Ruby method, inherited reader, or native binding. Use the same predicate for the dynamic-hook scan and called-name scan, including the boxed String proof. Real native methods and method_missing hooks remain conservative.

Generic tests load StringIO and mutate a fresh String returned by a boxed user call, and retain the refusals for inherited method_missing implementations and native-class method_missing readers.

Validation

Stacked on #8420, including its exact attested commit d02b51ec9fb1. The eight-path native-specific patch is byte-identical to the previously reviewed patch; the singleton-accessor guard remains ahead of native filtering.

Full make -j6 gate CC=gcc-12 GATE_CACHE=1 TEST_JOBS= BENCH_PJOBS=1 passed on Linux x86_64, merged with master 4412e698feb4. Head c0d5945467cc carries the verified Gate trailer for tested tree 9145b082f74d; attestation changed no source or compiler bytes.

Optcarrot: OK
rubyspec-gate[language]: all 1340 expected-PASS examples still pass
Benchmarks: 70 pass, 0 fail, 0 error, 0 skip
rubyspec-gate[core/array]: all 819 expected-PASS examples still pass
rubyspec-gate[core/string]: all 878 expected-PASS examples still pass
rubyspec-gate[core/hash]: all 300 expected-PASS examples still pass
rubyspec-gate[core/integer]: all 347 expected-PASS examples still pass
rubyspec-gate[core/range]: all 210 expected-PASS examples still pass
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)
gate-test-shared: known ERR: string_plain_mutator_result_kept
gate-test-shared: known ERR: yield_string_mutator_tail
gate-test-shared: 6816 pass, 2 known failures, 0 new failures
Tests: 6818 pass, 0 fail, 0 error
gate: ALL GREEN

• Seven focused sharing-verifier programs on this combined tree pass with zero diagnostics, failures, or channel gaps. Inherited method_missing and reader refusal fixtures pass in the full gate; the real native borrowed-binding control remains conservative.

• Optcarrot generated C is byte-identical to master4412 (616,836 bytes; SHA-256 c05a6c722851314f695b908536097947f324d9c430fb81d5749b42d71b4adefa); checksum 59662 and all six scale ratios match.

• Both listed shared-corpus failures produce exactly the clean baseline's Discussion: share a mutable String by default, and copy only where the analysis proves no one else sees the change #6765 refusals. No known-failure entry was changed.

• The separate verifier is not globally green: five known affected cases match the clean master4412 log after only temporary-path normalization. Both exit 1 with two unratcheted exception-channel diagnostics, eight stale entries, one net_http_zlib generated-C parity failure, and 10 channel gaps. No ratchet entry was added or waived; unobserved channels are not proven safe.

• Final-head hosted clang CI and Gate verification pass for c0d5945467cc. CodeRabbit completed a full review of that exact head with no actionable comments and zero review threads. Other platform lanes are skipped by the PR workflow. Its generic docstring-coverage warning remains disclosed; it is not represented as passing.

• New tests have .expected files matching CRuby 4.0 with frozen string literals; refusal tests carry # spinel: not-cruby

• No added test uses values past 2^31

• Full make gate, Optcarrot C, and scale comparison against the same master

• Depends on: Compose boxed String builtin freshness with user returns #8420

[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: 7bdbdb9f-b906-4eb1-b578-47820fd2751d

---

---

---

📥 Commits

Reviewing files that changed from the base of the PR and between 4412e69 and c0d5945.

---

---

---

📒 Files selected for processing (22)

• docs/limitations.md
• src/analyze_share.c
• src/call_plan.c
• src/call_plan.h
• test/share/boxed_pure_user_arm_keeps_nested_argument.rb
• test/share/boxed_pure_user_arm_keeps_nested_argument.rb.expected
• test/share/boxed_sub_class_value_return_keeps_nested_string.rb
• test/share/boxed_sub_class_value_return_keeps_nested_string.rb.expected
• test/share/boxed_sub_fresh_with_user_arm.rb
• test/share/boxed_sub_fresh_with_user_arm.rb.expected
• test/share/boxed_sub_transitive_class_accessor_keeps_nested_string.rb
• test/share/boxed_sub_transitive_class_accessor_keeps_nested_string.rb.expected
• test/share/boxed_sub_transitive_explicit_class_ivar_keeps_nested_string.rb
• test/share/boxed_sub_transitive_explicit_class_ivar_keeps_nested_string.rb.expected
• test/share/boxed_sub_user_arms_run_both.rb
• test/share/boxed_sub_user_arms_run_both.rb.expected
• test/share/native_placeholder_fresh_user_return.rb
• test/share/native_placeholder_fresh_user_return.rb.expected
• test/share/refuse/native_placeholder_inherited_method_missing.rb
• test/share/refuse/native_placeholder_inherited_method_missing.rb.expected
• test/share/refuse/native_placeholder_method_missing_reader.rb
• test/share/refuse/native_placeholder_method_missing_reader.rb.expected

---

---

---

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

---

---

---

---

📝 Walkthrough

📝 Walkthrough

📝 Walkthrough

Walkthrough

The change updates shared-String analysis for boxed dispatch. It records and revalidates evidence before composing eligible builtin and user-method returns. It also refines native-placeholder and method-missing checks, adds regression tests, and documents analysis limitations.

Changes

Boxed String dispatch analysis

Layer / File(s)	Summary
Resolve and record boxed dispatch proofs src/call_plan.c, src/call_plan.h, src/analyze_share.c	Adds an uncached poly-plan resolver and records boxed-fresh evidence, including dispatch targets and types, for later validation.
Compose boxed builtin and user returns src/analyze_share.c, test/share/boxed_*.rb, test/share/boxed_*.expected, docs/limitations.md	The share walk joins builtin and user returns when the boxed-row composability proof succeeds. Tests cover nested arguments and sub calls with user-defined return cases. Limitation notes describe native candidates, fresh empty Hash returns, and boxed dispatch.
Filter candidates and validate freshness src/analyze_share.c, test/share/native_placeholder_*.rb, test/share/native_placeholder_*.expected, test/share/refuse/native_placeholder_*.rb, test/share/refuse/native_placeholder_*.expected	Freshness checks distinguish empty native placeholders from candidates with bindings, readers, methods, or method-missing routes. Regression tests cover placeholder and method-missing cases.

Priority: ➖ Normal

Estimated code review effort: 4 (Complex) | ~45 minutes

Change: Bug fix

Sequence Diagram(s)

sequenceDiagram
  participant ShareWalk
  participant CallPlan
  participant BoxedRowProof
  participant UserTargets
  ShareWalk->>CallPlan: Resolve dispatch plan
  ShareWalk->>BoxedRowProof: Check boxed-row composability
  BoxedRowProof->>UserTargets: Validate user arms against plan targets
  ShareWalk->>UserTargets: Join reachable user returns when proof succeeds

Loading

---

---

---

---

Merge Risk: ⚪ Minimal · up to c0d59

No actionable issue was identified in the boxed String freshness change; it is ready to merge after normal checks.

Security Architecture Review

Security architecture risk: 🔵 Low · up to c0d59

Eligible calls can now return strings that may be mutated without a sharing refusal. Reviewed safeguards preserve conservative handling of ambiguous calls, but coverage does not extend to every affected path.

Retained concerns
No architecture-level concerns identified.

Security review details

Security Blast Radius

• inferred — The reviewed exposure is compiled-program string ownership: source-defined calls, methods, and accessors influence freshness decisions. An incorrect proof could permit mutation of shared data, but the inspected paths did not establish credential, tenant, service, or infrastructure authority expansion.

Trust Boundaries and Controls

• observed — Composition refuses dynamic lookup, method_missing routes, singleton accessors, competing builtin faces, and unaccounted returning arms. Reachable user-return holders remain joined, and user-call freshness separately requires every reachable user target to return fresh data.

Resilience and Maintainability Implications

• observed — Missing or changed recorded evidence refuses that proof rather than authorizing freshness. Invalid fresh-plan node IDs clear prior arm storage and count, preventing reuse of another call's arms through that failure path.

---

---

---

Pre-merge checks | 4 | 1

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Docstring Coverage		Docstring coverage is 19.05% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 42 functions across 12 files. (10 skipped…	Write docstrings for the functions missing them to satisfy the coverage threshold.

✅ Passed checks (4 passed)

Check name	Status	Explanation
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.
Description Check		Check skipped - CodeRabbit’s high-level summary is enabled.
Title check		The title clearly and concisely describes the main change: excluding empty native candidates from boxed return freshness analysis.

---

Full details: Docstring Coverage

Explanation

Docstring coverage is 19.05% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 42 functions across 12 files. (10 skipped: 10 unsupported.)

---

• Fix all pre-merge checks with AI

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

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

---

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

[dchuk]

@coderabbitai full review

[coderabbitai]

✅ Action performed

Full review finished.

[dchuk]

@coderabbitai full review

[coderabbitai]

✅ Action performed

Full review finished.

[dchuk]

@coderabbitai full review

[coderabbitai]

✅ Action performed

Full review finished.