Skip to content

Commit 33de806

Browse files
MagicalTuxclaude
andcommitted
vm: native foreach over non-by-ref non-local targets
Previously `foreach ($arr as [$a,$b])` / `as $obj->prop` / `as Foo::$bar` / `as $k => [$a,$b]` etc. were AST-delegated via OP_TRY_FINALLY, forcing the enclosing function out of slot-only mode and re-running the whole loop through the AST tree walker. Adds two opcodes: OP_FOREACH_STEP_PUSH (A=keyFlag, C=exhaust-jmp): like OP_FOREACH_STEP but pushes value (and key when A=1) onto the stack instead of storing into a local slot. Always Dup()s to match the AST runner's snapshot. OP_ASSIGN_WRITABLE (A=lhs-ast-idx): pops a value and writes it to the AST node at SubASTs[A] via the phpv.Writable interface — handles obj prop, array element, static prop, dynamic-name var, etc. emitForeach now picks per-target shape: - Both targets bare locals → OP_FOREACH_STEP (unchanged fast path). - Destructure target → push value, OP_DESTRUCTURE_ASSIGN + refresh slots. - Other Writable target → push value, OP_ASSIGN_WRITABLE + refresh slots. By-ref (`as &$v`) still AST-delegates — the iterator needs to yield refs, which OpForeachInit doesn't build yet. That's the next sub-step. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
1 parent 1714315 commit 33de806

3 files changed

Lines changed: 139 additions & 14 deletions

File tree

core/vm/opcode.go

Lines changed: 20 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -394,6 +394,24 @@ const (
394394
// crossing an enclosing finally.
395395
OpFinallyEnd
396396

397+
// OP_FOREACH_STEP_PUSH is the stack-pushing variant of OP_FOREACH_STEP,
398+
// used for foreach value/key targets that aren't a bare local (e.g.
399+
// `foreach ($arr as [$a, $b])` or `as $obj->prop => $val`). If the
400+
// iterator is exhausted it jumps by C as OP_FOREACH_STEP does;
401+
// otherwise it pushes the current value onto the stack (always
402+
// Dup()'d to match the AST runner's snapshot semantics). When A != 0
403+
// the current key is pushed FIRST (below the value), so the emitter
404+
// can pop+assign value then pop+assign key — matching the AST's
405+
// "key write before value write" order.
406+
OpForeachStepPush
407+
408+
// OP_ASSIGN_WRITABLE pops a value off the stack and writes it to
409+
// the AST Writable node at SubASTs[A] via WriteValue. Used for
410+
// foreach targets whose shape isn't a bare local (destructure,
411+
// object prop, array element, …) so we can drive the loop natively
412+
// while delegating only the per-iteration write.
413+
OpAssignWritable
414+
397415
// Sentinel — keep last.
398416
opLast
399417
)
@@ -521,4 +539,6 @@ var opNames = [...]string{
521539
OpStaticVarBind: "STATIC_VAR_BIND",
522540
OpDestructureAssign: "DESTRUCTURE_ASSIGN",
523541
OpFinallyEnd: "FINALLY_END",
542+
OpForeachStepPush: "FOREACH_STEP_PUSH",
543+
OpAssignWritable: "ASSIGN_WRITABLE",
524544
}

core/vm/vm.go

Lines changed: 34 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1019,6 +1019,40 @@ func (f *Frame) runUntilError(ctx phpv.Context) (retVal *phpv.ZVal, finished boo
10191019
return nil, false, err
10201020
}
10211021

