diff --git a/internal/compiler/compiler.go b/internal/compiler/compiler.go index f1288210..0ab0e600 100644 --- a/internal/compiler/compiler.go +++ b/internal/compiler/compiler.go @@ -47,6 +47,10 @@ type builder struct { children []*bytecode.ISeq parent *builder isBlock bool + // forScope marks the each-block a `for` loop is compiled into. It is a block + // iseq, but `for` opens no Ruby scope: a variable first assigned inside the + // body belongs to the scope AROUND the loop, so declarations hop out of it. + forScope bool // borrowed marks a scope whose locals belong to somebody else's already-built // frame — the synthetic parent CompileWithLocals puts a Binding's locals in. // Its slots may be read and written but no new one may be added: the frame @@ -83,6 +87,19 @@ func (b *builder) resolve(name string) (depth, index int, ok bool) { return 0, 0, false } +// declOwner returns the scope a variable first assigned in b belongs to, and +// how many block hops away it is. `for` opens no scope of its own, so a name +// introduced in a `for` body is declared in the nearest enclosing scope that is +// not itself a `for` body — the surrounding block when there is one, else the +// method or top level. For any other builder the answer is b itself, at depth 0. +func (b *builder) declOwner() (owner *builder, depth int) { + owner, depth = b, 0 + for owner.forScope && owner.parent != nil { + owner, depth = owner.parent, depth+1 + } + return owner, depth +} + func (b *builder) emit(op bytecode.Op, a, bb int) int { b.insns = append(b.insns, bytecode.Instr{Op: op, A: a, B: bb}) return len(b.insns) - 1 @@ -460,12 +477,9 @@ func (c *Compiler) compileNode(n ast.Node) { case *ast.Assign: c.compileNode(v.Value) // Assign to an enclosing local if one is visible; otherwise create a - // new local in the current scope. - if depth, slot, ok := b.resolve(v.Name); ok { - b.emit(bytecode.OpSetLocal, slot, depth) - } else { - b.emit(bytecode.OpSetLocal, b.localSlot(v.Name), 0) - } + // new local in the current scope (or, inside a `for` body, in the scope + // around the loop — see declOwner). + c.storeLocal(v.Name) case *ast.MultiAssign: c.compileMultiAssign(v) case *ast.Begin: @@ -498,10 +512,7 @@ func (c *Compiler) compileNode(n ast.Node) { case *ast.OpAssign: // Allocate the slot before the read so a fresh `x ||= v` sees nil // rather than failing to resolve. - depth, slot, ok := b.resolve(v.Name) - if !ok { - slot, depth = b.localSlot(v.Name), 0 - } + slot, depth := c.bindLocal(v.Name) c.compileNode(&ast.BinaryExpr{Op: v.Op, Left: &ast.VarRef{Name: v.Name}, Right: v.Value}) b.emit(bytecode.OpSetLocal, slot, depth) case *ast.UnaryExpr: @@ -799,7 +810,8 @@ func (c *Compiler) compileForwardCall(v *ast.Call, fwdAt int) { } c.emitForwardArgsArray(v.Args[:fwdAt], v.Args[fwdAt+1:]) // Forward the block (&...blk; nil ⇒ no block) and send. - b.emit(bytecode.OpGetLocal, c.mustResolve(fwdBlockName), 0) + blkSlot, blkDepth := c.mustResolve(fwdBlockName) + b.emit(bytecode.OpGetLocal, blkSlot, blkDepth) b.emit(bytecode.OpSendArrayBlockArg, b.addName(v.Name), 0) } @@ -819,14 +831,16 @@ func (c *Compiler) emitForwardArgsArray(leading, trailing []ast.Node) { for _, a := range leading { appendOne(a) } - b.emit(bytecode.OpGetLocal, c.mustResolve(fwdRestName), 0) + restSlot, restDepth := c.mustResolve(fwdRestName) + b.emit(bytecode.OpGetLocal, restSlot, restDepth) b.emit(bytecode.OpSplatToArray, 0, 0) b.emit(bytecode.OpConcatArray, 0, 0) for _, a := range trailing { appendOne(a) } // Append **...kw as a trailing hash, but only when it has entries. - b.emit(bytecode.OpGetLocal, c.mustResolve(fwdKwName), 0) + kwSlot, kwDepth := c.mustResolve(fwdKwName) + b.emit(bytecode.OpGetLocal, kwSlot, kwDepth) b.emit(bytecode.OpDup, 0, 0) b.emit(bytecode.OpSend, b.addName("empty?"), 0) skip := b.emit(bytecode.OpBranchIf, 0, 0) @@ -839,14 +853,18 @@ func (c *Compiler) emitForwardArgsArray(leading, trailing []ast.Node) { } // mustResolve resolves a (compiler-synthesised) local that is guaranteed to -// exist in scope, returning its slot. It panics via fail otherwise, which only -// happens on a compiler bug. -func (c *Compiler) mustResolve(name string) int { - _, slot, ok := c.cur().resolve(name) +// exist in scope, returning its slot AND its block depth. The depth matters: +// `def f(...)` parks the forwarded arguments in the method's own locals, so a +// `g(...)` written inside a block — including the each-block a `for` compiles +// into — reads them from an enclosing frame, and a depth of 0 would index the +// block's own frame instead. It panics via fail when the name is out of scope, +// which only happens on a compiler bug. +func (c *Compiler) mustResolve(name string) (slot, depth int) { + depth, slot, ok := c.cur().resolve(name) if !ok { c.fail("argument forwarding outside a def(...) method") } - return slot + return slot, depth } // anonLocal returns a VarRef reading the enclosing method's anonymous-forward @@ -1292,10 +1310,7 @@ func (c *Compiler) compileMatchWithCaptures(v *ast.BinaryExpr, names []string) { b.emit(bytecode.OpSend, b.addName("=~"), 1) // leaves the match position (or nil) for _, name := range names { // Reuse an existing local of this name, else declare it here. - depth, slot, ok := b.resolve(name) - if !ok { - slot, depth = b.localSlot(name), 0 - } + slot, depth := c.bindLocal(name) b.emit(bytecode.OpGetConst, b.addName("Regexp"), 0) b.emit(bytecode.OpPushConst, b.addConst(object.SymVal(name)), 0) b.emit(bytecode.OpSend, b.addName("last_match"), 1) // $~[:name], nil if unmatched @@ -1535,6 +1550,24 @@ func (c *Compiler) compileBlock(blk *ast.Block) int { if blk.BlockParam != "" { b.blockSlot = b.localSlot(blk.BlockParam) } + // Block-local variables (`{ |a; b, c| … }`) take slots of their own right + // after the parameters, and are NOT parameters: they start nil and shadow any + // enclosing binding of the same name, so an assignment inside the block cannot + // reach the outer one. MRI builds exactly this table — prism_compile.c + // v3_4_0:6325-6334 walks the block-parameters node's `locals` list and calls + // pm_insert_local_index for each, after the requireds/optionals/rest/posts and + // before "fill in any locals we missed". + // + // A block-local that repeats a parameter name (`{ |foo; foo| }`) is a + // SyntaxError in MRI, raised by the PARSER (parse.y v3_4_0:13621 new_bv → + // shadowing_lvar_0), and go-ruby-parser does not raise it yet — which is the + // one ruby/spec example in "Block-local variables" still failing. Nothing is + // forced here on its behalf: localLookup answers with the first slot of a + // name, so the repeated declaration is simply dead and the name keeps meaning + // the parameter. Raising belongs where MRI raises it. + for _, name := range blk.Locals { + b.localSlot(name) + } // Optional keyword params: the VM binds the supplied ones natively, so the // prologue only fills in defaults for the absent ones. for i, kp := range kwParams { @@ -1660,7 +1693,8 @@ func (c *Compiler) compileSuper(v *ast.Super) { // built args array (with the captured &...blk overriding the frame block). if fwdAt := forwardIndex(v.Args); fwdAt >= 0 { c.emitForwardArgsArray(v.Args[:fwdAt], v.Args[fwdAt+1:]) - b.emit(bytecode.OpGetLocal, c.mustResolve(fwdBlockName), 0) + blkSlot, blkDepth := c.mustResolve(fwdBlockName) + b.emit(bytecode.OpGetLocal, blkSlot, blkDepth) at := b.emit(bytecode.OpInvokeSuperArray, 0, 0) b.insns[at].C = 1 return @@ -2468,6 +2502,19 @@ func (c *Compiler) storeMultiTarget(target ast.Node) { b.emit(bytecode.OpPop, 0, 0) pre := c.masgnPre[target] c.pushMasgnRecv(target, t.Recv) + // `for o&.x in …`: a nil receiver skips the setter and leaves nil as this + // arm's value. MRI reaches the same store arm for a safe-navigation for-var + // (prism_compile.c v3_4_0:4862 routes PM_CALL_TARGET_NODE through + // pm_compile_target_node, which honours PM_CALL_NODE_FLAGS_SAFE_NAVIGATION); + // a multiple assignment cannot get here, since `a&.x, b = …` is a + // SyntaxError. OpBranchNil pops its tested copy, so the receiver left + // underneath is the nil result, matching the ordinary `o&.x = v` lowering. + safeSkip := -1 + if t.Safe { + b.emit(bytecode.OpDup, 0, 0) + safeSkip = b.emit(bytecode.OpBranchNil, 0, 0) + defer func() { b.patch(safeSkip, b.here()) }() + } if pre != nil && pre.argsArr >= 0 { // Dynamic arity, argument array already built before the RHS: append // the assigned value and send. OpConcatArray always allocates a fresh @@ -2516,15 +2563,23 @@ func (c *Compiler) storeMultiTarget(target ast.Node) { } } -// storeLocal emits a SetLocal for name, resolving an existing local or -// allocating a fresh slot, mirroring ast.Assign. -func (c *Compiler) storeLocal(name string) { +// bindLocal returns the slot and block depth to write name at: an existing +// visible local if there is one, else a freshly declared slot in the scope +// declOwner names (the current builder, unless it is a `for` body). +func (c *Compiler) bindLocal(name string) (slot, depth int) { b := c.cur() if depth, slot, ok := b.resolve(name); ok { - b.emit(bytecode.OpSetLocal, slot, depth) - } else { - b.emit(bytecode.OpSetLocal, b.localSlot(name), 0) + return slot, depth } + owner, depth := b.declOwner() + return owner.localSlot(name), depth +} + +// storeLocal emits a SetLocal for name, resolving an existing local or +// allocating a fresh slot, mirroring ast.Assign. +func (c *Compiler) storeLocal(name string) { + slot, depth := c.bindLocal(name) + c.cur().emit(bytecode.OpSetLocal, slot, depth) } // compileBegin compiles begin/rescue/else/ensure. When an ensure clause is @@ -2592,11 +2647,7 @@ func (c *Compiler) compileBeginRescue(v *ast.Begin) { } switch { case clause.Var != "": // bind the exception to a local (reusing an existing slot) - if depth, slot, ok := b.resolve(clause.Var); ok { - b.emit(bytecode.OpSetLocal, slot, depth) - } else { - b.emit(bytecode.OpSetLocal, b.localSlot(clause.Var), 0) - } + c.storeLocal(clause.Var) case clause.VarTarget != nil: // `rescue => @e` / `=> $g` / `=> Foo::E`: a non-local target c.storeMultiTarget(clause.VarTarget) } @@ -2682,27 +2733,39 @@ func (c *Compiler) compileFor(v *ast.For) { // The loop variables live in the enclosing scope; ensure each has a slot // there now (before compiling the block) so the block resolves them by depth. // A `for` does not open a scope, so an absent variable is created in the - // nearest method/top-level scope (the first non-block ancestor), making it - // leak past the whole construct — including out of any enclosing blocks, e.g. - // an outer `for` — exactly as MRI does. + // nearest scope that is not itself a `for` body, making it leak past the whole + // construct — including out of an enclosing `for`, but NOT out of an enclosing + // block, which is a scope. That is what declOwner walks. parent := c.cur() - owner := parent - for owner.isBlock && owner.parent != nil { - owner = owner.parent - } - for _, name := range v.Vars { + owner, _ := parent.declOwner() + declare := func(name string) { if _, _, ok := parent.resolve(name); !ok { owner.localSlot(name) } } + for _, name := range v.Vars { + declare(name) + } + declareForTargetLocals(v.Target, declare) // Receiver: the iterable. c.compileNode(v.Iter) - // Synthesize the each-block as a child ISeq with one hidden parameter. + // Synthesize the each-block as a child ISeq with one hidden parameter. MRI + // shapes that parameter by the kind of loop variable (prism_compile.c + // v3_4_0:6293-6301): a lone local target bumps lead_num, so the block takes + // ONE required argument and gets the yielded value as it stands; anything + // else sets has_rest, so the block collects ALL the yielded values into an + // Array and the destructuring below works from that. c.push(newBlockBuilder("", []string{"...for"}, parent)) b := c.cur() - b.numRequired = 1 + b.forScope = true + if forTakesOneValue(v) { + b.numRequired = 1 + } else { + b.numRequired = 0 + b.splatIndex = 0 + } c.ctxs = append(c.ctxs, &loopCtx{kind: ctxBlock}) - c.compileForVars(v.Vars) + c.compileForTarget(v) c.compileBody(v.Body) c.ctxs = c.ctxs[:len(c.ctxs)-1] b.emit(bytecode.OpReturn, 0, 0) @@ -2713,25 +2776,107 @@ func (c *Compiler) compileFor(v *ast.For) { parent.insns[at].C = idx + 1 // C-1 indexes Children } -// compileForVars binds the yielded value (the hidden first block local) to the -// `for` loop variables, which resolve to enclosing-scope locals. A single -// variable takes the value directly; multiple variables destructure it like a -// MultiAssign (the value is splatted into an Array and expanded). -func (c *Compiler) compileForVars(vars []string) { +// forTakesOneValue reports whether the loop variable is a single plain local +// (`for i in …`), the one shape whose each-block takes a required parameter +// rather than collecting the yielded values into a rest Array. +func forTakesOneValue(v *ast.For) bool { + return v.Target == nil && len(v.Vars) == 1 +} + +// declareForTargetLocals calls declare for every plain local name a For.Target +// assigns, so the name exists in the enclosing scope before the loop body is +// compiled — `for a, *b in […]` leaves both a and b visible after the loop, like +// any other `for` variable. Non-local targets (ivars, globals, constants, setter +// calls) declare nothing. +func declareForTargetLocals(target ast.Node, declare func(string)) { + switch t := target.(type) { + case *ast.VarRef: + declare(t.Name) + case *ast.MultiAssign: + if len(t.Targets) != len(t.Names) { + // All-locals form: Names carries every binding. The parser only builds + // it for a list of plain locals — `for i, in …` — so no entry is the + // empty name a nameless `*` would carry; every shape that HAS one (`for + // i, * in …`) comes with a parallel Targets list holding a nil there. + for _, name := range t.Names { + declare(name) + } + return + } + for _, sub := range t.Targets { + declareForTargetLocals(sub, declare) + } + } +} + +// compileForTarget binds the values yielded to the each-block to the `for` loop +// variable, which resolves to enclosing-scope locals (or to whatever else the +// target names). MRI emits this inside the block iseq, before the body and +// before the B_CALL event: prism_compile.c v3_4_0:6489-6494 calls +// pm_compile_for_node_index, whose three arms (v3_4_0:4844-4924) are the three +// below. +func (c *Compiler) compileForTarget(v *ast.For) { b := c.cur() - if len(vars) == 1 { - b.emit(bytecode.OpGetLocal, 0, 0) // the hidden parameter - c.storeLocal(vars[0]) + // A lone local: the block took the value as a required parameter, so it is + // already the value to bind (PM_LOCAL_VARIABLE_TARGET_NODE: getlocal, write). + if forTakesOneValue(v) { + b.emit(bytecode.OpGetLocal, 0, 0) + c.storeLocal(v.Vars[0]) b.emit(bytecode.OpPop, 0, 0) return } - b.emit(bytecode.OpGetLocal, 0, 0) - b.emit(bytecode.OpSplatToArray, 0, 0) - b.emit(bytecode.OpExpandArray, len(vars), 0) - for _, name := range vars { - c.storeLocal(name) + names, targets, splatIndex := v.Vars, []ast.Node(nil), -1 + switch t := v.Target.(type) { + case nil: + // Several plain locals (`for a, b in …`): MRI's multi-target arm. + case *ast.MultiAssign: + names, splatIndex = t.Names, t.SplatIndex + if len(t.Targets) == len(t.Names) { + targets = t.Targets + } + default: + // A single non-local target (`for @v in …`, `for a[0] in …`): MRI pushes + // the rest Array and runs `expandarray 1, 0` on it, taking the first + // yielded value and discarding the rest, then performs the write. + b.emit(bytecode.OpGetLocal, 0, 0) + b.emit(bytecode.OpExpandArray, 1, 0) + c.storeMultiTarget(v.Target) b.emit(bytecode.OpPop, 0, 0) + return } + c.pushForMasgnOperand() + c.expandAndStore(names, targets, splatIndex) +} + +// pushForMasgnOperand pushes the value a multi-target `for` destructures, from +// the rest Array of yielded values held in the block's hidden parameter. +// +// MRI's sequence (compile.c compile_for_masgn, ruby/ruby v3_4_0:8302-8329, and +// the identical prism arm at prism_compile.c v3_4_0:4881-4919) is +// +// (args.length == 1 && Array.try_convert(args[0])) || args +// +// followed by expandarray. The try_convert half is redundant in front of an +// expandarray that already performs it: when args holds exactly one element, +// expanding args[0] gives the converted Array if args[0] has one, and otherwise +// the one-element list [args[0]] — which is what expanding args itself would +// have given. So the same lowering is `args.length == 1 ? args[0] : args`, with +// the conversion left where it always was, in expandarray. Both halves use the +// same sends MRI uses (idLength and idAREF), so a redefined Array#length or +// Array#[] is honoured here exactly as it is there. +func (c *Compiler) pushForMasgnOperand() { + b := c.cur() + b.emit(bytecode.OpGetLocal, 0, 0) // the hidden *rest parameter + b.emit(bytecode.OpDup, 0, 0) + b.emit(bytecode.OpSend, b.addName("length"), 0) + b.emit(bytecode.OpPushConst, b.addConst(object.IntValue(1)), 0) + b.emit(bytecode.OpEq, 0, 0) + notSingle := b.emit(bytecode.OpBranchUnless, 0, 0) + b.emit(bytecode.OpPushConst, b.addConst(object.IntValue(0)), 0) + b.emit(bytecode.OpSend, b.addName("[]"), 1) + done := b.emit(bytecode.OpJump, 0, 0) + b.patch(notSingle, b.here()) + b.patch(done, b.here()) } // Reserved local names for argument forwarding (`def f(...)`): they capture the diff --git a/internal/compiler/defined.go b/internal/compiler/defined.go index 306e8c4e..c230b5bd 100644 --- a/internal/compiler/defined.go +++ b/internal/compiler/defined.go @@ -14,6 +14,7 @@ import ( // sub-expression maps to nil rather than propagating. func (c *Compiler) compileDefined(operand ast.Node) { b := c.cur() + c.declareDefinedLocals(operand) switch v := operand.(type) { case *ast.NilLit: c.pushDefinedTag("nil") @@ -44,6 +45,26 @@ func (c *Compiler) compileDefined(operand ast.Node) { *ast.ScopedConstAssign, *ast.IvarAssign, *ast.CVarAssign, *ast.GVarAssign: c.pushDefinedTag("assignment") case *ast.Call: + // A compound assignment to an attribute or an index (`a.b += 1`, + // `a[:b] ||= 1`) reaches the compiler as the parser's textual desugaring + // into a setter Call, but MRI tags it "assignment", not "method": + // PM_CALL_OPERATOR_WRITE_NODE / PM_CALL_OR_WRITE_NODE / + // PM_CALL_AND_WRITE_NODE and their PM_INDEX_* counterparts are all in the + // DEFINED_ASGN group (prism_compile.c v3_4_0:4052-4093, matching + // compile.c's NODE_OP_ASGN1/NODE_OP_ASGN2 at v3_4_0:6151-6164). A PLAIN + // `a[0] = 1` is not in that group — it is an ordinary attrasgn call, and + // stays "method". The same shared-node recognisers compileCall uses tell + // the two apart; see opassign.go for why identity is the test. + if isSetterCall(v) { + if _, _, ok := indexOpAssign(v); ok { + c.pushDefinedTag("assignment") + return + } + if _, ok := attrOpAssign(v); ok { + c.pushDefinedTag("assignment") + return + } + } c.compileDefinedCall(v) case *ast.BinaryExpr: c.compileDefinedBinary(v) @@ -107,12 +128,54 @@ func (c *Compiler) compileDefinedCall(v *ast.Call) { // every other binary operator is a method on the left operand. func (c *Compiler) compileDefinedBinary(v *ast.BinaryExpr) { if v.Op == "&&" || v.Op == "||" { + // `A ||= v` and `A::B ||= v` are not written as an assignment node: the + // parser desugars them to `(defined?(A) && A) || A = v`, so they arrive + // here as a `||`. MRI still calls them assignments — PM_CONSTANT_OR_WRITE + // and PM_CONSTANT_PATH_OR_WRITE are in the DEFINED_ASGN group + // (prism_compile.c v3_4_0:4059 and 4063; compile.c's NODE_OP_CDECL at + // v3_4_0:6162 says the same) — and ruby/spec pins it. + if constOrAssign(v) { + c.pushDefinedTag("assignment") + return + } c.pushDefinedTag("expression") return } c.compileDefinedReceiverMethod(v.Left, v.Op) } +// constOrAssign recognises the parser's desugaring of `A ||= v` / `A::B ||= v`: +// `(defined?(A) && A) || A = v`, with the constant READ shared (same AST +// pointer) between the `defined?` probe, the `&&`'s right operand and — for the +// scoped form — the assignment's target. That identity is what tells the +// desugaring apart from a hand-written `(defined?(A) && A) || A = v`, whose +// three reads are distinct nodes; it is the same test opassign.go uses for the +// index and attribute compound assignments. +func constOrAssign(v *ast.BinaryExpr) bool { + if v.Op != "||" { + return false + } + guard, ok := v.Left.(*ast.BinaryExpr) + if !ok || guard.Op != "&&" { + return false + } + probe, ok := guard.Left.(*ast.Call) + if !ok || probe.Name != "defined?" || probe.Recv != nil || len(probe.Args) != 1 { + return false + } + if probe.Args[0] != guard.Right { + return false + } + switch w := v.Right.(type) { + case *ast.ConstAssign: + read, ok := guard.Right.(*ast.ConstRef) + return ok && read.Name == w.Name + case *ast.ScopedConstAssign: + return w.Target == guard.Right + } + return false +} + // compileDefinedReceiverMethod emits, under the guard, the response probe for // `recv.name`. recv == nil means an implicit self receiver. An explicit receiver // is itself subject to defined? first (MRI: `defined?(@x.m)` is nil when `@x` is @@ -151,3 +214,74 @@ func (c *Compiler) guarded(body func(gb *builder)) { parent.children = append(parent.children, child) parent.emit(bytecode.OpDefinedGuard, idx, 0) } + +// declareDefinedLocals declares the locals an operand of `defined?` introduces. +// +// `defined?` inspects its operand's syntactic kind instead of compiling it, so +// the assignments inside it emit no code — but they still DECLARE. MRI builds +// its local table in the parser, which walks the operand like any other code, +// so `defined?(a += 1)` leaves `a` as a (nil) local of the enclosing scope and a +// later bare `a` is a local-variable read rather than a method call. rbgo +// declares locals as it compiles, so without this walk the name has no slot and +// the next mention of it fails to compile — which is where +// language/defined_spec.rb stopped. +// +// Only assignment targets are declared; nothing is evaluated. The walk descends +// through the value and receiver/argument positions an assignment can hide in, +// which is what the parser's own walk amounts to. +func (c *Compiler) declareDefinedLocals(n ast.Node) { + switch v := n.(type) { + case *ast.Assign: + c.declareLocal(v.Name) + c.declareDefinedLocals(v.Value) + case *ast.OpAssign: + c.declareLocal(v.Name) + c.declareDefinedLocals(v.Value) + case *ast.MultiAssign: + for _, name := range v.Names { + c.declareLocal(name) + } + for _, t := range v.Targets { + c.declareDefinedLocals(t) // a nested group declares its own names + } + for _, val := range v.Values { + c.declareDefinedLocals(val) + } + case *ast.IvarAssign: + c.declareDefinedLocals(v.Value) + case *ast.CVarAssign: + c.declareDefinedLocals(v.Value) + case *ast.GVarAssign: + c.declareDefinedLocals(v.Value) + case *ast.ConstAssign: + c.declareDefinedLocals(v.Value) + case *ast.ScopedConstAssign: + c.declareDefinedLocals(v.Value) + case *ast.Call: + if v.Recv != nil { + c.declareDefinedLocals(v.Recv) + } + for _, a := range v.Args { + c.declareDefinedLocals(a) + } + case *ast.BinaryExpr: + c.declareDefinedLocals(v.Left) + c.declareDefinedLocals(v.Right) + case *ast.UnaryExpr: + c.declareDefinedLocals(v.Operand) + } +} + +// declareLocal gives name a slot if it does not already have one, without +// emitting anything. An empty name is the nameless `*` of `a, * = …`. +func (c *Compiler) declareLocal(name string) { + if name == "" { + return + } + b := c.cur() + if _, _, ok := b.resolve(name); ok { + return + } + owner, _ := b.declOwner() + owner.localSlot(name) +} diff --git a/internal/compiler/wave26_for_defined_test.go b/internal/compiler/wave26_for_defined_test.go new file mode 100644 index 00000000..3410b46b --- /dev/null +++ b/internal/compiler/wave26_for_defined_test.go @@ -0,0 +1,499 @@ +package compiler + +import ( + "strings" + "testing" + + "github.com/go-embedded-ruby/ruby/internal/bytecode" + "github.com/go-embedded-ruby/ruby/internal/object" + "github.com/go-ruby-parser/parser" + "github.com/go-ruby-parser/parser/ast" +) + +// compileSrc parses and compiles src, failing the test on either error. +func compileSrc(t *testing.T, src string) *bytecode.ISeq { + t.Helper() + prog, err := parser.Parse(src) + if err != nil { + t.Fatalf("parse %q: %v", src, err) + } + iseq, err := Compile(prog) + if err != nil { + t.Fatalf("compile %q: %v", src, err) + } + return iseq +} + +// hasLocal reports whether iseq's local table holds name. +func hasLocal(iseq *bytecode.ISeq, name string) bool { + for _, l := range iseq.Locals { + if l == name { + return true + } + } + return false +} + +// forBlock returns the child ISeq a `for` compiles its body into. A `for` emits +// exactly one child at the top level of these sources. +func forBlock(t *testing.T, iseq *bytecode.ISeq) *bytecode.ISeq { + t.Helper() + if len(iseq.Children) != 1 { + t.Fatalf("expected exactly one child ISeq, got %d", len(iseq.Children)) + } + return iseq.Children[0] +} + +// Every shape of loop variable declares its plain locals in the scope AROUND +// the loop, so they outlive it — `for` opens no scope. The shapes come in +// through two different parser fields: the simple lists through For.Vars, and +// everything Vars cannot spell through For.Target. +func TestForDeclaresLoopVariablesInTheEnclosingScope(t *testing.T) { + cases := []struct { + src string + want []string + unset []string + }{ + {src: "for i in [1]; end", want: []string{"i"}}, + {src: "for a, b in [[1,2]]; end", want: []string{"a", "b"}}, + {src: "for i, in [[1,2]]; end", want: []string{"i"}}, + {src: "for i, * in [[1,2]]; end", want: []string{"i"}}, + {src: "for i, *j in [[1,2]]; end", want: []string{"i", "j"}}, + {src: "for i, *j, k in [[1,2,3,4]]; end", want: []string{"i", "j", "k"}}, + {src: "for (i, j, k) in [[1,2,3]]; end", want: []string{"i", "j", "k"}}, + {src: "for i, (j, k) in [[1,[2,3]]]; end", want: []string{"i", "j", "k"}}, + // Non-local targets bind nothing in the local table. + {src: "for @v in [1]; end", unset: []string{"@v", "v"}}, + {src: "for $v in [1]; end", unset: []string{"$v", "v"}}, + {src: "for @@v in [1]; end", unset: []string{"@@v", "v"}}, + {src: "for V in [1]; end", unset: []string{"V"}}, + // A variable FIRST ASSIGNED in the body belongs to the scope around the + // loop too, and hops out of an enclosing `for` as well. + {src: "for a in [6]; c = a; end", want: []string{"a", "c"}}, + {src: "for a in [6]; for b in [7]; c = a * b; end; end", want: []string{"a", "b", "c"}}, + {src: "for a in [6]; d += 1; end", want: []string{"a", "d"}}, + {src: "for a in [6]; begin; raise; rescue => e; end; end", want: []string{"a", "e"}}, + } + for _, tc := range cases { + iseq := compileSrc(t, tc.src) + for _, name := range tc.want { + if !hasLocal(iseq, name) { + t.Errorf("%s: %q is not a local of the enclosing scope (locals %v)", tc.src, name, iseq.Locals) + } + } + for _, name := range tc.unset { + if hasLocal(iseq, name) { + t.Errorf("%s: %q should not be a local (locals %v)", tc.src, name, iseq.Locals) + } + } + } +} + +// An enclosing BLOCK is a scope, so a `for` inside one declares into the block, +// not into the method around it. The for_spec example "can be nested with blocks +// in between" pins exactly this. +func TestForInsideABlockDeclaresIntoTheBlock(t *testing.T) { + iseq := compileSrc(t, "[1].each { for c in [3]; c1 = c; end }") + if hasLocal(iseq, "c") || hasLocal(iseq, "c1") { + t.Fatalf("c/c1 leaked past the block into the top level: %v", iseq.Locals) + } + blk := iseq.Children[0] + if !hasLocal(blk, "c") || !hasLocal(blk, "c1") { + t.Fatalf("c/c1 are not locals of the enclosing block: %v", blk.Locals) + } +} + +// The each-block's hidden parameter is shaped by the kind of loop variable: a +// lone local takes one REQUIRED argument, everything else collects the yielded +// values into a REST Array. That is what lets `for x, y in o` see both values of +// an `each` that yields two. +func TestForBlockParameterShape(t *testing.T) { + cases := []struct { + src string + required int + splat int + }{ + {"for i in [1]; end", 1, -1}, + {"for a, b in [[1,2]]; end", 0, 0}, + {"for i, in [[1,2]]; end", 0, 0}, + {"for @v in [1]; end", 0, 0}, + } + for _, tc := range cases { + blk := forBlock(t, compileSrc(t, tc.src)) + if blk.NumRequired != tc.required || blk.SplatIndex != tc.splat { + t.Errorf("%s: NumRequired=%d SplatIndex=%d, want %d/%d", + tc.src, blk.NumRequired, blk.SplatIndex, tc.required, tc.splat) + } + } +} + +// A single non-local target takes the FIRST yielded value and drops the rest, +// which MRI spells `expandarray 1, 0` on the rest Array. A multi-target instead +// runs the length test that picks between args[0] and args. +func TestForTargetLowering(t *testing.T) { + blk := forBlock(t, compileSrc(t, "for @v in [1]; end")) + var sawExpand, sawSetIvar bool + for _, in := range blk.Insns { + if in.Op == bytecode.OpExpandArray && in.A == 1 && in.B == 0 && in.C == 0 { + sawExpand = true + } + if in.Op == bytecode.OpSetIvar { + sawSetIvar = true + } + } + if !sawExpand || !sawSetIvar { + t.Fatalf("for @v: expandarray(1,0)=%v setivar=%v", sawExpand, sawSetIvar) + } + + blk = forBlock(t, compileSrc(t, "for a, b in [[1,2]]; end")) + var sawLength, sawAref bool + for _, in := range blk.Insns { + if in.Op == bytecode.OpSend { + switch blk.Names[in.A] { + case "length": + sawLength = true + case "[]": + sawAref = true + } + } + } + if !sawLength || !sawAref { + t.Fatalf("multi-target for: length=%v aref=%v", sawLength, sawAref) + } +} + +// `for o&.x in m` must skip the setter on a nil receiver instead of raising, so +// the store arm emits the same nil guard an ordinary `o&.x = v` does. A +// multiple assignment never reaches that branch — `a&.x, b = …` is a SyntaxError. +func TestForSafeNavigationTargetGuardsOnNil(t *testing.T) { + for _, tc := range []struct { + src string + want bool + }{ + {"o = nil; for o&.x in [1]; end", true}, + {"o = nil; for o.x in [1]; end", false}, + } { + blk := forBlock(t, compileSrc(t, tc.src)) + var got bool + for _, in := range blk.Insns { + if in.Op == bytecode.OpBranchNil { + got = true + } + } + if got != tc.want { + t.Errorf("%s: BranchNil present = %v, want %v", tc.src, got, tc.want) + } + } +} + +// An index or attribute target is written through its setter, with the receiver +// and the index evaluated inside the block — once per iteration, as MRI does. +func TestForIndexAndAttributeTargets(t *testing.T) { + for _, tc := range []struct{ src, setter string }{ + {"arr = []; for arr[1] in [1]; end", "[]="}, + {"o = nil; for o.x in [1]; end", "x="}, + } { + blk := forBlock(t, compileSrc(t, tc.src)) + var got bool + for _, in := range blk.Insns { + if in.Op == bytecode.OpSend && blk.Names[in.A] == tc.setter { + got = true + } + } + if !got { + t.Errorf("%s: no %q send in the each-block", tc.src, tc.setter) + } + } +} + +// declareForTargetLocals ignores a target kind that names no local. The parser +// never builds this shape (a For.Target is always an assignable node), so it is +// synthesized to reach the walk's default arm. +func TestDeclareForTargetLocalsIgnoresANonTarget(t *testing.T) { + var got []string + declareForTargetLocals(&ast.IntLit{Value: 1}, func(n string) { got = append(got, n) }) + declareForTargetLocals(nil, func(n string) { got = append(got, n) }) + if len(got) != 0 { + t.Fatalf("expected no declarations, got %v", got) + } +} + +// `def f(...)` parks the forwarded arguments in the METHOD's locals, so a +// forwarding call written inside a block — a `for` body as much as any other — +// must read them from the enclosing frame. A depth of 0 indexes the block's own +// frame, which held one local and crashed. +func TestForwardingCallInsideABlockReadsTheMethodFrame(t *testing.T) { + for _, src := range []string{ + "def f(...); [1].each { g(...) }; end", + "def f(...); for x in [1]; g(...); end; end", + "class C; def f(...); [1].each { super(...) }; end; end", + } { + blocks := blockISeqs(compileSrc(t, src)) + if len(blocks) == 0 { + t.Fatalf("%s: no block ISeq compiled", src) + } + var sawDepth bool + for _, blk := range blocks { + for _, in := range blk.Insns { + if in.Op == bytecode.OpGetLocal && in.B > 0 { + sawDepth = true + } + } + } + if !sawDepth { + t.Errorf("%s: the block reads no enclosing-frame local; the forward locals were resolved at depth 0", src) + } + } +} + +// blockISeqs collects every block/for child ISeq in the tree. +func blockISeqs(iseq *bytecode.ISeq) []*bytecode.ISeq { + var out []*bytecode.ISeq + for _, ch := range iseq.Children { + if ch.Name == "" || ch.Name == "" { + out = append(out, ch) + } + out = append(out, blockISeqs(ch)...) + } + return out +} + +// mustResolve reports a compiler bug rather than returning a bogus slot when the +// forwarding locals are not in scope. +func TestMustResolveOutsideAForwardingMethod(t *testing.T) { + c := &Compiler{} + c.push(newBuilder("t", nil)) + defer func() { + r := recover() + if r == nil { + t.Fatal("expected mustResolve to panic outside a def(...) method") + } + if e, ok := r.(compileError); !ok || !strings.Contains(e.msg, "argument forwarding") { + t.Fatalf("expected an argument-forwarding compileError, got %#v", r) + } + }() + c.mustResolve(fwdRestName) +} + +// Block-local variables (`{ |a; b| … }`) are slots of the BLOCK, not parameters, +// and shadow any enclosing binding of the same name. +func TestBlockLocalsTakeTheirOwnSlots(t *testing.T) { + iseq := compileSrc(t, "x = 1; [1].each { |q; x, y| x = 9; y = 8 }") + blk := iseq.Children[0] + for _, name := range []string{"q", "x", "y"} { + if !hasLocal(blk, name) { + t.Fatalf("%q is not a local of the block: %v", name, blk.Locals) + } + } + if blk.NumRequired != 1 { + t.Fatalf("block locals must not be parameters: NumRequired=%d", blk.NumRequired) + } + // The write to x must reach the block's own slot (depth 0), not the outer one. + for _, in := range blk.Insns { + if in.Op == bytecode.OpSetLocal && in.B != 0 { + t.Fatalf("a block-local write escaped to depth %d", in.B) + } + } +} + +// `defined?` inspects its operand instead of compiling it, but the assignments +// inside it still DECLARE — MRI builds its local table in the parser. Without +// that, the next mention of the name has no slot and fails to compile. +func TestDefinedDeclaresTheLocalsItsOperandIntroduces(t *testing.T) { + cases := []struct { + src string + want []string + }{ + {"defined?(a += 1)", []string{"a"}}, + {"defined?(a = 1)", []string{"a"}}, + {"defined?(a, b = 1, 2)", []string{"a", "b"}}, + {"defined?(a, (b, c) = 1, [2, 3])", []string{"a", "b", "c"}}, + {"defined?(a, * = 1, 2)", []string{"a"}}, + {"defined?(a = (b = 1))", []string{"a", "b"}}, + {"defined?(a += (b = 1))", []string{"a", "b"}}, + {"defined?(foo(a = 1))", []string{"a"}}, + {"defined?(foo.bar(a = 1))", []string{"a"}}, + {"defined?(1 + (a = 1))", []string{"a"}}, + {"defined?(-(a = 1))", []string{"a"}}, + {"defined?(@v = (a = 1))", []string{"a"}}, + {"defined?(@@v = (a = 1))", []string{"a"}}, + {"defined?($v = (a = 1))", []string{"a"}}, + {"defined?(V = (a = 1))", []string{"a"}}, + {"defined?(Object::V = (a = 1))", []string{"a"}}, + {"defined?(a = 1); defined?(a = 2)", []string{"a"}}, + } + for _, tc := range cases { + iseq := compileSrc(t, tc.src) + for _, name := range tc.want { + if !hasLocal(iseq, name) { + t.Errorf("%s: %q got no slot (locals %v)", tc.src, name, iseq.Locals) + } + } + } + // And the point of it: the name is then readable as a local. + compileSrc(t, "defined?(a += 1); a") + compileSrc(t, "defined?(a += 1); defined?(a.b += 1)") +} + +// A defined? operand that assigns nothing declares nothing. +func TestDefinedDeclaresNothingForAPlainOperand(t *testing.T) { + iseq := compileSrc(t, "defined?(1 + 2)") + if len(iseq.Locals) != 0 { + t.Fatalf("expected no locals, got %v", iseq.Locals) + } +} + +// A compound assignment to an attribute or an index arrives as the parser's +// desugaring into a setter Call, but MRI tags it "assignment", not "method". A +// PLAIN `a[0] = 1` is an ordinary attrasgn call and stays "method". +func TestDefinedTagsDesugaredCompoundAssignment(t *testing.T) { + for _, src := range []string{ + "a = nil; defined?(a.b += 1)", + "a = nil; defined?(a.b ||= 1)", + "a = nil; defined?(a.b &&= 1)", + "a = nil; defined?(a[:b] += 1)", + "a = nil; defined?(a[:b] ||= 1)", + "a = nil; defined?(a[:b] &&= 1)", + } { + iseq := compileSrc(t, src) + if !hasDefinedTag(iseq, "assignment") { + t.Errorf("%s: no \"assignment\" tag (tags %v)", src, definedTags(iseq)) + } + if hasOp(iseq, bytecode.OpDefinedMethod) { + t.Errorf("%s: probed for a method; MRI answers from the syntax alone", src) + } + } + // A plain index or attribute assignment is NOT in MRI's DEFINED_ASGN group. + for _, src := range []string{"a = []; defined?(a[0] = 1)", "a = nil; defined?(a.b)"} { + iseq := compileSrc(t, src) + if hasDefinedTag(iseq, "assignment") { + t.Errorf("%s: tagged \"assignment\"; MRI says \"method\"", src) + } + // "method" is not a compile-time constant: OpDefinedMethod produces it at + // run time from the response test, so the probe itself is the witness. + if !hasOp(iseq, bytecode.OpDefinedMethod) { + t.Errorf("%s: expected a method probe (tags %v)", src, definedTags(iseq)) + } + } +} + +// definedTags collects every defined? tag String pushed anywhere in the tree. +func definedTags(iseq *bytecode.ISeq) []string { + var out []string + for _, v := range iseq.Consts { + if s, ok := v.(*object.String); ok { + switch s.Str() { + case "assignment", "method", "expression", "local-variable": + out = append(out, s.Str()) + } + } + } + for _, ch := range iseq.Children { + out = append(out, definedTags(ch)...) + } + return out +} + +func hasDefinedTag(iseq *bytecode.ISeq, tag string) bool { + for _, t := range definedTags(iseq) { + if t == tag { + return true + } + } + return false +} + +func hasOp(iseq *bytecode.ISeq, op bytecode.Op) bool { + for _, in := range iseq.Insns { + if in.Op == op { + return true + } + } + for _, ch := range iseq.Children { + if hasOp(ch, op) { + return true + } + } + return false +} + +// `A ||= v` is not an assignment NODE: the parser desugars it to +// `(defined?(A) && A) || A = v`, with the constant read shared between the +// probe, the `&&`'s right operand and (for the scoped form) the assignment's +// target. MRI still calls it an assignment. A hand-written expression of the +// same shape has three distinct reads and stays "expression". +func TestDefinedTagsDesugaredConstantOrAssign(t *testing.T) { + for _, src := range []string{"defined?(A ||= 1)", "defined?(Object::A ||= 1)"} { + if iseq := compileSrc(t, src); !hasDefinedTag(iseq, "assignment") { + t.Errorf("%s: tags %v, want assignment", src, definedTags(iseq)) + } + } + for _, src := range []string{ + "X = 5; defined?((defined?(X) && X) || X = 1)", + "defined?(1 || 2)", + "defined?(1 && 2)", + "y = 3; defined?(y || 4)", + } { + if iseq := compileSrc(t, src); hasDefinedTag(iseq, "assignment") { + t.Errorf("%s: tagged assignment; MRI says expression", src) + } + } +} + +// Every way the recogniser must say no. The parser builds none of these shapes +// for a `||=`, so they are synthesized; each one leaves the expression an +// ordinary `||`, which defined? tags "expression". +func TestConstOrAssignRejectsOtherShapes(t *testing.T) { + read := &ast.ConstRef{Name: "A"} + probe := func(arg ast.Node) *ast.Call { return &ast.Call{Name: "defined?", Args: []ast.Node{arg}} } + guard := func(l, r ast.Node) *ast.BinaryExpr { return &ast.BinaryExpr{Op: "&&", Left: l, Right: r} } + one := &ast.IntLit{Value: 1} + cases := []struct { + name string + expr *ast.BinaryExpr + }{ + {"not ||", &ast.BinaryExpr{Op: "&&", Left: guard(probe(read), read), Right: &ast.ConstAssign{Name: "A", Value: one}}}, + {"left is not a binary expression", &ast.BinaryExpr{Op: "||", Left: read, Right: &ast.ConstAssign{Name: "A", Value: one}}}, + {"left is not &&", &ast.BinaryExpr{Op: "||", Left: &ast.BinaryExpr{Op: "+", Left: probe(read), Right: read}, + Right: &ast.ConstAssign{Name: "A", Value: one}}}, + {"guard's left is not a call", &ast.BinaryExpr{Op: "||", Left: guard(read, read), Right: &ast.ConstAssign{Name: "A", Value: one}}}, + {"probe is not defined?", &ast.BinaryExpr{Op: "||", Left: guard(&ast.Call{Name: "frozen?", Args: []ast.Node{read}}, read), + Right: &ast.ConstAssign{Name: "A", Value: one}}}, + {"probe has a receiver", &ast.BinaryExpr{Op: "||", Left: guard(&ast.Call{Recv: read, Name: "defined?", Args: []ast.Node{read}}, read), + Right: &ast.ConstAssign{Name: "A", Value: one}}}, + {"probe takes no argument", &ast.BinaryExpr{Op: "||", Left: guard(&ast.Call{Name: "defined?"}, read), + Right: &ast.ConstAssign{Name: "A", Value: one}}}, + {"probe reads a different node", &ast.BinaryExpr{Op: "||", Left: guard(probe(&ast.ConstRef{Name: "A"}), read), + Right: &ast.ConstAssign{Name: "A", Value: one}}}, + {"right is not an assignment", &ast.BinaryExpr{Op: "||", Left: guard(probe(read), read), Right: one}}, + {"assigns a different constant", &ast.BinaryExpr{Op: "||", Left: guard(probe(read), read), + Right: &ast.ConstAssign{Name: "B", Value: one}}}, + {"read is not a plain constant", func() *ast.BinaryExpr { + sc := &ast.ScopedConst{Recv: &ast.ConstRef{Name: "Object"}, Name: "A"} + return &ast.BinaryExpr{Op: "||", Left: guard(probe(sc), sc), Right: &ast.ConstAssign{Name: "A", Value: one}} + }()}, + {"scoped assignment targets another node", func() *ast.BinaryExpr { + sc := &ast.ScopedConst{Recv: &ast.ConstRef{Name: "Object"}, Name: "A"} + return &ast.BinaryExpr{Op: "||", Left: guard(probe(sc), sc), + Right: &ast.ScopedConstAssign{Target: &ast.ScopedConst{Recv: &ast.ConstRef{Name: "Object"}, Name: "A"}, Value: one}} + }()}, + } + for _, tc := range cases { + if constOrAssign(tc.expr) { + t.Errorf("%s: recognised as a constant ||=", tc.name) + } + } + // …and the two shapes it must say yes to. + sc := &ast.ScopedConst{Recv: &ast.ConstRef{Name: "Object"}, Name: "A"} + for _, tc := range []struct { + name string + expr *ast.BinaryExpr + }{ + {"A ||= 1", &ast.BinaryExpr{Op: "||", Left: guard(probe(read), read), Right: &ast.ConstAssign{Name: "A", Value: one}}}, + {"Object::A ||= 1", &ast.BinaryExpr{Op: "||", Left: guard(probe(sc), sc), Right: &ast.ScopedConstAssign{Target: sc, Value: one}}}, + } { + if !constOrAssign(tc.expr) { + t.Errorf("%s: not recognised", tc.name) + } + } +}