fix(codegen): guard cyclic inheritance walks against non-termination - #4845
Merged
Conversation
Several codegen helper walks over the class `extends_name` chain assumed the inheritance graph is a tree. Heavily-modular TypeScript packages (Effect, OpenCode) declare same-named classes across modules; when those are pulled into one importing module's class table by name, the parent chains can form a cycle (local `Base` → extends `Class` → resolves back to local `Base`). The unbounded `while let Some(name) = cur` walks then either CPU-hang (counter walks) or OOM (accumulating walks), which is what stalled the OpenCode native-graph codegen after `visited=N/N`. Add a visited-set + depth cap (64) to the inheritance-chain walks in: - is_class_getter / is_class_setter (escape-analysis accessor dispatch) - class_uses_this_as_value + parent-field-init walk - class_chain_extends_builtin_error - declared_field_type - class_field_global_index (recursive walk and its inner parent-count loop) - class_field_declared_type Each walk now terminates on a repeated class name instead of treating the parent chain as a guaranteed tree. Focused regression tests cover the accessor, this-as-value, and builtin-error walks under a 2-cycle graph. The three class-field helpers (declared_field_type, class_field_global_index, class_field_declared_type) move to a new sibling module `type_analysis_class_fields.rs` so type_analysis.rs stays under the 2000-line file-size gate; they're re-exported from `type_analysis` so call sites are unchanged. Salvaged from the still-relevant core of #4261 (h/t @andrewtdiz); the rest of that branch is a stale OpenCode-migration grab-bag, superseded on main.
proggeramlug
force-pushed
the
guard-cyclic-inheritance-walks
branch
from
June 9, 2026 15:20
e922a9c to
2c8f6c7
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What changed
Add a visited-set + depth cap (64) to the codegen helper walks over the class
extends_nameinheritance chain, so they terminate on cyclic parent links instead of assuming the graph is a tree:is_class_getter/is_class_setter(escape-analysis accessor dispatch)class_uses_this_as_value+ its parent-field-initializer walkclass_chain_extends_builtin_errordeclared_field_typeclass_field_global_index(the recursive walk and its inner parent-count loop)class_field_declared_typePlus focused regression tests for the accessor, this-as-value, and builtin-error walks under a 2-class cycle.
The three class-field helpers (
declared_field_type,class_field_global_index,class_field_declared_type) move into a new sibling moduletype_analysis_class_fields.rssotype_analysis.rsstays under the 2000-line file-size gate (it was already at 1997). They're re-exported fromtype_analysis, so call sites are unchanged.Why
Heavily-modular TypeScript packages (Effect, OpenCode) declare same-named classes across modules. When those are pulled into a single importing module's class table by name, the parent chains can form a cycle — e.g. local
Base → extends "Class"and importedClass → parent "Base"resolving back to the localBase. Every chain-walking site here used an unboundedwhile let Some(name) = curloop, so a single cycle would CPU-hang the counter walks or OOM the accumulating ones. This is what stalled the OpenCode native-graph codegen right after it reachedvisited=N/N.The walks now bail on a repeated class name. The deeper name-collision problem (two distinct classes sharing a name across modules) is orthogonal and out of scope — this is purely the termination guard.
Provenance
Salvaged from the still-relevant core of #4261 (h/t @andrewtdiz). The rest of that branch is a stale OpenCode-migration grab-bag now 420+ commits behind
mainand largely superseded, so it's been closed in favor of this focused fix. The guard logic was re-applied on top of currentmain(thetype_analysis.rsportion in particular, since #4261's copy of that file predated a lot of unrelated work).class_field_declared_typehad an unguarded walk that #4261 didn't touch — guarded here too.Validation
cargo test -p perry-codegen cyclic— 3 tests pass.cargo test -p perry-codegen --lib— 92 pass, 0 fail.cargo fmt --all -- --checkclean../scripts/check_file_size.sh— passes (type_analysis.rs back under 2000 lines).