Skip to content

Commit ab63482

Browse files
MagicalTuxclaude
MagicalTux
and
claude
committed
vm: lower $arr[k] OP= rhs and $arr[k]++ on simple-local arr natively
Add OP_ARRAY_COMPOUND_ASSIGN_LOCAL and OP_ARRAY_INC_DEC_LOCAL, extending the existing OP_ARRAY_SET_LOCAL fast path to compound assigns and inc/dec on simple-local array containers. Both opcodes read the current element via arrayGet (which warns on undef keys, matching the AST), Dup snapshot (defending against __toString/error_handler slot-mutation during '.=' coercion), apply compoundOp / DoInc, then write back via arraySetLocal. Eliminates two AST-delegation call sites in emit_expr.go: - $local[k] OP= rhs (was line 1139) - $local[k]++ / dec (was line 1271) Side effect: fixes a pre-existing AST panic on '$x = $a[k]++' expr-context postfix where the snapshot+DoInc path crashed with 'SetVal() called on cached ZVal'. The VM dispatch correctly Dup's before DoInc. Nested containers ($obj->arr[i] OP= …, $a[i][j]++, …) still AST-delegate. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
1 parent 6bc8695 commit ab63482

3 files changed

Lines changed: 170 additions & 3 deletions

File tree

‎core/vm/opcode.go‎

Lines changed: 33 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -634,6 +634,37 @@ const (
634634
// `res.IsRef() && r.b is *runRef` branch in run-operator.go.
635635
OpStoreLocalRef
636636

637+
// OP_ARRAY_COMPOUND_ASSIGN_LOCAL handles `$local[offset] OP= rhs`
638+
// where the LHS container is a simple local variable. Reads the
639+
// current element via arrayGet, snapshots it (defending against
640+
// handlers that mutate the slot during `.=` array→string coercion,
641+
// bug81705-family), applies the resolved compoundOp, then writes
642+
// back via arraySetLocal.
643+
//
644+
// Stack on entry (top-down): rhs, offset.
645+
// Pops rhs, offset; in expr context (C bit 0) pushes the result.
646+
//
647+
// Encoding:
648+
// A = local slot index
649+
// B = compound op token (T_PLUS_EQUAL, T_CONCAT_EQUAL, …)
650+
// C bit 0 = keep result on stack
651+
OpArrayCompoundAssignLocal
652+
// OP_ARRAY_INC_DEC_LOCAL handles `$local[offset]++`, `++$local[offset]`
653+
// and dec variants on a simple-local array container. Reads the
654+
// current element via arrayGet, Dup's it (DoInc mutates in place),
655+
// applies DoInc, writes back via arraySetLocal. In expr context,
656+
// pushes the original value for postfix or the new value for prefix.
657+
//
658+
// Stack on entry: offset (one operand).
659+
// Pops offset; in expr context (C bit 0) pushes the result.
660+
//
661+
// Encoding:
662+
// A = local slot index
663+
// B bit 0 = inc (1) / dec (0)
664+
// B bit 1 = post (1) / pre (0)
665+
// C bit 0 = keep result on stack
666+
OpArrayIncDecLocal
667+
637668
// Sentinel — keep last.
638669
opLast
639670
)
@@ -785,4 +816,6 @@ var opNames = [...]string{
785816
OpObjectDynCompoundAssign: "OBJECT_DYN_COMPOUND_ASSIGN",
786817
OpIncDecObjDynProp: "INC_DEC_OBJ_DYN_PROP",
787818
OpStoreLocalRef: "STORE_LOCAL_REF",
819+
OpArrayCompoundAssignLocal: "ARRAY_COMPOUND_ASSIGN_LOCAL",
820+
OpArrayIncDecLocal: "ARRAY_INC_DEC_LOCAL",
788821
}

‎core/vm/vm.go‎

Lines changed: 78 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -918,6 +918,84 @@ func (f *Frame) runUntilError(ctx phpv.Context) (retVal *phpv.ZVal, finished boo
918918
return nil, false, err
919919
}
920920

921+
case OpArrayCompoundAssignLocal:
922+
// `$local[offset] OP= rhs` on a simple-local container.
923+
// Mirrors OpObjectCompoundAssign's read+snapshot+op+write
924+
// flow but with the array element as the LHS slot.
925+
rhs := f.pop()
926+
offset := f.pop()
927+
op := tokenizer.ItemType(ins.B())
928+
fn := compoundOp(op)
929+
if fn == nil {
930+
return nil, false, fmt.Errorf("vm: OP_ARRAY_COMPOUND_ASSIGN_LOCAL unknown op %d", ins.B())
931+
}
932+
container := f.locals[ins.A()]
933+
if container == nil {
934+
container = phpv.ZNULL.ZVal()
935+
}
936+
cur, err := arrayGet(ctx, container, offset)
937+
if err != nil {
938+
return nil, false, err
939+
}
940+
if cur == nil {
941+
cur = phpv.ZNULL.ZVal()
942+
}
943+
// Snapshot cur — `.=` on an array element can trigger
944+
// __toString / error_handler side effects that mutate the
945+
// underlying slot mid-op (bug81705-family). Without the
946+
// Dup, the operator would read the post-handler value.
947+
cur = cur.Dup()
948+
res, err := fn(ctx, cur, rhs)
949+
if err != nil {
950+
return nil, false, err
951+
}
952+
if err := arraySetLocal(ctx, f, ins.A(), offset, res); err != nil {
953+
return nil, false, err
954+
}
955+
if ins.C()&1 != 0 {
956+
f.push(res)
957+
}
958+
959+
case OpArrayIncDecLocal:
960+
// `$local[offset]++`, `++$local[offset]` and dec variants
961+
// on a simple-local container. Reads via arrayGet (which
962+
// warns on undefined keys, matching the AST), Dup's so
963+
// DoInc's in-place mutation doesn't escape, applies DoInc,
964+
// writes back via arraySetLocal.
965+
offset := f.pop()
966+
inc := ins.B()&1 != 0
967+
post := ins.B()&2 != 0
968+
keep := ins.C()&1 != 0
969+
container := f.locals[ins.A()]
970+
if container == nil {
971+
container = phpv.ZNULL.ZVal()
972+
}
973+
cur, err := arrayGet(ctx, container, offset)
974+
if err != nil {
975+
return nil, false, err
976+
}
977+
if cur == nil {
978+
cur = phpv.ZNULL.ZVal()
979+
}
980+
cur = cur.Dup()
981+
var pre *phpv.ZVal
982+
if post {
983+
pre = cur.Dup()
984+
}
985+
if err := compiler.DoInc(ctx, cur, inc); err != nil {
986+
return nil, false, err
987+
}
988+
if err := arraySetLocal(ctx, f, ins.A(), offset, cur); err != nil {
989+
return nil, false, err
990+
}
991+
if keep {
992+
if post {
993+
f.push(pre)
994+
} else {
995+
f.push(cur)
996+
}
997+
}
998+
921999
// --- throw ---------------------------------------------------
9221000
case OpThrow:
9231001
v := f.pop()

