Skip to content

Commit cdcd493

Browse files
committed
Guard forward walker against exponential blowup on deep loop nesting
1 parent d00ac65 commit cdcd493

6 files changed

Lines changed: 222 additions & 71 deletions

File tree

docs/ARCHITECTURE.md

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1051,6 +1051,13 @@ Each worker is created during `initialized` via `clone_for_diagnostic_worker`, w
10511051

10521052
Non-`Arc` fields are snapshotted at spawn time: `php_version`, `vendor_uri_prefixes`, `vendor_dir_paths`, and `config`. These fields are only written during `initialized` (before the workers are spawned) and never change afterwards. If a future feature adds hot-reloading of `.phpantom.toml` or runtime PHP version changes, the workers would need to be notified or re-cloned. This invariant ("init-time fields are write-once") should be verified before adding any post-init mutation to these fields.
10531053

1054+
## Forward Walker
1055+
1056+
The forward walker (`src/completion/variable/forward_walk.rs`) walks
1057+
method bodies top-to-bottom, building a scope of variable types at
1058+
each statement boundary. It is shared by diagnostics, completion,
1059+
hover, go-to-definition, and signature help.
1060+
10541061
## Name Resolution
10551062

10561063
PHP class names go through resolution at parse time (`resolve_parent_class_names`):

docs/CHANGELOG.md

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -19,6 +19,10 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
1919
- **Import all missing classes.** A bulk code action that imports every unresolved class name in the file at once. Only names with a single unambiguous candidate are imported; ambiguous names are left for manual resolution via the single-class import action. Short-name conflicts are detected and skipped. Appears in the quick-fix menu when the cursor is on an unresolved class name and the file has two or more unresolved names.
2020
- **Context-aware import candidate filtering.** Import class actions now filter candidates by the syntactic context of the reference. After `implements` only interfaces are offered, after trait `use` only traits, after `extends` only classes or interfaces (as appropriate). This eliminates wrong-kind suggestions and reduces ambiguity for both single and bulk imports.
2121

22+
### Fixed
23+
24+
- **Analyzer and LSP no longer hang on files with deeply nested loops.** Files with multiple levels of foreach/while/for inside if-branches caused exponential blowup in the forward walker's two-pass loop strategy. A unified loop-depth guard now bounds re-walks regardless of code path (diagnostics, completion, hover), preventing hangs on previously stuck files across three test projects.
25+
2226
### Changed
2327

2428
- **Rewritten variable resolver.** Variable type resolution now uses a single top-to-bottom pass through each function body on both the diagnostic and completion paths, with zero recursion and no depth limit.

docs/todo.md

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -25,6 +25,7 @@ within the same impact tier.
2525

