Skip to content

Commit e40521e

Browse files
MagicalTuxclaude
andcommitted
vm: lower by-ref/named/spread calls to dedicated ByExprs opcodes
Previously, function/method calls whose argument shape required the full ctx.Call binding pipeline (by-ref params, named args, spread) were lowered to OP_CLASS_CONST / OP_TRY_FINALLY — generic AST.Run() delegation that ran the entire runnableFunctionCall / runObjectFunc / runnableFunctionCallRef node as one opaque step. Replace that with three dedicated opcodes that hand the raw argument-expression list to ctx.Call: - OP_CALL_USER_BY_EXPRS: foo($a, $b, ...) - OP_OBJECT_CALL_BY_EXPRS: $obj->method($a, $b, ...) - OP_CALL_INDIRECT_BY_EXPRS: $f($a, $b, ...) The arg expressions live in a new Function.SubArgs slice (parallel to SubASTs / SubFns) so the opcode only carries the SubArgs index. At runtime, each handler resolves its callable (same CallableCache hookup as OP_CALL_USER, same ResolveCallable as OP_CALL_INDIRECT, same method-lookup as OP_OBJECT_CALL) then calls ctx.Call(..., exprs, ...), which evaluates each expression with by-ref binding, named-arg reordering, and spread expansion — the path the AST runner's runnableFunctionCall.Run already takes. For OP_OBJECT_CALL_BY_EXPRS, extract a sibling helper CallInstanceMethodByExprs in core/compiler/dispatch.go that mirrors CallInstanceMethod byte-for-byte; only the terminal call switches from ctx.CallZVal to ctx.Call so by-ref params bind correctly. The __call magic-method fallback still uses ZVal args (the magic array needs them), so the helper evaluates exprs up-front for that branch only. emit_call.go's emitFunctionCall and emitFunctionCallRef, and emit_object.go's emitObjectFuncCall, now route the by-ref/special-args/writable-arg cases through these opcodes instead of emitCallViaAST. Stmt-context drops the result with OP_POP; OP_REFRESH_SLOTS follows each call so subsequent slot- cache reads see any caller-local mutations from a by-ref param. Smoke tests pass: sort/array_pop/preg_match by-ref builtins, user-function by-ref params, closure by-ref params, instance method by-ref params, mixed named+positional+spread args, array_walk by-ref callback, __call fallback, nullsafe with spread args. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
1 parent 33de806 commit e40521e

7 files changed

Lines changed: 396 additions & 15 deletions

File tree

core/compiler/dispatch.go

Lines changed: 147 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -614,6 +614,153 @@ func CallInstanceMethod(ctx phpv.Context, obj *phpv.ZVal, name phpv.ZString, arg
614614
return ctx.CallZVal(ctx, method.Method, args, objBound)
615615
}
616616