‎core/vm/vmcompiler/emit_expr.go‎

Lines changed: 59 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1134,8 +1134,33 @@ func (e *emitter) emitCompoundAssign(n operatorNode, op tokenizer.ItemType) erro
11341134
}
11351135
return unsupportedf("object-dyn-var compound LHS shape: %T", n.OperatorA())
11361136
}
1137-
// Array compound assigns still need a dedicated path.
1138-
if _, ok := n.OperatorA().(arrayAccessNode); ok {
1137+
// `$local[k] OP= rhs` on simple-local container: native via
1138+
// OP_ARRAY_COMPOUND_ASSIGN_LOCAL. Reads via arrayGet, snapshots,
1139+
// applies compoundOp, writes back via arraySetLocal. Nested
1140+
// containers (`$obj->arr[i]`, `$a[i][j]`, …) still AST-delegate.
1141+
if aa, ok := n.OperatorA().(arrayAccessNode); ok {
1142+
if v, ok := aa.ArrayAccessContainer().(variableNode); ok &&
1143+
!aa.ArrayAccessIsNullSafe() && aa.ArrayAccessOffset() != nil {
1144+
stmtCtx := e.stmtCtx
1145+
if err := e.withSubexpr(func() error { return e.emitExpr(aa.ArrayAccessOffset()) }); err != nil {
1146+
return err
1147+
}
1148+
if err := e.withSubexpr(func() error { return e.emitExpr(n.OperatorB()) }); err != nil {
1149+
return err
1150+
}
1151+
idx := e.localIndex(v.VariableName())
1152+
var c int32
1153+
if !stmtCtx {
1154+
c = 1
1155+
}
1156+
e.emit(vm.OpArrayCompoundAssignLocal, idx, uint16(op), c)
1157+
if stmtCtx {
1158+
e.popStack(2)
1159+
} else {
1160+
e.popStack(1) // pops offset+rhs, pushes res → net -1
1161+
}
1162+
return nil
1163+
}
11391164
return e.emitAssignViaAST(n)
11401165
}
11411166
if compiler.IsStaticPropertyTarget(n.OperatorA()) {
@@ -1267,7 +1292,38 @@ func (e *emitter) emitIncDec(n operatorNode, inc bool) error {
12671292
if compiler.IsClassStaticDynVarReadNode(target) {
12681293
return e.emitStaticPropDynIncDec(target, inc, post, e.stmtCtx)
12691294
}
1270-
// ++/-- on $arr[$k] etc. — still route through the AST.
1295+
// `$local[k]++` / `++$local[k]` / dec on a simple-local
1296+
// container: native via OP_ARRAY_INC_DEC_LOCAL. Nested
1297+
// containers (`$obj->arr[i]`, `$a[i][j]`, …) still AST-delegate.
1298+
if aa, ok := target.(arrayAccessNode); ok {
1299+
if v, ok := aa.ArrayAccessContainer().(variableNode); ok &&
1300+
!aa.ArrayAccessIsNullSafe() && aa.ArrayAccessOffset() != nil {
1301+
stmtCtx := e.stmtCtx
1302+
if err := e.withSubexpr(func() error { return e.emitExpr(aa.ArrayAccessOffset()) }); err != nil {
1303+
return err
1304+
}
1305+
idx := e.localIndex(v.VariableName())
1306+
var b uint16
1307+
if inc {
1308+
b |= 1
1309+
}
1310+
if post {
1311+
b |= 2
1312+
}
1313+
var c int32
1314+
if !stmtCtx {
1315+
c = 1
1316+
}
1317+
e.emit(vm.OpArrayIncDecLocal, idx, b, c)
1318+
if stmtCtx {
1319+
e.popStack(1)
1320+
}
1321+
// expr-context: pops offset, pushes res → net 0.
1322+
return nil
1323+
}
1324+
}
1325+
// Other cases (`$obj->arr[i]++`, `$a[i][j]++`, …) still
1326+
// AST-delegate.
12711327
return e.emitAssignViaAST(n)
12721328
}
12731329
idx := e.localIndex(tv.VariableName())

0 commit comments

Comments
 (0)