Repository navigation
The write pass's widening walks share what they found unchanged, as the parameter pass's do - #10179
Conversation
…he parameter pass's do infer_write_container_usage follows a value of another kind pushed into a boxed local or element back to the arrays it can hold (widen_boxed_array_sources), through every method its calls can resolve to and each method's values in turn. The walk's memo of the visits that changed nothing lasted one walk, so the next store of the same scope walked the same web of methods again. The pass now shares the memo across its walks the way infer_param_types_ex does: a generation ends wherever the pass reports a change, at each node, and as a walk begins when a change was reported since the last step. SPINEL_WBAS_SHADOW=1 passes on the related corpus tests and on a large program. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01VXZndU8ST2PpoRWZ4iiejs
|
No actionable comments were generated in the recent review.
 |
| Layer / File(s) | Summary |
|---|---|
Coordinate traversal generations src/analyze_pass.c |
The write pass shares traversal tracking across stores and returns changes accumulated by the shared traversal. The traversal wrapper advances or synchronizes generations at outermost calls. Comments describe how parameter binding and write-pass stores affect generation changes. |
Priority:
Estimated code review effort: 2 (Simple) | ~10 minutes
Change: Refactor
Possibly related issues
- Storing into a Hash with a non-literal key types an unrelated Struct field #10016: The PR changes shared traversal behavior in the write pass, but the available summary does not establish whether it fixes the reported non-literal-key Hash write.
Suggested reviewers: matz
Merge Risk: 
9c788
-
No actionable risk introduced by this change remains after the review. The PR is mergeable subject to normal checks.
Pre-merge checks: 
-
Check name Status Explanation Description Check Check skipped - CodeRabbit’s high-level summary is enabled. Title check The title clearly describes the main change: sharing unchanged widening-walk results in the write pass, following the parameter pass pattern. It is specific and related to the pull request. Docstring Coverage No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check. Docstring coverage is scoped to functions touched by this diff. Analyzed 0 functions across 0… 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
No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check. Docstring coverage is scoped to functions touched by this diff. Analyzed 0 functions across 0 files. (1 skipped: 1 too large.)
-
- 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.
The write pass's widening walks share what they found unchanged, as the parameter pass's do.
What happened
infer_write_container_usagefollows a value of another kind pushed into a boxed local or element back to the arrays it can hold, widening them where they are built (widen_boxed_array_sources). A boxed receiver's arrays are reached through every method its calls can resolve to, and each method's values through theirs, to depth 6, so one walk fans out over the methods of a common name; the walk keeps a memo of the visits that changed nothing, but each walk from the pass opened a generation of its own (wbas_touchat depth 0), so the next store of the same scope walked the same web again.On a Rails application, in the rounds after the optimistic ones settle (where the walk runs), the walk was 33% and 42% of two sampled fixpoint windows and the whole of a third, all of it the recursion
widen_boxed_array_sources<->widen_method_value_sources.The fix
The pass shares the memo across its walks the way
infer_param_types_exalready does (wbas_share_begin/wbas_share_step/wbas_share_end): a generation ends wherever the pass reports a change, at each node it visits, and now also as a walk begins when the pass reported a change since its last step, so no walk reads a record left before a write it has not seen. A visit skipped on the strength of the shared record is one that changed nothing in the same state, as before within a walk.SPINEL_WBAS_SHADOW=1, which makes every skipped visit all the same and aborts if it would have widened or written, passes on the Active Support sample (C identical to the unshadowed build's) and on the related corpus tests.Measurements
Rails application (
--timing, same machine, the runs alongside each other):(The pass runs again in the post-fixpoint and backstops phases, which is where the rest of its time was.)
The application stops on the same deferred refusals, and its refusal output is identical line for line to master's. The Active Support sample's C is byte-identical to plain master's. 703 related corpus tests pass locally, under the shadow check as well; the gate check passes.
🤖 Generated with Claude Code
https://claude.ai/code/session_01VXZndU8ST2PpoRWZ4iiejs
Summary by CodeRabbit