1022+
case OpForeachStepPush:
1023+
it := f.iters[len(f.iters)-1]
1024+
if !it.Valid(ctx) {
1025+
f.pc = uint32(int32(f.pc) + ins.C())
1026+
break
1027+
}
1028+
cur, err := it.Current(ctx)
1029+
if err != nil {
1030+
return nil, false, err
1031+
}
1032+
// Match AST runner's "key written before value" order:
1033+
// push key first (deeper in stack), then value (on top).
1034+
// Consumers pop value first, assign value; then pop key,
1035+
// assign key — which yields key-write-then-value-write at
1036+
// the WriteValue level.
1037+
if ins.A() != 0 {
1038+
key, err := it.Key(ctx)
1039+
if err != nil {
1040+
return nil, false, err
1041+
}
1042+
f.push(key.Dup())
1043+
}
1044+
f.push(cur.Dup())
1045+
1046+
case OpAssignWritable:
1047+
val := f.pop()
1048+
lhs, ok := f.fn.SubASTs[ins.A()].(phpv.Writable)
1049+
if !ok {
1050+
return nil, false, fmt.Errorf("OP_ASSIGN_WRITABLE: SubASTs[%d] is not Writable (%T)", ins.A(), f.fn.SubASTs[ins.A()])
1051+
}
1052+
if err := lhs.WriteValue(ctx, val); err != nil {
1053+
return nil, false, err
1054+
}
1055+
10221056
case OpForeachUnwind:
10231057
it := f.iters[len(f.iters)-1]
10241058
if c, ok := it.(interface{ Cleanup() }); ok {

core/vm/vmcompiler/emit_stmt.go

Lines changed: 85 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -549,28 +549,37 @@ func (e *emitter) emitSwitch(n compiler.SwitchNode) error {
549549
}
550550

551551
func (e *emitter) emitForeach(n foreachNode) error {
552-
// foreach-by-ref (`foreach($arr as &$v)`) and non-local targets
553-
// (`foreach($arr as $obj->prop => $val)`, list destructure, etc.)
554-
// are AST-delegated — the surrounding scope's body is flagged
555-
// slot-unsafe by IsSlotSafe so the hashtable stays authoritative
556-
// while the AST runs the loop.
557-
if n.ForeachIsRef() || !isSimpleLocal(n.ForeachValue()) || (n.ForeachKey() != nil && !isSimpleLocal(n.ForeachKey())) {
552+
// foreach-by-ref (`foreach($arr as &$v)`) still AST-delegates — the
553+
// iterator needs to yield refs and the helper that snapshots arrays
554+
// uses different CoW semantics. The slot-unsafe flag on the
555+
// enclosing function (set by IsSlotSafe) keeps the hashtable
556+
// authoritative for the AST loop. Non-simple-local targets WITHOUT
557+
// by-ref take the native path below.
558+
if n.ForeachIsRef() {
558559
raw, ok := any(n).(phpv.Runnable)
559560
if !ok {
560-
return unsupportedf("foreach delegation: cannot retrieve raw Runnable")
561+
return unsupportedf("foreach by-ref delegation: cannot retrieve raw Runnable")
561562
}
562563
idx := e.astIndex(raw)
563564
e.emit(vm.OpTryFinally, idx, 0, 0)
564565
e.emit(vm.OpRefreshSlots, 0, 0, 0)
565566
return nil
566567
}
567-
valNode := n.ForeachValue().(variableNode)
568-
valIdx := e.localIndex(valNode.VariableName())
569568

570-
keyIdx := uint16(0xFFFF)
571-
if k := n.ForeachKey(); k != nil {
572-
kn := k.(variableNode)
573-
keyIdx = e.localIndex(kn.VariableName())
569+
valIsLocal := isSimpleLocal(n.ForeachValue())
570+
keyExpr := n.ForeachKey()
571+
keyIsLocal := keyExpr == nil || isSimpleLocal(keyExpr)
572+
573+
// Compute the operand for OP_FOREACH_STEP / OP_FOREACH_STEP_PUSH.
574+
// When both targets are bare locals, the in-place store path (OP_FOREACH_STEP)
575+
// avoids the stack push/pop entirely.
576+
valLocalIdx := uint16(0xFFFF)
577+
keyLocalIdx := uint16(0xFFFF)
578+
if valIsLocal {
579+
valLocalIdx = e.localIndex(n.ForeachValue().(variableNode).VariableName())
580+
}
581+
if keyExpr != nil && keyIsLocal {
582+
keyLocalIdx = e.localIndex(keyExpr.(variableNode).VariableName())
574583
}
575584

576585
// Eval src and emit the init op. C is patched to point past the
@@ -586,7 +595,33 @@ func (e *emitter) emitForeach(n foreachNode) error {
586595
loopHead := uint32(len(e.code))
587596

588597
// Step: jumps to unwind on iterator exhaustion.
589-
stepPC := e.emit(vm.OpForeachStep, valIdx, keyIdx, 0)
598+
useNative := valIsLocal && keyIsLocal
599+
var stepPC uint32
600+
if useNative {
601+
stepPC = e.emit(vm.OpForeachStep, valLocalIdx, keyLocalIdx, 0)
602+
} else {
603+
// Push key (if present) and value onto stack.
604+
keyFlag := uint16(0)
605+
if keyExpr != nil {
606+
keyFlag = 1
607+
}
608+
stepPC = e.emit(vm.OpForeachStepPush, keyFlag, 0, 0)
609+
if keyExpr != nil {
610+
e.pushStack(1) // key
611+
}
612+
e.pushStack(1) // value
613+
614+
// Pop value off the stack first, write to value target.
615+
if err := e.emitForeachTargetWrite(n.ForeachValue()); err != nil {
616+
return err
617+
}
618+
// Then pop key (if present) and write to key target.
619+
if keyExpr != nil {
620+
if err := e.emitForeachTargetWrite(keyExpr); err != nil {
621+
return err
622+
}
623+
}
624+
}
590625

591626
// Body
592627
if err := e.emitStmt(n.ForeachCode()); err != nil {
@@ -618,6 +653,42 @@ func (e *emitter) emitForeach(n foreachNode) error {
618653
return nil
619654
}
620655

656+
// emitForeachTargetWrite pops a value off the stack and writes it to
657+
// the foreach target node `target`. Used when the value/key isn't a
658+
// bare local — the runtime push the value, this code consumes it.
659+
//
660+
// - destructure (`[$a, $b]`) → OP_DESTRUCTURE_ASSIGN
661+
// - any other Writable shape → OP_ASSIGN_WRITABLE delegating to the
662+
// AST node's WriteValue (handles obj prop, array element, static
663+
// prop, dynamic-name variable, etc.)
664+
// - simple local → OP_STORE_LOCAL (only happens when this is called
665+
// for the key side and the value side forced the push path)
666+
func (e *emitter) emitForeachTargetWrite(target phpv.Runnable) error {
667+
if isSimpleLocal(target) {
668+
idx := e.localIndex(target.(variableNode).VariableName())
669+
e.emit(vm.OpStoreLocal, idx, 0, 0)
670+
e.popStack(1)
671+
return nil
672+
}
673+
if compiler.IsDestructureTarget(target) {
674+
idx := e.astIndex(target)
675+
// flags = 0 → stmt-context (drop the value, don't push it back).
676+
e.emit(vm.OpDestructureAssign, idx, 0, 0)
677+
e.popStack(1)
678+
// Locals may have changed via destructure's WriteValue.
679+
e.emit(vm.OpRefreshSlots, 0, 0, 0)
680+
return nil
681+
}
682+
// Generic Writable (object prop, array access, static prop, …).
683+
idx := e.astIndex(target)
684+
e.emit(vm.OpAssignWritable, idx, 0, 0)
685+
e.popStack(1)
686+
// Conservatively refresh — the WriteValue may have set a local
687+
// through OffsetSet that the slot cache doesn't see.
688+
e.emit(vm.OpRefreshSlots, 0, 0, 0)
689+
return nil
690+
}
691+
621692
func (e *emitter) emitReturn(n returnNode) error {
622693
v := n.ReturnValue()
623694
if n.ReturnHasTypeHint() {

0 commit comments

Comments
 (0)