2626
| # | Item | Impact | Effort |
2727
| --- | ------------------------------------------------------------------------------------------------------------------------------------------------------------------- | -------- | ----------- |
28+
| P20 | [Forward walker: Mago-style assignment-depth-bounded loop iteration](todo/performance.md#p20-forward-walker-mago-style-assignment-depth-bounded-loop-iteration) | High | Medium |
2829
| ER5 | [Mago-style separated metadata](todo/eager-resolution.md#er5--mago-style-separated-metadata) (pre-populated immutable codebase, O(1) method resolution) | High | High |
2930
| P9 | [`resolved_class_cache` generic-arg specialisation](todo/performance.md#p9-resolved_class_cache-generic-arg-specialisation) | Medium | Medium |
3031
| P18 | [Subtype result caching](todo/performance.md#p18-subtype-result-caching) (per-request HashMap for hierarchy walks) | Medium | Low |

docs/todo/bugs.md

Lines changed: 2 additions & 25 deletions
Original file line numberDiff line numberDiff line change
@@ -357,31 +357,6 @@ chosen, every URI stored in `ast_map`, `class_index`, `fqn_index`,
357357
absolute `file:///` URIs so that lookups and cache eviction work
358358
correctly regardless of how the tool was invoked.
359359

360-
## B21 — `shared/src/database/Model/Products/Product.php` analysis hangs
361-
362-
**Symptom:** `phpantom_lsp analyze --project-root shared -- src/database/Model/Products/Product.php`
363-
does not complete within 60 seconds (debug build). The other two
364-
baseline files (`BaseCsvProductUpdaterTemplate.php` and `Klarna.php`)
365-
complete in under 5 seconds. This was reproducible before and after
366-
the T25 backward-scanner removal, so it is not a regression from
367-
that work.
368-
369-
**Likely cause:** The Product model is a large Eloquent model with
370-
many relations, scopes, accessors, and virtual members. The
371-
diagnostic pass likely triggers deep resolution chains (e.g.
372-
Builder generic substitution, relation return types, virtual member
373-
synthesis) that multiply into excessive work. A single expensive
374-
method body with many member-access diagnostics could account for
375-
the hang.
376-
377-
**Where to fix:** Profile the analysis of this file to identify the
378-
hot path. Likely candidates: `resolve_target_classes` called
379-
repeatedly for the same subject within a single method body,
380-
`type_hint_to_classes` resolving the same generic type repeatedly
381-
without caching, or `inheritance::merge` being called redundantly
382-
for the same class. The `resolved_class_cache` (P9) may also help
383-
if generic-arg specialisation is implemented.
384-
385360
---
386361

387362
## B22 — `$this` resolves in static methods
@@ -454,3 +429,5 @@ statement. Possible causes:
454429

455430
Once the root cause is understood, the re-entry guard can be removed
456431
in favour of a proper fix.
432+
433+

docs/todo/performance.md

Lines changed: 45 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -772,6 +772,51 @@ resolution pipeline. See
772772

773773
---
774774

775+
## P20. Forward walker: Mago-style assignment-depth-bounded loop iteration
776+
777+
The forward walker's two-pass loop strategy (walk body once to
778+
discover assignments, merge, walk again) interacts multiplicatively
779+
with if-branch merging. Currently a `LOOP_DEPTH` counter hard-caps
780+
re-walks at depth 4 and bails out entirely at depth 8. This
781+
prevents hangs but sacrifices accuracy on deeply nested code.
782+
783+
Mago (`references/mago/crates/analyzer/src/statement/loop/`) solves
784+
this properly with bounded iteration:
785+
786+
1. **Assignment map.** Before analyzing a loop body, a cheap AST
787+
walk (no type resolution) builds a map of which variables depend
788+
on which other variables. The loop body is re-walked at most
789+
`assignment_depth` times (typically 1–3), not proportional to
790+
nesting depth.
791+
792+
2. **Fixed-point early exit.** After each re-walk, check whether any
793+
variable's type actually changed. If types have stabilized, stop.
794+
795+
3. **Type widening.** Integer bounds widen (e.g. `int<0,5>`
796+
`int<0,max>`) to ensure convergence in few iterations.
797+
798+
4. **Single-pass branch merging.** If/elseif/else branches are each
799+
walked exactly once. Results are merged via type union into an
800+
accumulator. Nesting an if inside a foreach does not multiply
801+
walks.
802+
803+
### Fix
804+
805+
- Replace the `LOOP_DEPTH` hard cap with assignment-dependency-depth
806+
counting.
807+
- Add fixed-point detection: compare scope before/after a re-walk
808+
and stop when no types changed.
809+
- Consider type widening for integer literal types inside loops.
810+
811+
### When to implement
812+
813+
After the forward walker stabilizes (it is less than a week old).
814+
The current `LOOP_DEPTH` guard is sufficient to prevent hangs; this
815+
item improves accuracy on deeply nested loops without risking
816+
exponential blowup.
817+
818+
---
819+
775820
## Appendix: Profiling
776821

777822
### Commands

0 commit comments

Comments
 (0)