617+
// CallInstanceMethodByExprs is the by-AST-expression variant of
618+
// CallInstanceMethod. Used by OP_OBJECT_CALL_BY_EXPRS for method calls
619+
// whose argument shape needs the full ctx.Call binding pipeline
620+
// (by-ref parameters, named arguments, spread). The dispatch logic
621+
// mirrors CallInstanceMethod byte-for-byte; only the terminal call
622+
// switches from ctx.CallZVal to ctx.Call so the binding layer sees the
623+
// raw argument expressions.
624+
//
625+
// The __call magic-method fallback still needs ZVal args (it builds
626+
// a magic array), so this evaluates exprs up-front for that branch
627+
// only — by-ref binding doesn't apply through __call anyway, since
628+
// the user wrote `__call($name, $args)` with the args array.
629+
func CallInstanceMethodByExprs(ctx phpv.Context, obj *phpv.ZVal, name phpv.ZString, exprs []phpv.Runnable) (*phpv.ZVal, error) {
630+
zobj, ok := obj.Value().(*phpobj.ZObject)
631+
if !ok {
632+
if zo, ok := obj.Value().(phpv.ZObject); ok {
633+
m, mok := zo.GetClass().GetMethod(name)
634+
if !mok {
635+
return nil, phpobj.ThrowError(ctx, phpobj.Error,
636+
fmt.Sprintf("Call to undefined method %s::%s()", zo.GetClass().GetName(), name))
637+
}
638+
return ctx.Call(ctx, m.Method, exprs, zo)
639+
}
640+
return nil, fmt.Errorf("CallInstanceMethodByExprs: receiver is not a ZObject")
641+
}
642+
643+
class := zobj.GetClass()
644+
method, ok := class.GetMethod(name)
645+
if !ok {
646+
if cm, hasCall := class.GetMethod("__call"); hasCall {
647+
args, err := evalExprArgs(ctx, exprs)
648+
if err != nil {
649+
return nil, err
650+
}
651+
a := phpv.NewZArray()
652+
for _, sub := range args {
653+
a.OffsetSet(ctx, nil, sub.Dup())
654+
}
655+
return ctx.CallZVal(ctx, cm.Method, []*phpv.ZVal{name.ZVal(), a.ZVal()}, zobj)
656+
}
657+
return nil, phpobj.ThrowError(ctx, phpobj.Error,
658+
fmt.Sprintf("Call to undefined method %s::%s()", class.GetName(), name))
659+
}
660+
661+
if method.Modifiers.Has(phpv.ZAttrAbstract) || (method.Empty && method.Class != nil && method.Class.GetType() != phpv.ZClassTypeInterface) {
662+
return nil, phpobj.ThrowError(ctx, phpobj.Error,
663+
fmt.Sprintf("Cannot call abstract method %s::%s()", method.Class.GetName(), method.Name))
664+
}
665+
666+
methodNotVisible := false
667+
var visErrMsg string
668+
if method.Modifiers.Has(phpv.ZAttrPrivate) {
669+
callerClass := ctx.Class()
670+
methodClass := method.Class
671+
if callerClass == nil || methodClass == nil || callerClass.GetName() != methodClass.GetName() {
672+
methodClassName := class.GetName()
673+
if methodClass != nil {
674+
methodClassName = methodClass.GetName()
675+
}
676+
scope := "global scope"
677+
if callerClass != nil {
678+
scope = "scope " + string(callerClass.GetName())
679+
}
680+
methodNotVisible = true
681+
visErrMsg = fmt.Sprintf("Call to private method %s::%s() from %s", methodClassName, method.Name, scope)
682+
}
683+
} else if method.Modifiers.Has(phpv.ZAttrProtected) {
684+
callerClass := ctx.Class()
685+
if callerClass == nil {
686+
methodNotVisible = true
687+
visErrMsg = fmt.Sprintf("Call to protected method %s::%s() from global scope", class.GetName(), method.Name)
688+
} else if !callerClass.InstanceOf(method.Class) && !method.Class.InstanceOf(callerClass) && !callerClass.InstanceOf(class) && !class.InstanceOf(callerClass) {
689+
protectedVisible := false
690+
if method.Class != nil {
691+
rootClass := method.Class
692+
for rootClass.GetParent() != nil {
693+
if pm, ok := rootClass.GetParent().GetMethod(method.Name); ok && pm.Modifiers.Has(phpv.ZAttrProtected) {
694+
rootClass = rootClass.GetParent()
695+
} else {
696+
break
697+
}
698+
}
699+
if callerClass.InstanceOf(rootClass) {
700+
protectedVisible = true
701+
}
702+
}
703+
if !protectedVisible {
704+
methodNotVisible = true
705+
visErrMsg = fmt.Sprintf("Call to protected method %s::%s() from scope %s", class.GetName(), method.Name, callerClass.GetName())
706+
}
707+
}
708+
}
709+
if methodNotVisible {
710+
if cm, hasCall := class.GetMethod("__call"); hasCall {
711+
args, err := evalExprArgs(ctx, exprs)
712+
if err != nil {
713+
return nil, err
714+
}
715+
a := phpv.NewZArray()
716+
for _, sub := range args {
717+
a.OffsetSet(ctx, nil, sub.Dup())
718+
}
719+
return ctx.CallZVal(ctx, cm.Method, []*phpv.ZVal{name.ZVal(), a.ZVal()}, zobj)
720+
}
721+
return nil, phpobj.ThrowError(ctx, phpobj.Error, visErrMsg)
722+
}
723+
724+
if method.Modifiers.IsStatic() {
725+
m := phpv.BindClassLSB(method.Method, class, class, true)
726+
m.Attributes = method.Attributes
727+
return ctx.Call(ctx, m, exprs, nil)
728+
}
729+
730+
var objBound phpv.ZObject = zobj
731+
if method.Class != nil {
732+
if kin := zobj.GetKin(string(method.Class.GetName())); kin != nil {
733+
objBound = kin
734+
}
735+
}
736+
if len(method.Attributes) > 0 {
737+
wrapped := &phpv.MethodCallable{
738+
Callable: method.Method,
739+
Class: class,
740+
Attributes: method.Attributes,
741+
AliasName: string(method.Name),
742+
}
743+
return ctx.Call(ctx, wrapped, exprs, objBound)
744+
}
745+
return ctx.Call(ctx, method.Method, exprs, objBound)
746+
}
747+
748+
// evalExprArgs evaluates each AST argument expression to a ZVal,
749+
// stopping at the first error. Used only by the __call magic-method
750+
// fallback paths in CallInstanceMethodByExprs, which need ZVal args
751+
// to build the magic array.
752+
func evalExprArgs(ctx phpv.Context, exprs []phpv.Runnable) ([]*phpv.ZVal, error) {
753+
out := make([]*phpv.ZVal, 0, len(exprs))
754+
for _, e := range exprs {
755+
v, err := e.Run(ctx)
756+
if err != nil {
757+
return nil, err
758+
}
759+
out = append(out, v)
760+
}
761+
return out, nil
762+
}
763+
617764
// EvalBinop computes the result of a binary operator with full PHP
618765
// semantics. The VM uses this so its arithmetic / bitwise / compare /
619766
// concat opcodes match the AST runOperator path byte-for-byte.

