From a265877981a32aa0bd29d3e747928597e5029bcb Mon Sep 17 00:00:00 2001 From: tannevaled Date: Sun, 1 Nov 2026 09:48:32 +0100 Subject: [PATCH] compiler: `_1` beside a written parameter list is a SyntaxError MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The last of the five defects in #799 that this repository can fix — it fails at 3.4.1 and at 4.0.9 alike — and the one that cost nothing at all before it was looked for: ```console $ rbgo -e 'proc { |x| _1 }' # on main $ # no error, no warning, nothing ``` MRI rejects that at parse time. A block cannot have both a written parameter list and a numbered parameter, because the grammar reserves `_1`..`_9` for the numbered-parameter mechanism, so the two would be the same slot under two names. `language/numbered_parameters_spec.rb:49` pins eight shapes in one example, and the pairing is the interesting part: ```ruby -> { eval("-> () { _1 }") }.should.raise(SyntaxError, /ordinary parameter is defined/) -> { eval("-> (x) { _1 }") }.should.raise(SyntaxError, /ordinary parameter is defined/) -> { eval("proc { || _1 }") }.should.raise(SyntaxError, /ordinary parameter is defined/) -> { eval("proc { |x| _1 }") }.should.raise(SyntaxError, /ordinary parameter is defined/) … ``` Each kind of block appears twice, once with an EMPTY written list and once with a parameter in it: writing `||` or `()` at all is what declares the list, not what it contains. A check that looked for a parameter would have passed four of the eight. ## Where it goes, and why there `blockParamsAreImplicit` already tells a written list from one the parser synthesised out of `it` / `_1`..`_9` in the body — the trace being that a written list pads `Defaults` to one entry per parameter and a synthesised one leaves it empty. The builder now records that answer as `writtenParams`, set BEFORE the body is compiled, because the body is where `_1` is met. The refusal itself sits in `compileCall`, beside the `__LINE__` and `block_given?` intrinsics and shaped like them — receiverless, blockless, argumentless, bare name. Reaching that point means the name resolved to no local, so it was about to become a receiverless send; that is exactly the position in which MRI has already decided the program is ill-formed. Two scopes are deliberately NOT covered, because MRI's answer there is a different one and no spec in the gate asks for it: - a `for` body (`forScope`), since `for` opens no Ruby scope; - a method body and the top level, where `_1` stays a runtime NameError. ## Measured, with the control `language/numbered_parameters_spec.rb`, pinned corpus 87b1631, shim at 3.4.1: | binary | result | |---|---| | `main` | `pass=16 fail=1` | | this branch | **`pass=17 fail=0`** | The file is clean. The eight shapes, probed directly: | | `main` | this branch | |---|---|---| | `-> () { _1 }` … `lambda { \|x\| _1 }` (6 forms) | **no error at all** | SyntaxError | | `['a'].map { \|\| _1 }`, `['a'].map { \|x\| _1 }` | NameError, at CALL time | SyntaxError | | `proc { _1 }.call(7)` | `7` | `7` | | `proc { _1 + _2 }.call(3, 4)` | `7` | `7` | | `proc { \|x\| _10 }` | fine | fine | The two rows that said NameError are worth keeping in view: on `main` only the shapes that were actually CALLED failed, and the four that were merely defined raised nothing — so the defect was invisible to anything but a spec that evaluates the source. ## ⛔ Not one row of the new test was run against a reference interpreter The Ruby on this machine is 2.6.10 and numbered parameters arrived in 2.7, so the local oracle cannot answer any of these. The authority is the spec file, which pins all eight verbatim, and MRI's rule that the grammar reserves the names. The test says so in its own comment rather than borrowing the "verified against MRI 4.0.5" line its neighbour in the same file has earned. Tests: `internal/compiler/numbered_param_ordinary_test.go` — the eight shapes, three more written-list kinds (`_9`, a block parameter, a keyword), thirteen controls, and a second test asserting the two refusals stay DISTINGUISHABLE: binding `_1` is "reserved for numbered parameters" and reading it beside an ordinary parameter is "ordinary parameter is defined". They are different sentences in MRI and the specs match them separately, so collapsing them into one message would satisfy neither. Refs #799 Co-authored-by: Claude Opus 5 --- internal/compiler/compiler.go | 28 +++++ .../compiler/numbered_param_ordinary_test.go | 105 ++++++++++++++++++ 2 files changed, 133 insertions(+) create mode 100644 internal/compiler/numbered_param_ordinary_test.go diff --git a/internal/compiler/compiler.go b/internal/compiler/compiler.go index 34241cb1..918758fb 100644 --- a/internal/compiler/compiler.go +++ b/internal/compiler/compiler.go @@ -48,6 +48,11 @@ type builder struct { children []*bytecode.ISeq parent *builder isBlock bool + // writtenParams records that this block's parameter list was WRITTEN — `|x|`, + // `|&b|`, `|k: 1|`, or even an empty `||` or `()` — rather than synthesised by + // the parser from `it` / `_1`..`_9` in the body. It is what makes `_1` inside + // the block a SyntaxError instead of a method call; see compileCall. + writtenParams 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. @@ -1337,6 +1342,27 @@ func (c *Compiler) compileCall(v *ast.Call) { b.emit(bytecode.OpPushConst, b.addConst(object.IntValue(int64(b.curLine))), 0) return } + // ⛔ `_1`..`_9` inside a block that WROTE its parameter list is a SyntaxError, + // not a method call. MRI's lexer reserves the nine names for the + // numbered-parameter mechanism, so a block cannot have both: `proc { |x| _1 }` + // is rejected at parse time with "ordinary parameter is defined", and so are + // the empty-list forms `proc { || _1 }` and `-> () { _1 }` — writing `||` at + // all is what declares the list, not what it contains. + // language/numbered_parameters_spec.rb:49 pins all eight shapes. + // + // Reaching here means the name resolved to no local, so it was about to be + // compiled as a receiverless send and fail at RUNTIME with NameError — which + // is what rbgo did, and only for the shapes that were actually called: + // `proc { |x| _1 }` on its own raised nothing at all. + // + // A `for` body is excluded: forScope is the each-block `for` lowers into, and + // `for` opens no Ruby scope, so the numbered-parameter rules do not reach it. + // Confined to the bare, argumentless, receiverless, blockless name, matching + // the shape of the __LINE__ and block_given? intrinsics above. + if v.Recv == nil && v.Block == nil && len(callArgs) == 0 && + isNumberedParamName(v.Name) && b.isBlock && b.writtenParams && !b.forScope { + c.fail("ordinary parameter is defined") + } // block_given? is a frame intrinsic, not a real dispatch. if v.Recv == nil && v.Block == nil && v.Name == "block_given?" && len(callArgs) == 0 { b.emit(bytecode.OpBlockGiven, 0, 0) @@ -2116,6 +2142,8 @@ func (c *Compiler) compileBlock(blk *ast.Block) int { b.emit(bytecode.OpPop, 0, 0) b.patch(skip, b.here()) } + // Recorded BEFORE the body is compiled, because the body is where `_1` is met. + b.writtenParams = !blockParamsAreImplicit(blk) c.ctxs = append(c.ctxs, &loopCtx{kind: ctxBlock}) c.compileBody(blk.Body) c.ctxs = c.ctxs[:len(c.ctxs)-1] diff --git a/internal/compiler/numbered_param_ordinary_test.go b/internal/compiler/numbered_param_ordinary_test.go new file mode 100644 index 00000000..df139c5f --- /dev/null +++ b/internal/compiler/numbered_param_ordinary_test.go @@ -0,0 +1,105 @@ +// Copyright (c) the go-embedded-ruby/ruby authors +// +// SPDX-License-Identifier: BSD-3-Clause + +package compiler + +import ( + "strings" + "testing" +) + +// A block cannot have both a WRITTEN parameter list and a numbered parameter: +// MRI's lexer reserves `_1`..`_9` for the numbered-parameter mechanism, so +// `proc { |x| _1 }` is rejected at parse time with "ordinary parameter is +// defined". Writing `||` at all is what declares the list, which is why the +// empty forms are rejected too. +// +// ⛔ Unlike its neighbour TestNumberedParameterNameIsReserved, NOT ONE row here +// was run against a reference interpreter: the Ruby on this machine is 2.6.10 +// and numbered parameters arrived in 2.7, so the local oracle cannot answer any +// of these. The authority is language/numbered_parameters_spec.rb:49, which pins +// all eight shapes below verbatim, and MRI's own rule that the nine names are +// reserved by the grammar. Said plainly so nobody reads a measurement here that +// was never taken. +// +// Before this check the names resolved to no local and were compiled as +// receiverless sends, so the shapes that were CALLED failed at runtime with +// NameError and the ones that were only defined — `proc { |x| _1 }` on its own — +// raised nothing at all. +func TestNumberedParameterWithOrdinaryParameters(t *testing.T) { + const want = "ordinary parameter is defined" + refused := []string{ + // The eight shapes the spec lists, in its order: a written list that is + // empty and one that is not, across ->, proc, lambda and a method block. + "-> () { _1 }", + "-> (x) { _1 }", + "proc { || _1 }", + "proc { |x| _1 }", + "lambda { || _1 }", + "lambda { |x| _1 }", + "['a'].map { || _1 }", + "['a'].map { |x| _1 }", + // The same rule reaches the other nine names and the other kinds of + // written parameter — a block parameter and a keyword are still a written + // list, even though neither is positional. + "proc { |x| _9 }", + "proc { |&blk| _1 }", + "proc { |k: 1| _1 }", + } + for _, src := range refused { + _, err := compileString(src) + if err == nil || !strings.Contains(err.Error(), want) { + t.Errorf("%q: err = %v, want one containing %q", src, err, want) + } + } + + // The controls. Each one would be broken by a check that was too eager, and + // the first two are the ones that matter most: the numbered parameter's whole + // reason for existing is the block that writes NO list. + allowed := []string{ + "proc { _1 }", // synthesised by the parser — the normal case + "proc { _1 + _2 }", // and more than one of them + "proc { |x| x }", // an ordinary written parameter, no _N in sight + "proc { |x| _0 }", // _0 is an ordinary identifier + "proc { |x| _10 }", // so is _10 + "proc { |x| _1a }", // and so is anything longer + "proc { |x| @_1 }", // an ivar is not a numbered parameter + "proc { |x| self._1 }", // an explicit receiver is an ordinary call + "proc { |x| _1(2) }", // so is a call with arguments + "proc { |x| proc { _1 } }", // the INNER block wrote no list; _1 is its own + "def m; _1; end", // not a block at all: left as a runtime NameError + "_1", // nor at the top level + "for x in [1]; _1; end", // `for` opens no Ruby scope, so the rule misses it + } + for _, src := range allowed { + if _, err := compileString(src); err != nil && strings.Contains(err.Error(), want) { + t.Errorf("%q: refused with %q, must be allowed", src, err) + } + } +} + +// The control for the control: the two refusals must stay DISTINGUISHABLE. +// Binding `_1` is "reserved for numbered parameters" (storeLocal) and reading it +// beside an ordinary parameter is "ordinary parameter is defined" (compileCall). +// They are different sentences in MRI and the specs match on them separately — +// `/_1 is reserved for numbered parameter/` and `/ordinary parameter is defined/` +// — so a fix that collapsed them into one message would satisfy neither spec. +func TestNumberedParameterRefusalsStayDistinct(t *testing.T) { + for _, tc := range []struct{ src, want, notWant string }{ + {"proc { _1 = 0 }", "reserved for numbered parameters", "ordinary parameter is defined"}, + {"proc { |x| _1 }", "ordinary parameter is defined", "reserved for numbered parameters"}, + } { + _, err := compileString(tc.src) + if err == nil { + t.Errorf("%q: compiled, want a refusal", tc.src) + continue + } + if !strings.Contains(err.Error(), tc.want) { + t.Errorf("%q: err = %v, want one containing %q", tc.src, err, tc.want) + } + if strings.Contains(err.Error(), tc.notWant) { + t.Errorf("%q: err = %v, must NOT contain %q", tc.src, err, tc.notWant) + } + } +}