Skip to content

Commit 6e735b7

Browse files
AJenboAJenbo
committed
Update roadmap
1 parent 224ec96 commit 6e735b7

3 files changed

Lines changed: 314 additions & 111 deletions

File tree

‎docs/todo.md‎

Lines changed: 13 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -23,12 +23,14 @@ within the same impact tier.
2323

2424
## Sprint 5 — Polish for office adoption
2525

26-
| # | Item | Impact | Effort |
27-
| --- | ------------------------------------------------------------------------------------------------------------------------------------------------------------------- | -------- | ----------- |
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 |
29-
| ER5 | [Mago-style separated metadata](todo/eager-resolution.md#er5--mago-style-separated-metadata) (pre-populated immutable codebase, O(1) method resolution) | High | High |
30-
| P9 | [`resolved_class_cache` generic-arg specialisation](todo/performance.md#p9-resolved_class_cache-generic-arg-specialisation) | Medium | Medium |
31-
| P18 | [Subtype result caching](todo/performance.md#p18-subtype-result-caching) (per-request HashMap for hierarchy walks) | Medium | Low |
26+
| # | Item | Impact | Effort |
27+
| --- | ------------------------------------------------------------------------------------------------------------------------------------------------------------------- | ----------- | ----------- |
28+
| P26 | [Chain resolution cache only active during diagnostics](todo/performance.md#p26-chain-resolution-cache-only-active-during-diagnostics) | Medium | Low |
29+
| P24 | [`IN_ARRAY_KEY_ASSIGN` re-entry guard in forward walker](todo/performance.md#p24-in_array_key_assign-re-entry-guard-in-forward-walker) | Low | Low |
30+
| P20 | [Forward walker: bounded iteration and depth-cap cleanup](todo/performance.md#p20-forward-walker-bounded-iteration-and-depth-cap-cleanup) | High | Medium |
31+
| ER5 | [Mago-style separated metadata](todo/eager-resolution.md#er5--mago-style-separated-metadata) | High | High |
32+
| P9 | [`resolved_class_cache` generic-arg specialisation](todo/performance.md#p9-resolved_class_cache-generic-arg-specialisation) | Medium | Medium |
33+
| P18 | [Subtype result caching](todo/performance.md#p18-subtype-result-caching) (per-request HashMap for hierarchy walks) | Medium | Low |
3234
| D4 | [Unused variable diagnostic](todo/diagnostics.md#d4-unused-variable-diagnostic) | Medium | Medium |
3335
| D12 | [Mago diagnostic proxy](todo/diagnostics.md#d12-mago-diagnostic-proxy) | Medium | Medium |
3436
| F4 | [Return type and closure parameter type inlay hints](todo/lsp-features.md#f4-return-type-and-closure-parameter-type-inlay-hints) | Medium | Medium |
@@ -58,15 +60,14 @@ within the same impact tier.
5860
| # | Item | Impact | Effort |
5961
| --- | --------------------------------------------------------------------------------------------------------------------------------------------------------------- | ----------- | ----------- |
6062
| | Clear [refactoring gate](todo/refactor.md) | — | — |
61-
| E5 | [Extension stub coverage audit](todo/external-stubs.md#e5-extension-stub-audit) | Medium | Low |
63+
| E5 | [Extension stub coverage audit](todo/external-stubs.md#e5-extension-stub-selection-stubs-extensions) | Medium | Low |
6264
| X1 | [Staleness detection and auto-refresh](todo/indexing.md#x1-staleness-detection-and-auto-refresh) | Medium | Medium |
63-
| E1 | [External stub packages (ide-helper, etc.)](todo/external-stubs.md#e1-external-stub-packages) | Medium-High | Medium |
64-
| E2 | [Project-level stubs as type resolution source](todo/external-stubs.md#e2-project-level-stubs-as-type-resolution-source) (depends on E1) | Medium | Medium |
65+
| E1 | [External stub packages (ide-helper, etc.)](todo/external-stubs.md#e1-project-level-phpstorm-stubs-for-gtd) | Medium-High | Medium |
66+
| E2 | [Project-level stubs as type resolution source](todo/external-stubs.md#e2-project-level-stubs-as-resolution-source) (depends on E1) | Medium | Medium |
6567
| E3 | [IDE-provided and `.phpantom.toml` stub paths](todo/external-stubs.md#e3-ide-provided-and-phpantomtoml-stub-paths) (depends on E2) | Low-Medium | Low |
66-
| E4 | [Stub version alignment with target PHP](todo/external-stubs.md#e4-stub-version-alignment) (depends on E1) | Medium | Medium |
68+
| E4 | [Stub version alignment with target PHP](todo/external-stubs.md#e4-embedded-stub-override-with-external-stubs) (depends on E1) | Medium | Medium |
6769
| L11 | [Relation dot-notation string and column name string completion](todo/laravel.md#l11-relation-dot-notation-string-completion-and-column-name-string-completion) | Medium-High | Medium-High |
68-
| P1a | `type_hint_to_classes` returns `Vec<Arc<ClassInfo>>` (memory prerequisite for X4) | Low | Low |
69-
| X4 | [Full background indexing](todo/indexing.md#x4-full-background-indexing) (workspace symbols, fast find-references; depends on P1a) | Medium | High |
70+
| X4 | [Full background indexing](todo/indexing.md#x4-full-background-indexing) (workspace symbols, fast find-references) | Medium | High |
7071
| L1 | [Facade completion](todo/laravel.md#l1-facade-completion) | High | High |
7172
| | **Release 1.0.0 + IDE extensions** | | |
7273

@@ -169,8 +170,6 @@ unlikely to move the needle for most users.
169170
| P10 | [Redundant `parse_and_cache_file` from multiple threads](todo/performance.md#p10-redundant-parse_and_cache_file-from-multiple-threads) | Medium | Low |
170171
| P11 | [Uncached base-resolution in `build_scope_methods_for_builder`](todo/performance.md#p11-uncached-base-resolution-in-build_scope_methods_for_builder) | Low-Medium | Low |
171172
| P3 | Parallel pre-filter in `find_implementors` | Low-Medium | Medium |
172-
| P19 | [Analysis time dominated by per-expression class resolution](todo/performance.md#p19-analysis-time-dominated-by-per-expression-class-resolution) (root cause; fix is ER5) | High | High |
173-
| P1b | Propagate `Arc<ClassInfo>` through variable-resolution pipeline | Low | Medium |
174173
| P4 | `memmem` for block comment terminator search | Low | Low |
175174
| P5 | `memmap2` for file reads during scanning | Low | Low |
176175
| P6 | O(n²) transitive eviction in `evict_fqn` | Low | Low |

‎docs/todo/eager-resolution.md‎

Lines changed: 103 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -243,6 +243,103 @@ inside the type resolution pipeline, not by resolution logic.
243243

244244
---
245245

246+
#### Phase 4e: Eliminate recursion guards in class resolution (absorbs P21, P22)
247+
248+
**Impact: Medium-High. Effort: Medium.**
249+
250+
Once Phase 4d (or earlier) makes the resolved codebase metadata
251+
immutable after population, the re-entrant resolution that currently
252+
requires thread-local recursion guards cannot occur. This phase
253+
removes those guards and the depth caps they protect:
254+
255+
1. **`RESOLVING` set and `MAX_RESOLVE_DEPTH` (30) in
256+
`virtual_members/mod.rs`.** Thread-local set of class FQNs
257+
currently being resolved. When a class is already in the set,
258+
re-entrant calls return a partial result (base inheritance only,
259+
no virtual members). This produces non-deterministic results:
260+
whichever class in a mutual dependency (e.g. Model/Builder) is
261+
resolved first gets full virtual members, the other gets degraded.
262+
After eager population, all classes are resolved before any
263+
consumer queries them, so re-entry cannot happen.
264+
265+
2. **`RESOLVE_DEPTH` and `MAX_RESOLVE_TARGET_DEPTH` (60) in
266+
`completion/resolver.rs`.** Thread-local depth counter for
267+
`resolve_target_classes_expr_inner`. Guards against mutual
268+
recursion between subject resolution, call-return resolution,
269+
and variable resolution. The limit of 60 (vs typical chain
270+
depth of 5-10) indicates the recursion is caused by accidental
271+
re-entry into class resolution, not by the problem size. Once
272+
class resolution is a cache lookup, this re-entry path vanishes.
273+
274+
3. **LSP server eager population.** The `analyse.rs` CLI already
275+
runs `populate_from_sorted` before diagnostics. The LSP server's
276+
Tokio threads do not. Extend eager population to run on file
277+
change in the LSP server (incrementally, not full re-population)
278+
so that interactive requests also benefit from pre-resolved
279+
metadata.
280+
281+
**How the reference projects avoid this problem:**
282+
283+
- **Mago:** topologically sorts classes (`codex/src/populator/
284+
sorter.rs`) using a DFS with `visited` + `visiting` sets. Cycles
285+
are broken silently when `visiting.contains(&class_like)`. Each
286+
class is then populated exactly once by
287+
`populate_class_like_metadata_iterative` (non-recursive, assumes
288+
dependencies are done). No re-entrant resolution is possible.
289+
290+
- **PHPStan:** member lookup on `ClassReflection` delegates to
291+
`PhpClassReflectionExtension`, which calls PHP's native
292+
`ReflectionClass` (already resolved by the runtime). Each class
293+
has a single canonical instance via `MemoizingReflectionProvider`
294+
(keyed by lowercase name), so re-entrant lookups hit the same
295+
cached object. Explicit cycle guards exist only for specific
296+
features: `$resolvingTypeAliasImports` for `@type-import` cycles,
297+
`$inferClassConstructorPropertyTypesInProcess` for constructor
298+
property inference.
299+
300+
- **Phpactor:** three independent cycle-protection layers.
301+
(1) `ClassHierarchyResolver::doResolve()` passes a `$resolved`
302+
map by value; if a class name is already a key, recursion stops.
303+
(2) Every reflection object carries a `$visited` array through its
304+
constructor; `reflectClassLike()` throws `CycleDetected` on
305+
re-entry. (3) Direct self-reference guards in `parent()` and
306+
`ancestors()`.
307+
308+
**Success criteria:**
309+
- `RESOLVING`, `RESOLVE_DEPTH`, `MAX_RESOLVE_DEPTH`, and
310+
`MAX_RESOLVE_TARGET_DEPTH` are removed from the codebase.
311+
- `mark_resolving` / `unmark_resolving` functions are removed.
312+
- No test regressions.
313+
- LSP server runs eager population incrementally on file change.
314+
315+
#### Phase 4f: Remove inflated stack sizes (absorbs P25)
316+
317+
**Impact: Medium. Effort: Low.**
318+
319+
After Phase 4e eliminates re-entrant class resolution and P20
320+
eliminates exponential forward-walker blowup, the 32 MB stack
321+
threads in `analyse.rs` (index workers, eager population, diagnostic
322+
workers) and the 16 MB stack threads in `references/mod.rs` (parallel
323+
parsing) should no longer be necessary.
324+
325+
- Run the full test suite and `analyse` on the largest available
326+
project with default 8 MB stacks.
327+
- Remove each `stack_size` call that no longer causes overflow.
328+
- The reference parsing threads (16 MB) may still be needed for
329+
pathological PHP files with extreme nesting; verify separately.
330+
331+
Note: Mago's `build.rs` uses a 36 MB stack for prelude parsing, so
332+
inflated stacks are not inherently wrong for one-off build tasks.
333+
The problem is needing them for every runtime analysis thread.
334+
335+
**Success criteria:**
336+
- All `stack_size` calls in `analyse.rs` removed.
337+
- `references/mod.rs` `PARSE_STACK_SIZE` reduced to 8 MB or removed.
338+
- No stack overflows on the full test suite or the largest available
339+
project.
340+
341+
---
342+
246343

247344
## Success criteria
248345

@@ -255,4 +352,9 @@ inside the type resolution pipeline, not by resolution logic.
255352
current `ast_map` + `class_index` combined size.
256353
- The diagnostic/analysis pass never calls
257354
`resolve_class_with_inheritance`. All class metadata is
258-
pre-populated and immutable during analysis.
355+
pre-populated and immutable during analysis.
356+
- All thread-local recursion guards (`RESOLVING`, `RESOLVE_DEPTH`)
357+
and depth caps (`MAX_RESOLVE_DEPTH`, `MAX_RESOLVE_TARGET_DEPTH`)
358+
are removed (Phase 4e).
359+
- All inflated `stack_size` calls are removed or reduced to default
360+
(Phase 4f).

0 commit comments

Comments
 (0)