Skip to content

Commit 645023b

Browse files
committed
Remove IN_ARRAY_KEY_ASSIGN re-entry guard from forward walker
1 parent 39e28ef commit 645023b

3 files changed

Lines changed: 1 addition & 52 deletions

File tree

docs/todo.md

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

2626
| # | Item | Impact | Effort |
2727
| --- | ------------------------------------------------------------------------------------------------------------------------------------------------------------------- | ----------- | ----------- |
28-
| 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 |
28+
2929
| P20 | [Forward walker: bounded iteration and depth-cap cleanup](todo/performance.md#p20-forward-walker-bounded-iteration-and-depth-cap-cleanup) | High | Medium |
3030
| ER5 | [Mago-style separated metadata](todo/eager-resolution.md#er5--mago-style-separated-metadata) | High | High |
3131
| P9 | [`resolved_class_cache` generic-arg specialisation](todo/performance.md#p9-resolved_class_cache-generic-arg-specialisation) | Medium | Medium |

docs/todo/performance.md

Lines changed: 0 additions & 33 deletions
Original file line numberDiff line numberDiff line change
@@ -852,36 +852,3 @@ like:
852852
Most depth-cap and recursion-guard issues are addressed by P20
853853
(forward walker) and ER5 (class resolution). The items below are
854854
independent fixes that do not depend on either.
855-
856-
---
857-
858-
## P24. `IN_ARRAY_KEY_ASSIGN` re-entry guard in forward walker
859-
860-
**Impact: Low · Effort: Low**
861-
862-
`forward_walk.rs` line ~3861 uses a thread-local `Cell<bool>` to
863-
break re-entry in `process_array_key_assignment` when
864-
`resolve_rhs_with_scope` triggers re-evaluation of the same array
865-
key (e.g. `$a['k'] = f($a['k'])`). This is a symptom of the
866-
forward walker calling back into expression resolution which calls
867-
back into the forward walker.
868-
869-
### How Mago solves this
870-
871-
Mago's forward pass never re-enters itself. Array key assignments
872-
are processed by recording the LHS target, resolving the RHS type
873-
in isolation (using the scope snapshot from before the assignment),
874-
and then updating the scope. The RHS resolution reads from an
875-
immutable scope snapshot, so it cannot trigger a write that would
876-
re-enter the assignment handler.
877-
878-
### Fix
879-
880-
When processing `$a['k'] = expr`, snapshot the scope before the
881-
assignment, resolve `expr` against the snapshot (read-only), then
882-
apply the result to the live scope. This eliminates the re-entry
883-
path entirely and the `IN_ARRAY_KEY_ASSIGN` guard can be removed.
884-
885-
---
886-
887-

src/completion/variable/forward_walk.rs

Lines changed: 0 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -3859,24 +3859,6 @@ fn process_array_key_assignment<'b>(
38593859
scope: &mut ScopeState,
38603860
ctx: &ForwardWalkCtx<'_>,
38613861
) {
3862-
// Guard against infinite re-entry. When `resolve_rhs_with_scope`
3863-
// triggers re-evaluation of the same array key assignment (e.g.
3864-
// `$a['k'] = f($a['k'])` where reading `$a['k']` re-enters
3865-
// through the scope resolver), bail out to break the cycle.
3866-
thread_local! {
3867-
static IN_ARRAY_KEY_ASSIGN: Cell<bool> = const { Cell::new(false) };
3868-
}
3869-
if IN_ARRAY_KEY_ASSIGN.with(|c| c.get()) {
3870-
return;
3871-
}
3872-
IN_ARRAY_KEY_ASSIGN.with(|c| c.set(true));
3873-
struct ArrayKeyAssignGuard;
3874-
impl Drop for ArrayKeyAssignGuard {
3875-
fn drop(&mut self) {
3876-
IN_ARRAY_KEY_ASSIGN.with(|c| c.set(false));
3877-
}
3878-
}
3879-
let _guard = ArrayKeyAssignGuard;
38803862
// Delegate to the existing check_expression_for_assignment
38813863
// infrastructure for array key assignments. This handles
38823864
// both string-keyed shape building and generic element tracking.

0 commit comments

Comments
 (0)