core/vm/function.go

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -24,6 +24,13 @@ type Function struct {
2424
SubFns []*Function // direct-call targets (resolved at emit time)
2525
SubClosures []phpv.Runnable // *ZClosure templates referenced by OP_MAKE_CLOSURE
2626
SubASTs []phpv.Runnable // delegated AST nodes (class const, static, …)
27+
// SubArgs holds per-call-site argument expression lists for the
28+
// "ByExprs" call opcodes (OP_CALL_USER_BY_EXPRS, OP_CALL_INDIRECT_BY_EXPRS,
29+
// OP_OBJECT_CALL_BY_EXPRS). The runtime passes args[idx] to ctx.Call,
30+
// which evaluates each expression in caller scope with full by-ref /
31+
// named / spread support — the path the AST runner's
32+
// runnableFunctionCall.Run takes. Indexed by the opcode's B field.
33+
SubArgs [][]phpv.Runnable
2734
LocsSparse []LocEntry // sorted by PC
2835
TryHandlers []TryHandler
2936
NumParams int

core/vm/opcode.go

Lines changed: 27 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -412,6 +412,30 @@ const (
412412
// while delegating only the per-iteration write.
413413
OpAssignWritable
414414

415+
// OP_CALL_USER_BY_EXPRS is the by-ref / named / spread variant of
416+
// OP_CALL_USER. A = const-pool ZString function name (with the same
417+
// CallableCache hookup OP_CALL_USER uses); B = SubArgs index of the
418+
// arg-expression list. The handler resolves the callable then calls
419+
// ctx.Call(ctx, callable, exprs, nil), which evaluates each arg
420+
// expression with by-ref binding, named-arg reordering, and spread
421+
// expansion. Pushes the result on top of the stack. No args are
422+
// pushed beforehand — the expressions live in fn.SubArgs.
423+
OpCallUserByExprs
424+
425+
// OP_CALL_INDIRECT_BY_EXPRS is the by-ref / named / spread variant
426+
// of OP_CALL_INDIRECT. A = SubArgs index of the arg-expression list.
427+
// The callable is at the top of the stack (already emitted as a
428+
// native expression); the handler pops it, resolves it via
429+
// ResolveCallable, then calls ctx.Call. Pushes the result.
430+
OpCallIndirectByExprs
431+
432+
// OP_OBJECT_CALL_BY_EXPRS is the by-ref / named / spread variant of
433+
// instance method calls. A = const-pool ZString method name; B =
434+
// SubArgs index of the arg-expression list. The receiver is at the
435+
// top of the stack; the handler pops it, resolves the method, then
436+
// calls ctx.Call with the receiver as $this. Pushes the result.
437+
OpObjectCallByExprs
438+
415439
// Sentinel — keep last.
416440
opLast
417441
)
@@ -541,4 +565,7 @@ var opNames = [...]string{
541565
OpFinallyEnd: "FINALLY_END",
542566
OpForeachStepPush: "FOREACH_STEP_PUSH",
543567
OpAssignWritable: "ASSIGN_WRITABLE",
568+
OpCallUserByExprs: "CALL_USER_BY_EXPRS",
569+
OpCallIndirectByExprs: "CALL_INDIRECT_BY_EXPRS",
570+
OpObjectCallByExprs: "OBJECT_CALL_BY_EXPRS",
544571
}

core/vm/vm.go

Lines changed: 108 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -709,6 +709,76 @@ func (f *Frame) runUntilError(ctx phpv.Context) (retVal *phpv.ZVal, finished boo
709709
}
710710
f.push(res)
711711

712+
case OpCallUserByExprs:
713+
// By-ref / named / spread variant. A = const-pool name idx,
714+
// B = SubArgs index of the argument-expression list. The
715+
// callable is resolved at runtime (with CallableCache reuse)
716+
// then handed to ctx.Call which evaluates each expression
717+
// with full binding semantics.
718+
name, ok := f.fn.Consts[ins.A()].(phpv.ZString)
719+
if !ok {
720+
return nil, false, fmt.Errorf("vm: OP_CALL_USER_BY_EXPRS name const is %T not ZString", f.fn.Consts[ins.A()])
721+
}
722+
nameIdx := ins.A()
723+
var callable phpv.Callable
724+
if int(nameIdx) < len(f.fn.CallableCache) {
725+
callable = f.fn.CallableCache[nameIdx]
726+
}
727+
if callable == nil {
728+
c, err := ctx.Global().GetFunction(ctx, name)
729+
if err != nil {
730+
return nil, false, err
731+
}
732+
callable = c
733+
if f.fn.CallableCache == nil {
734+
f.fn.CallableCache = make([]phpv.Callable, len(f.fn.Consts))
735+
}
736+
if int(nameIdx) < len(f.fn.CallableCache) {
737+
f.fn.CallableCache[nameIdx] = callable
738+
}
739+
}
740+
argsIdx := int(ins.B())
741+
if argsIdx >= len(f.fn.SubArgs) {
742+
return nil, false, fmt.Errorf("vm: OP_CALL_USER_BY_EXPRS SubArgs index %d out of range", argsIdx)
743+
}
744+
res, err := ctx.Call(ctx, callable, f.fn.SubArgs[argsIdx], nil)
745+
if err != nil {
746+
return nil, false, err
747+
}
748+
if res == nil {
749+
res = phpv.ZNULL.ZVal()
750+
}
751+
f.push(res)
752+
753+
case OpCallIndirectByExprs:
754+
// A = SubArgs index. Callable expression already on top of
755+
// stack. Pop it, resolve via ResolveCallable, then dispatch
756+
// via ctx.Call so by-ref / named / spread args bind through
757+
// the standard call-binding pipeline.
758+
callable := f.pop()
759+
c, this, err := compiler.ResolveCallable(ctx, callable)
760+
if err != nil {
761+
return nil, false, err
762+
}
763+
argsIdx := int(ins.A())
764+
if argsIdx >= len(f.fn.SubArgs) {
765+
return nil, false, fmt.Errorf("vm: OP_CALL_INDIRECT_BY_EXPRS SubArgs index %d out of range", argsIdx)
766+
}
767+
exprs := f.fn.SubArgs[argsIdx]
768+
var res *phpv.ZVal
769+
if this != nil {
770+
res, err = ctx.Call(ctx, c, exprs, this)
771+
} else {
772+
res, err = ctx.Call(ctx, c, exprs, nil)
773+
}
774+
if err != nil {
775+
return nil, false, err
776+
}
777+
if res == nil {
778+
res = phpv.ZNULL.ZVal()
779+
}
780+
f.push(res)
781+
712782
// --- return --------------------------------------------------
713783
case OpRet:
714784
v := f.pop()
@@ -969,6 +1039,44 @@ func (f *Frame) runUntilError(ctx phpv.Context) (retVal *phpv.ZVal, finished boo
9691039
}
9701040
f.push(res)
9711041

1042+
case OpObjectCallByExprs:
1043+
// By-ref / named / spread variant. A = const-pool method
1044+
// name idx, B = SubArgs index, C != 0 marks nullsafe. The
1045+
// receiver is on top of stack; the call dispatch goes
1046+
// through CallInstanceMethodByExprs which mirrors
1047+
// CallInstanceMethod but uses ctx.Call so by-ref params
1048+
// bind correctly.
1049+
receiver := f.pop()
1050+
if ins.C() != 0 && (receiver == nil || phpv.IsNull(receiver)) {
1051+
f.push(phpv.ZNULL.ZVal())
1052+
break
1053+
}
1054+
if receiver == nil || receiver.GetType() != phpv.ZtObject {
1055+
typeName := "null"
1056+
if receiver != nil {
1057+
typeName = receiver.GetType().TypeName()
1058+
}
1059+
name, _ := f.fn.Consts[ins.A()].(phpv.ZString)
1060+
return nil, false, phpobj.ThrowError(ctx, phpobj.Error,
1061+
fmt.Sprintf("Call to a member function %s() on %s", string(name), typeName))
1062+
}
1063+
name, ok := f.fn.Consts[ins.A()].(phpv.ZString)
1064+
if !ok {
1065+
return nil, false, fmt.Errorf("vm: OP_OBJECT_CALL_BY_EXPRS name const is %T not ZString", f.fn.Consts[ins.A()])
1066+
}
1067+
argsIdx := int(ins.B())
1068+
if argsIdx >= len(f.fn.SubArgs) {
1069+
return nil, false, fmt.Errorf("vm: OP_OBJECT_CALL_BY_EXPRS SubArgs index %d out of range", argsIdx)
1070+
}
1071+
res, err := compiler.CallInstanceMethodByExprs(ctx, receiver, name, f.fn.SubArgs[argsIdx])
1072+
if err != nil {
1073+
return nil, false, err
1074+
}
1075+
if res == nil {
1076+
res = phpv.ZNULL.ZVal()
1077+
}
1078+
f.push(res)
1079+
9721080
// --- foreach -------------------------------------------------
9731081
case OpForeachInit:
9741082
src := f.pop()

core/vm/vmcompiler/emit.go

Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -19,6 +19,7 @@ type emitter struct {
1919
subFns []*vm.Function
2020
subClosures []phpv.Runnable
2121
subASTs []phpv.Runnable
22+
subArgs [][]phpv.Runnable
2223
tryHandlers []vm.TryHandler
2324

2425
locs []vm.LocEntry
@@ -131,6 +132,17 @@ func (e *emitter) astIndex(r phpv.Runnable) uint16 {
131132
return idx
132133
}
133134

135+
// subArgsIndex registers a call-site argument expression list in the
136+
// SubArgs table. Used by the "ByExprs" call opcodes which hand the raw
137+
// AST argument expressions to ctx.Call so the binding layer can apply
138+
// by-ref / named / spread semantics. No dedup — each call site gets
139+
// its own slot.
140+
func (e *emitter) subArgsIndex(args []phpv.Runnable) uint16 {
141+
idx := uint16(len(e.subArgs))
142+
e.subArgs = append(e.subArgs, args)
143+
return idx
144+
}
145+
134146
// localIndex returns the local-table index for the named variable,
135147
// adding it on first use.
136148
func (e *emitter) localIndex(name phpv.ZString) uint16 {
@@ -194,6 +206,7 @@ func (e *emitter) finish(name phpv.ZString, numParams int) *vm.Function {
194206
SubFns: e.subFns,
195207
SubClosures: e.subClosures,
196208
SubASTs: e.subASTs,
209+
SubArgs: e.subArgs,
197210
LocsSparse: e.locs,
198211
TryHandlers: e.tryHandlers,
199212
NumParams: numParams,

0 commit comments

Comments
 (0)