The inherited-hook pass finds classes, Structs and modules through an index built once

[FrancescoK]

Probed at master 61dedaeac, macOS, Apple clang 21.

make scale-test's "work at 4x the program" ratio rose from 4.93 to 5.00 after #8353 (Fire inherited hooks from extended modules and Struct.new blocks), which touched only the inherited-hook pass in src/. The pass was already K x n0 before that change; #8353 doubled the constant.

--emit-rbs work count of test/scale/gen.sh	100 units	400 units	ratio (limit 5.20)
master	520,394,636	2,603,762,384	5.00
this branch	512,292,948	2,480,293,656	4.84
the compiled-to-C leg of scale-test			6.27 -> 6.15 (limit 6.90)

• desugar_inherited_hooks calls inh_chain_has_hook once per class with a superclass (K classes in the scale program). Each call scans all n0 nodes for every ancestor step, so the walk is K x n0 whether or not the program has a hook anywhere. Inherited hooks fire from an extended module and a Struct.new block #8353 put inh_struct_body (an nt_kind per node) ahead of the NK_ClassNode test of that scan, which doubled the constant and moved the ratio.
• The same holds for inh_module_hook, which scans all nodes for every extend argument, for the module loop (every module with a hook scans every node and calls inh_extended_hook on each), and for the once-per-class set of qualified names (a linear list searched per class).
• Found by bisecting the ratio to the merge of Inherited hooks fire from an extended module and a Struct.new block #8353.

One index per pass. desugar_inherited_hooks builds a hashed index (ANameHash, as compute_reachable does for called names) once: for a class or Struct constant name, whether a body of it defines inherited or extends a module that does, and the superclass name its first class gives; for a module name, the def inherited of the first module of that name that has one. An ancestor walk (inh_chain_has_hook) is then one lookup per link, still at most 64 links and the first superclass each name gives. The module loop reads, per module, the number of bodies that extend it and whether one of them sits below a class with a hook, counted in a single pass over the program instead of a scan per module. inh_module_hook and inh_extended_hook become inh_module_def_hook (one module) and inh_extended_entry (the module a body extends, through the index). The once-per-class set of qualified names is an ANameHash as well. When no class, Struct or module of the program has a hook, the pass returns before it builds the parent table: with no hook there is no super to turn into nil and no call to place. The index lives in a local and is freed at the end; there is no new global or file-scope table.

The answers are the scans' answers. A name's hook is the OR over all its definitions, which the scan also returned at the first match; the superclass is the first class of the name in node order that gives a constant superclass, which is what !super_name kept; the module's hook is the first module of that name that has one, which inh_module_hook returned. The tests of the old loops keep their order and arguments (a super is still read with nt_str(sc, "name") without a kind test in the two places that had none). The tree edits the pass makes (super to nil in a hook body, a call prepended to a class body, cloned nodes past n0) change none of the facts the index holds.

Cost. Work count of the pass alone (g_nt_work around the call), master -> branch:

program	master	branch
gen.sh 100	8,244,292	142,604
gen.sh 400	124,020,592	551,864
padded hooks, 50 units	4,998,478	52,324
padded hooks, 200 units	259,468,453	208,534
padded hooks, 800 units	15,669,473,353	835,401

The padded program has per unit a hook module, a class under a class with a hook, a class that extends the module, a class under that, a Struct block with a hook and a class two levels down. The branch is linear in it (4.0x per 4x). Whole --emit-rbs work on it: 50 units 30,953,669 -> 26,007,515, 200 units 590,216,718 -> 330,956,799; the rest of the total there is other passes (class structure), not covered here.

The corpus runs come from the local make gate below (this branch merged with master 123eaf323, macOS arm64, clang 21); the corpus-wide C diff was not run.

Name test. The one sp_streq(dn, "inherited") test in the pass is the old inh_module_hook's, moved unchanged into the index build.

