Skip to content

Commit bb1672f

Browse files
MagicalTuxclaude
andcommitted
vm: lower \$obj->prop OP= rhs to OP_OBJECT_COMPOUND_ASSIGN
Compound assigns on object properties (e.g. $obj->n += 5, $obj->s .= ' x') were delegated to the AST runner via emitAssignViaAST. They now lower to a dedicated OP_OBJECT_COMPOUND_ASSIGN opcode. Encoding: A = const-pool name index (ZString prop name) B = tokenizer ItemType (e.g. T_PLUS_EQUAL, T_CONCAT_EQUAL) C bit 0 = "keep value on stack" (expr vs stmt context) Stack: pops rhs, pops receiver. Inside the handler: cur := objectGet(receiver, name) (NULL on undefined) cur := cur.Dup() (bug81705-style snapshot) res := compoundOp(op)(cur, rhs) objectSet(receiver, name, res) if C&1 { push(res) } Only the static-name, non-nullsafe shape goes native; dynamic-name \$obj->{\$x} += v and nullsafe \$obj?->prop += v still fall through to emitAssignViaAST. Typed properties, hooks, and asymmetric visibility are already handled by the downstream ZObject.ObjectSet, so the native path inherits them unchanged. Verified locally: - int/string/nullable compound assigns - .= with array→string coercion - typed-prop weak-mode coercion (numeric string warning + float deprecation notice) - property hooks fire (set hook called with post-op value) - expr-context: \$x = (\$obj->n += 100) yields 100 Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
1 parent d3c1632 commit bb1672f

4 files changed

Lines changed: 104 additions & 4 deletions

File tree

core/vm/opcode.go

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -476,6 +476,17 @@ const (
476476
// dispatch to ObjectSet(name, nil).
477477
OpUnsetObjProp
478478

479+
// `$obj->prop OP= rhs` compound assign for static-name, non-
480+
// nullsafe property access. Encoding:
481+
// A = const-pool name index (ZString prop name)
482+
// B = tokenizer ItemType (compound op token, e.g.
483+
// T_PLUS_EQUAL, T_CONCAT_EQUAL)
484+
// C bit 0 = "keep value on stack" (expr-context vs stmt-ctx)
485+
// Stack: pops rhs, pops receiver. Reads cur via objectGet,
486+
// applies compoundOp(op)(cur, rhs), writes back via objectSet.
487+
// Pushes the post-op value back when keep-flag is set.
488+
OpObjectCompoundAssign
489+
479490
// Sentinel — keep last.
480491
opLast
481492
)
@@ -613,4 +624,5 @@ var opNames = [...]string{
613624
OpIssetObjProp: "ISSET_OBJ_PROP",
614625
OpEmptyObjProp: "EMPTY_OBJ_PROP",
615626
OpUnsetObjProp: "UNSET_OBJ_PROP",
627+
OpObjectCompoundAssign: "OBJECT_COMPOUND_ASSIGN",
616628
}

core/vm/vm.go

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

1076+
case OpObjectCompoundAssign:
1077+
rhs := f.pop()
1078+
receiver := f.pop()
1079+
name, ok := f.fn.Consts[ins.A()].(phpv.ZString)
1080+
if !ok {
1081+
return nil, false, fmt.Errorf("vm: OP_OBJECT_COMPOUND_ASSIGN name const is %T not ZString", f.fn.Consts[ins.A()])
1082+
}
1083+
op := tokenizer.ItemType(ins.B())
1084+
fn := compoundOp(op)
1085+
if fn == nil {
1086+
return nil, false, fmt.Errorf("vm: OP_OBJECT_COMPOUND_ASSIGN unknown op %d", ins.B())
1087+
}
1088+
cur, err := objectGet(ctx, receiver, name)
1089+
if err != nil {
1090+
return nil, false, err
1091+
}
1092+
if cur == nil {
1093+
cur = phpv.ZNULL.ZVal()
1094+
}
1095+
// Snapshot cur — mirrors OpOpAssignLocal's defense
1096+
// against handlers that mutate the slot in-place during
1097+
// `.=` coercion (bug81705 family).
1098+
cur = cur.Dup()
1099+
res, err := fn(ctx, cur, rhs)
1100+
if err != nil {
1101+
return nil, false, err
1102+
}
1103+
if err := objectSet(ctx, receiver, name, res); err != nil {
1104+
return nil, false, err
1105+
}
1106+
if ins.C()&1 != 0 {
1107+
f.push(res)
1108+
}
1109+
10761110
case OpObjectCall:
10771111
argc := int(ins.B())
10781112
args := make([]*phpv.ZVal, argc)

