The proc-return passes ask only the calls that can reach a scope

[makenowjust]

Fixes #7397.

an_call_targets_scope can only answer yes for a call on the scope's own name, a call on a name some class aliases a method under, or new for an initialize. Sections (9) and (9b) of an_phase_proc_returns asked it for every call against every scope anyway.

Change

src/analyze.c only. A small index (PRCallIdx), built once a round before (9) and freed after (9b):

• the CallNodes in kind-chain order (the order the old loop visited them), with their names;
• those positions sorted by (name, position), and the positions whose name is an alias name;
• the named scopes sorted by (name, index).

(9) visits, for each scope, the calls on its name merged with the aliased calls (and the new calls for an initialize), in the original order, and still asks an_call_targets_scope about each. (9b) visits, for each call with a block, the scopes of its name (and the initializes for new) in ascending order, or every scope when the name is an alias name. The answers and the order of the updates are the same as before.

Results

Time in an_phase_proc_returns for the issue's generator, promote mode:

M	before	after
2,000	0.21 s	0.01 s
4,000	1.42 s	0.03 s
8,000	5.58 s	0.05 s
16,000	36.2 s	0.10 s

The 56k-line machine-generated program from the issue: 10.6 s -> 0.22 s; a 52k-line one: 2.48 s -> 0.14 s; a 141k-line one: 11.35 s -> 0.40 s (promote mode, phase time, with four other slow analysis passes disabled for the measurement).

The generated C does not change

• Test corpus: every test/*.rb (5,746 programs) compiled with spinel -c in both --int-overflow modes, on master (e5e8f79) and on this change, built in turn in the same checkout and fed the same relative paths: the generated C, stderr and exit status are identical for all 11,492 compilations except the 8 (4 programs, 2 modes each) that embed RUBY_DESCRIPTION, whose only difference is the revision string.
• After the rebase onto 92510d6: 156 programs, both modes, compiled the same way: no difference.
• Six machine-generated programs of 0.6k-56k lines: identical before and after, both modes.

Gate

scale-test: instance_eval forwarding work at 2x the wrappers is 1.71x (limit 2.50)
scale-test: work at 4x the program is 4.73x (linear 4.00, limit 5.20)
scale-test: work at 4x the program, compiled to C, is 6.08x (limit 6.90)
scale-test: call-shape work at 4x the units, compiled to C, is 4.22x (linear 4.00, limit 4.50)
Tests: 5928 pass, 0 fail, 0 error
gate: stamp for tree 30c5c214706d on master ab9b925aa333; git commit --amend --no-edit adds the Gate: trailer
gate: ALL GREEN

The stamp line above names the origin/master the clone had fetched when the gate finished (ab9b925); the tree the gate ran was based on 92510d6, as described below.

make gate was run on 92510d6 with this change merged together with other fixes from the same investigation (#7384, #7386, #7388, #7390, #7392 and others); the branch was then rebased onto ab9b925 without conflicts.

Promote mode

SPINEL_INT_OVERFLOW=promote make test on the same combined branch: Tests: 5909 pass, 15 fail, 13 error. All 28 failing tests are among the 29 that master fails in promote mode at e5e8f79; none is new. The 29th (raise_rejects_invalid_arguments) no longer fails, which appears to come from master moving from e5e8f79 to 92510d6.

Summary by CodeRabbit

• Performance
  • Improved analysis efficiency when evaluating call targets, parameter bindings, and yield sites. The results of these checks continue to be validated before they are applied.

[coderabbitai]

Navigate logical layers of code changes, visualize relationships, and explore their blast radius.

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: a3655d82-38ea-48f7-95ff-39e18eea8a1b

📥 Commits

Reviewing files that changed from the base of the PR and between ab9b925 and 111b6d0.

📒 Files selected for processing (1)

• src/analyze.c

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

---

📝 Walkthrough

Walkthrough

The procedure return analysis builds a temporary index of calls and named scopes. The parameter-bound and yield-site passes use indexed candidates instead of scanning all calls or scopes. Existing checks still determine which candidates apply.

Changes

Procedure return analysis indexing

Layer / File(s)	Summary
Build call and scope candidate index src/analyze.c	Adds a temporary index that selects candidate calls and scopes by name. The index accounts for aliased calls and the new/initialize pairing.
Use candidates in analysis passes src/analyze.c	The parameter-bound pass uses indexed candidate calls. The yield-site pass uses indexed candidate scopes. Both retain their existing applicability checks.

Priority: ➖ Normal

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

Change: Refactor · Severity of issue fixed: Medium

Suggested reviewers: matz, francescok, elektronaut

Merge Risk: ⚪ Minimal · up to 111b6

No actionable issue is identified for this change. It is mergeable after normal checks.

Architecture Summary

Architecture risk: 🔵 Low · up to 111b6

The change affects 1 system.

Changed systems: src

Architecture concerns
No architecture-level concerns identified.

Review details

Systems and components

• observed — src (service) was modified; 1 changed file maps to changed impact.

Before / after behavior

• observed — Modified behavior in src/analyze.c: Adds a temporary index of call nodes and named scopes, with name-sorted lookup, ascending deduplicated candidate merges, and cleanup. Candidate selection includes calls with aliased names; new and initialize are paired in the relevant lookup direction, while aliased calls can reach any scope.
• observed — Modified behavior in src/analyze.c: Builds the call index once before iterating named scopes with parameters. Replaces scanning every call node for each such scope with the indexed candidate calls; the existing target-scope check still filters candidates.
• observed — Modified behavior in src/analyze.c: Replaces scanning every scope for each call with the indexed candidate scopes. Aliased calls iterate all non-top scopes; other calls iterate the scopes selected by name, with the existing yield and target checks retained.

🚥 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 clearly summarizes the main change: limiting proc-return passes to calls that can reach a scope.
Linked Issues check	✅ Passed	Issue #7397 requests reducing the calls-times-scopes work in the two an_phase_proc_returns re-derivations. The change builds a temporary call/scope index and limits each pass to candidate calls or s…
Out of Scope Changes check	✅ Passed	The reported change is confined to src/analyze.c. The temporary index and candidate selection directly address issue #7397. The reported tests and output comparisons validate that optimization; no u…
Docstring Coverage	✅ Passed	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…

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