Tests. test/class_inherited_hook_index.rb (new) pins the shapes the index answers as the scans did: a module reopened after a copy without the hook, several extends on one class, a reopened class (one call), hooks above and below one another, a Struct block with extend, class << self, and a module that no class extends. It passes on master as well (the change is not a behaviour change) and on this branch plain, with --int-overflow=promote, with --share-strings, and under SPINEL_GC_STRESS=1; test/class_inherited_extend_struct.rb and test/class_inherited_hook.rb pass the same four ways. The generated C of 1,126 programs (every test/*.rb and test/share/*.rb that mentions inherited, Struct.new, Data.define, extend or a subclass, benchmark/*.rb, and optcarrot) is byte-identical to master in the default build and with --share-strings, and so is that of four further shape programs for the module, extend and chain cases. In that gate make test passes (6835 pass, 0 fail, 0 error) and the corpus with sharing on has 6833 pass, 2 known failures (already listed in test/share/known-failures.txt) and 0 new failures.

make gate (on this branch merged with current master)

Local make gate on this branch merged with master 123eaf323 (macOS arm64, clang 21); the head commit carries its Gate trailer.

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 4.84x (linear 4.00, limit 5.20)
scale-test: work at 4x the program, compiled to C, is 6.15x (limit 6.90)
scale-test: call-shape work at 4x the units, compiled to C, is 4.13x (linear 4.00, limit 4.50)
Tests:     6835 pass,        0 fail,        0 error
gate-test-shared: known ERR: string_plain_mutator_result_kept
gate-test-shared: known ERR: yield_string_mutator_tail
gate-test-shared: 6833 pass, 2 known failures, 0 new failures
gate: stamp for tree 78326273cd93 on master 0fc287c2f43f; git commit --amend --no-edit adds the Gate: trailer
gate: ALL GREEN

• New tests have .expected files that match CRuby 4.0 run with --enable-frozen-string-literal
• Values past 2^31 are marked # spinel: int64
• If optcarrot's generated C changed: callgrind numbers, checksum 59662
• Depends on: #

🤖 Generated with Claude Code

Summary by CodeRabbit

• Bug Fixes
  • Improved handling of inherited hooks across class and module inheritance chains, including hooks that call super and classes reopened in multiple places.
  • Ensured hooks from modules are considered when those modules are extended, while unextended module hooks remain unaffected.
• Tests
  • Added coverage for inheritance chains, Struct subclasses, nested constants, and extension order.

[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: 1f554dd9-c219-41ff-8ba4-14302284e81f

---

📥 Commits

Reviewing files that changed from the base of the PR and between 108d210 and a2aa1c4.

---

📒 Files selected for processing (3)

• src/analyze_desugar.c
• test/class_inherited_hook_index.rb
• test/class_inherited_hook_index.rb.expected

---

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

---

---

📝 Walkthrough

Walkthrough

The inherited-hook desugaring pass now builds an index of module, class, and Struct hook information. It uses indexed lookups to process hooks, check superclass chains, and deduplicate inserted inherited-hook calls. New tests cover inheritance, module extension, reopenings, and related cases.

Changes

Inherited-hook desugaring

Layer / File(s)	Summary
Build the hook index src/analyze_desugar.c	The pass creates indexed module-hook lookups and records class and Struct hook status, superclass names, and module extenders.
Process hooks and verify cases src/analyze_desugar.c, test/class_inherited_hook_index.rb, test/class_inherited_hook_index.rb.expected	Hook processing uses indexed lookups and superclass-chain checks. An ANameHash deduplicates qualified class names. New fixtures and expected output cover inheritance, module extensions, reopened definitions, and nested constants.

Priority: ⬇️ Low

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

Change: Refactor

---

Merge Risk: ⚪ Minimal · up to a2aa1

This change speeds up inherited-hook processing in the compiler and is reported to produce identical generated C. It adds regression fixtures for inheritance and extension-order cases. No concrete defects were identified, so it appears ready to merge.

Pre-merge checks | 4 | 1

❌ Failed checks (1 inconclusive)

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

✅ Passed checks (4 passed)

Check name	Status	Explanation
Description Check		Check skipped - CodeRabbit’s high-level summary is enabled.
Title check		The title accurately identifies the main change: the inherited-hook pass now uses one index to find classes, Structs, and modules instead of repeated scans.
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

Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 10 functions across 1 files. (2 skipped: 1 unsupported, 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.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

---

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