core/vm/vmcompiler/emit_expr.go

Lines changed: 14 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -949,10 +949,20 @@ func (e *emitter) emitAssignViaAST(n operatorNode) error {
949949
func (e *emitter) emitCompoundAssign(n operatorNode, op tokenizer.ItemType) error {
950950
lhs, ok := n.OperatorA().(variableNode)
951951
if !ok {
952-
// Delegate property / static-prop / array-element compound
953-
// writes to the AST.
954-
switch n.OperatorA().(type) {
955-
case objectVarNode, arrayAccessNode:
952+
// `$obj->prop OP= rhs`: emit natively for static-name,
953+
// non-nullsafe property access. OP_OBJECT_COMPOUND_ASSIGN
954+
// dispatches via objectGet/objectSet (which reach into the
955+
// AST's ZObject layer for typed props, hooks, etc.) and
956+
// applies the resolved compoundOp inside the handler.
957+
if ov, ok := n.OperatorA().(objectVarNode); ok {
958+
name := ov.ObjectVarName()
959+
if !ov.ObjectVarIsNullSafe() && !(len(name) > 0 && name[0] == '$') {
960+
return e.emitObjectVarCompoundAssign(ov, n.OperatorB(), op, e.stmtCtx)
961+
}
962+
return e.emitAssignViaAST(n)
963+
}
964+
// Array compound assigns still need a dedicated path.
965+
if _, ok := n.OperatorA().(arrayAccessNode); ok {
956966
return e.emitAssignViaAST(n)
957967
}
958968
if compiler.IsStaticPropertyTarget(n.OperatorA()) {

core/vm/vmcompiler/emit_object.go

Lines changed: 44 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,7 @@ package vmcompiler
33
import (
44
"github.com/KarpelesLab/goro/core/compiler"
55
"github.com/KarpelesLab/goro/core/phpv"
6+
"github.com/KarpelesLab/goro/core/tokenizer"
67
"github.com/KarpelesLab/goro/core/vm"
78
)
89

@@ -106,6 +107,49 @@ func (e *emitter) emitObjectVarAssign(lhs objectVarNode, rhs phpv.Runnable, stmt
106107
return nil
107108
}
108109

110+
// emitObjectVarCompoundAssign emits `$obj->prop OP= rhs` for static-name,
111+
// non-nullsafe property access. The receiver is evaluated once; the
112+
// resulting value handle is reused for both objectGet (read current) and
113+
// objectSet (write back). The PHP semantics-level dance — typed prop
114+
// coercion, hook dispatch, asymmetric visibility — happens inside
115+
// ZObject.ObjectSet, same as for plain `=`.
116+
func (e *emitter) emitObjectVarCompoundAssign(lhs objectVarNode, rhs phpv.Runnable, op tokenizer.ItemType, stmtCtx bool) error {
117+
if lhs.ObjectVarIsNullSafe() {
118+
return unsupportedf("nullsafe property compound assign")
119+
}
120+
name := lhs.ObjectVarName()
121+
if len(name) > 0 && name[0] == '$' {
122+
return unsupportedf("dynamic property name in compound assign")
123+
}
124+
recv := lhs.ObjectVarReceiver()
125+
// Receiver write-context: an undefined simple-variable receiver
126+
// must not warn. Mirror emitObjectVarAssign's silent load path.
127+
if v, ok := recv.(variableNode); ok {
128+
idx := e.localIndex(v.VariableName())
129+
e.emit(vm.OpLoadLocal, idx, 0, 0)
130+
e.pushStack(1)
131+
} else {
132+
if err := e.withSubexpr(func() error { return e.emitExpr(recv) }); err != nil {
133+
return err
134+
}
135+
}
136+
if err := e.withSubexpr(func() error { return e.emitExpr(rhs) }); err != nil {
137+
return err
138+
}
139+
nameIdx := e.constIndex(name)
140+
var c int32
141+
if !stmtCtx {
142+
c = 1 // keep post-op value on stack
143+
}
144+
e.emit(vm.OpObjectCompoundAssign, nameIdx, uint16(op), c)
145+
if stmtCtx {
146+
e.popStack(2) // pop receiver + rhs, nothing pushed
147+
} else {
148+
e.popStack(1) // pop receiver+rhs, push result → net -1
149+
}
150+
return nil
151+
}
152+
109153
func (e *emitter) emitObjectVarRead(n objectVarNode) error {
110154
name := n.ObjectVarName()
111155
// `$obj->$x` (no curly braces) parses to runObjectVar with the

0 commit comments

Comments
 (0)