From 570b10542fdea08923f207fe6c484341e46738b4 Mon Sep 17 00:00:00 2001 From: tannevaled Date: Sun, 18 Oct 2026 09:11:42 +0200 Subject: [PATCH 1/2] FFT, Image: every method declares its arity, so a missing argument is MRI's error Closes #820. All 17 of FFT's methods and 14 of Image's indexed args[0..3] with no count check, so a missing argument surfaced as FFT.fft() ArgumentError: fft: runtime error: index out of range [0] with length 0 -- a Go panic recovered and relabelled, where MRI says "wrong number of arguments (given 0, expected 1)". Its class was accidental too: ArgumentError only because the recovery path labels every panic that way. NDArray was already clean because it uses ndArgc, which both now use. The counts go in the REGISTRATION SITE's signature rather than inside each closure: `def(name, min, max, fn)` for FFT and `d`/`s` for Image's instance and module methods. A check inside a body can be forgotten by the next method added; a parameter cannot. The three wrappers that forward to d (encode, unary, scalar) pass their own fixed arities, so the sixteen unary filters and the two scalar ones are covered by one line each rather than eighteen. Each arity comes from reading the body, not from a guess, and the ranges are the optional arguments a caller actually has: fftfreq/rfftfreq/psd 1..2 for numpy's sample spacing d, spectrogram 4..5, Image#set 5..6 for the alpha it defaults to 255. The test is a CENSUS, as #820 suggested, so a method added later is covered without being registered anywhere -- and it asserts the number WALKED (17, 3, 34) as well as the empty result, because a census that reads nothing otherwise looks exactly like a clean module. #save is held out of it, since it writes a file, and checked by name instead. Nineteen message rows pin MRI's wording for each shape -- fixed 1/2/3/4, both range forms, and the zero-argument form a caller trips by passing something. The count is only half of it: a reader rescuing ArgumentError and printing e.message has to be told what they passed wrongly, and "index out of range" does not tell them. A control for wrapping every method: eleven answers that were right before and have to stay so, including both optional arguments and #to_s, which is aliased from #inspect's native function and so inherits whatever wrapper #inspect got. Four negative controls, each checked to have really changed the file: dropping the check in FFT's def, dropping it in Image's d, taking fftfreq's optional spacing away, and taking Image#set's optional alpha away all turn the suite red. Co-Authored-By: Claude Opus 5 --- internal/vm/fft.go | 42 ++++++---- internal/vm/image.go | 113 +++++++++++++++----------- internal/vm/wave71_arity_test.go | 134 +++++++++++++++++++++++++++++++ 3 files changed, 226 insertions(+), 63 deletions(-) create mode 100644 internal/vm/wave71_arity_test.go diff --git a/internal/vm/fft.go b/internal/vm/fft.go index 0aff8417..daa47a20 100644 --- a/internal/vm/fft.go +++ b/internal/vm/fft.go @@ -129,54 +129,66 @@ func checkShape(n int, shape []int) { } // registerFFT installs the FFT module and its transform functions. +// +// Every method declares its arity AT the registration site, and def checks it +// before the body runs. That is deliberate rather than a check inside each +// closure: the bodies index args[0..3] directly, so a missing argument used to +// surface as "runtime error: index out of range" -- a recovered Go panic +// relabelled ArgumentError -- where MRI says "wrong number of arguments" +// (#820). Putting the counts in def's signature means a method added later +// cannot be registered without them. func (vm *VM) registerFFT() { mod := newClass("FFT", nil) mod.isModule = true vm.consts["FFT"] = mod - def := func(name string, fn NativeFn) { - mod.smethods[name] = &Method{name: name, owner: mod, native: fn} + def := func(name string, min, max int, fn NativeFn) { + mod.smethods[name] = &Method{name: name, owner: mod, native: func(vm *VM, self object.Value, args []object.Value, blk *Proc) object.Value { + ndArgc(args, min, max) + return fn(vm, self, args, blk) + }} } - def("fft", func(_ *VM, _ object.Value, args []object.Value, _ *Proc) object.Value { + def("fft", 1, 1, func(_ *VM, _ object.Value, args []object.Value, _ *Proc) object.Value { return complexArray(gofft.FFT(complexSlice(args[0]))) }) - def("ifft", func(_ *VM, _ object.Value, args []object.Value, _ *Proc) object.Value { + def("ifft", 1, 1, func(_ *VM, _ object.Value, args []object.Value, _ *Proc) object.Value { return complexArray(gofft.IFFT(complexSlice(args[0]))) }) - def("rfft", func(_ *VM, _ object.Value, args []object.Value, _ *Proc) object.Value { + def("rfft", 1, 1, func(_ *VM, _ object.Value, args []object.Value, _ *Proc) object.Value { return complexArray(gofft.RFFT(floatSlice(args[0]))) }) - def("irfft", func(_ *VM, _ object.Value, args []object.Value, _ *Proc) object.Value { + def("irfft", 2, 2, func(_ *VM, _ object.Value, args []object.Value, _ *Proc) object.Value { n := int(intArg(args[1])) return floatArray(gofft.IRFFT(complexSlice(args[0]), n)) }) - def("fftfreq", func(_ *VM, _ object.Value, args []object.Value, _ *Proc) object.Value { + // The bin-frequency helpers take numpy's optional sample spacing d. + def("fftfreq", 1, 2, func(_ *VM, _ object.Value, args []object.Value, _ *Proc) object.Value { return floatArray(gofft.FFTFreq(int(intArg(args[0])), sampleSpacing(args, 1))) }) - def("rfftfreq", func(_ *VM, _ object.Value, args []object.Value, _ *Proc) object.Value { + def("rfftfreq", 1, 2, func(_ *VM, _ object.Value, args []object.Value, _ *Proc) object.Value { return floatArray(gofft.RFFTFreq(int(intArg(args[0])), sampleSpacing(args, 1))) }) // N-dimensional transforms over flat row-major data plus an explicit shape. - def("fftn", func(_ *VM, _ object.Value, args []object.Value, _ *Proc) object.Value { + def("fftn", 2, 2, func(_ *VM, _ object.Value, args []object.Value, _ *Proc) object.Value { data, shape := complexSlice(args[0]), intSlice(args[1]) checkShape(len(data), shape) return complexArray(gofft.FFTN(data, shape)) }) - def("ifftn", func(_ *VM, _ object.Value, args []object.Value, _ *Proc) object.Value { + def("ifftn", 2, 2, func(_ *VM, _ object.Value, args []object.Value, _ *Proc) object.Value { data, shape := complexSlice(args[0]), intSlice(args[1]) checkShape(len(data), shape) return complexArray(gofft.IFFTN(data, shape)) }) // 2-D transforms over flat row-major data of shape rows×cols. - def("fft2", func(_ *VM, _ object.Value, args []object.Value, _ *Proc) object.Value { + def("fft2", 3, 3, func(_ *VM, _ object.Value, args []object.Value, _ *Proc) object.Value { data := complexSlice(args[0]) rows, cols := int(intArg(args[1])), int(intArg(args[2])) checkShape(len(data), []int{rows, cols}) return complexArray(gofft.FFT2(data, [2]int{rows, cols})) }) - def("ifft2", func(_ *VM, _ object.Value, args []object.Value, _ *Proc) object.Value { + def("ifft2", 3, 3, func(_ *VM, _ object.Value, args []object.Value, _ *Proc) object.Value { data := complexSlice(args[0]) rows, cols := int(intArg(args[1])), int(intArg(args[2])) checkShape(len(data), []int{rows, cols}) @@ -185,7 +197,7 @@ func (vm *VM) registerFFT() { // Window functions (numpy/scipy conventions): length n → []float64. window := func(name string, f func(int) []float64) { - def(name, func(_ *VM, _ object.Value, args []object.Value, _ *Proc) object.Value { + def(name, 1, 1, func(_ *VM, _ object.Value, args []object.Value, _ *Proc) object.Value { return floatArray(f(int(intArg(args[0])))) }) } @@ -197,10 +209,10 @@ func (vm *VM) registerFFT() { // Spectral helpers: one-sided power spectral density and a windowed // spectrogram (an array of per-segment PSD frames). - def("psd", func(_ *VM, _ object.Value, args []object.Value, _ *Proc) object.Value { + def("psd", 1, 2, func(_ *VM, _ object.Value, args []object.Value, _ *Proc) object.Value { return floatArray(gofft.PSD(floatSlice(args[0]), sampleSpacing(args, 1))) }) - def("spectrogram", func(_ *VM, _ object.Value, args []object.Value, _ *Proc) object.Value { + def("spectrogram", 4, 5, func(_ *VM, _ object.Value, args []object.Value, _ *Proc) object.Value { signal := floatSlice(args[0]) segment, overlap := int(intArg(args[1])), int(intArg(args[2])) win := floatSlice(args[3]) diff --git a/internal/vm/image.go b/internal/vm/image.go index 19c44dc6..d1e1bb42 100644 --- a/internal/vm/image.go +++ b/internal/vm/image.go @@ -44,50 +44,67 @@ func (vm *VM) registerImage() { vm.cImage = newClass("Image", vm.cObject) vm.consts["Image"] = vm.cImage - vm.cImage.smethods["new"] = &Method{name: "new", owner: vm.cImage, - native: func(_ *VM, _ object.Value, args []object.Value, _ *Proc) object.Value { - w, h := int(intArg(args[0])), int(intArg(args[1])) - if w <= 0 || h <= 0 { - raise("ArgumentError", "image dimensions must be positive") - } - return &Image{img: image.NewRGBA(image.Rect(0, 0, w, h))} - }} - vm.cImage.smethods["load"] = &Method{name: "load", owner: vm.cImage, - native: func(_ *VM, _ object.Value, args []object.Value, _ *Proc) object.Value { - img, err := goimg.Load(args[0].ToS()) - if err != nil { - raise("ArgumentError", "%s", err.Error()) - } - return &Image{img: img} - }} + // s registers a module method with its arity, for the same reason as d. + s := func(name string, min, max int, fn NativeFn) { + vm.cImage.smethods[name] = &Method{name: name, owner: vm.cImage, + native: func(vm *VM, self object.Value, args []object.Value, blk *Proc) object.Value { + ndArgc(args, min, max) + return fn(vm, self, args, blk) + }} + } + + s("new", 2, 2, func(_ *VM, _ object.Value, args []object.Value, _ *Proc) object.Value { + w, h := int(intArg(args[0])), int(intArg(args[1])) + if w <= 0 || h <= 0 { + raise("ArgumentError", "image dimensions must be positive") + } + return &Image{img: image.NewRGBA(image.Rect(0, 0, w, h))} + }) + s("load", 1, 1, func(_ *VM, _ object.Value, args []object.Value, _ *Proc) object.Value { + img, err := goimg.Load(args[0].ToS()) + if err != nil { + raise("ArgumentError", "%s", err.Error()) + } + return &Image{img: img} + }) // Image.decode(bytes) decodes an in-memory PNG/JPEG String — no filesystem, // so it works in a browser (wasm) where the program is handed image bytes. - vm.cImage.smethods["decode"] = &Method{name: "decode", owner: vm.cImage, - native: func(_ *VM, _ object.Value, args []object.Value, _ *Proc) object.Value { - s, ok := args[0].(*object.String) - if !ok { - raise("TypeError", "Image.decode expects a String of image bytes") - } - img, err := goimg.Decode(bytes.NewReader(s.Bytes())) - if err != nil { - raise("ArgumentError", "%s", err.Error()) - } - return &Image{img: img} - }} - - d := func(name string, fn NativeFn) { vm.cImage.define(name, fn) } - - d("width", func(_ *VM, v object.Value, _ []object.Value, _ *Proc) object.Value { + s("decode", 1, 1, func(_ *VM, _ object.Value, args []object.Value, _ *Proc) object.Value { + s, ok := args[0].(*object.String) + if !ok { + raise("TypeError", "Image.decode expects a String of image bytes") + } + img, err := goimg.Decode(bytes.NewReader(s.Bytes())) + if err != nil { + raise("ArgumentError", "%s", err.Error()) + } + return &Image{img: img} + }) + + // d registers an instance method WITH its arity. The counts live in the + // signature so a method added later cannot be registered without them: + // the bodies index args[0..3] directly, and a missing argument used to + // surface as "runtime error: index out of range" -- a recovered Go panic + // relabelled ArgumentError -- where MRI says "wrong number of + // arguments" (#820). + d := func(name string, min, max int, fn NativeFn) { + vm.cImage.define(name, func(vm *VM, self object.Value, args []object.Value, blk *Proc) object.Value { + ndArgc(args, min, max) + return fn(vm, self, args, blk) + }) + } + + d("width", 0, 0, func(_ *VM, v object.Value, _ []object.Value, _ *Proc) object.Value { return object.IntValue(int64(imgOf(v).Bounds().Dx())) }) - d("height", func(_ *VM, v object.Value, _ []object.Value, _ *Proc) object.Value { + d("height", 0, 0, func(_ *VM, v object.Value, _ []object.Value, _ *Proc) object.Value { return object.IntValue(int64(imgOf(v).Bounds().Dy())) }) - d("get", func(_ *VM, v object.Value, args []object.Value, _ *Proc) object.Value { + d("get", 2, 2, func(_ *VM, v object.Value, args []object.Value, _ *Proc) object.Value { c := imgOf(v).RGBAAt(int(intArg(args[0])), int(intArg(args[1]))) return object.NewArray(object.IntValue(int64(c.R)), object.IntValue(int64(c.G)), object.IntValue(int64(c.B)), object.IntValue(int64(c.A))) }) - d("set", func(_ *VM, v object.Value, args []object.Value, _ *Proc) object.Value { + d("set", 5, 6, func(_ *VM, v object.Value, args []object.Value, _ *Proc) object.Value { a := uint8(255) if len(args) > 5 { a = uint8(intArg(args[5])) @@ -96,7 +113,7 @@ func (vm *VM) registerImage() { color.RGBA{uint8(intArg(args[2])), uint8(intArg(args[3])), uint8(intArg(args[4])), a}) return object.NilV }) - d("save", func(_ *VM, v object.Value, args []object.Value, _ *Proc) object.Value { + d("save", 1, 1, func(_ *VM, v object.Value, args []object.Value, _ *Proc) object.Value { if err := goimg.Save(args[0].ToS(), imgOf(v)); err != nil { raise("ArgumentError", "%s", err.Error()) } @@ -105,7 +122,7 @@ func (vm *VM) registerImage() { // to_png / to_jpeg encode to an in-memory byte String (browser-friendly: hand // the bytes to JS for a canvas/Blob). Encoding to a buffer cannot fail. encode := func(name string, format goimg.Format) { - d(name, func(_ *VM, v object.Value, _ []object.Value, _ *Proc) object.Value { + d(name, 0, 0, func(_ *VM, v object.Value, _ []object.Value, _ *Proc) object.Value { var buf bytes.Buffer _ = goimg.Encode(&buf, imgOf(v), format) return object.NewStringBytes(buf.Bytes()) @@ -116,7 +133,7 @@ func (vm *VM) registerImage() { // Operations with no error: image → image. unary := func(name string, f func(image.Image) *image.RGBA) { - d(name, func(_ *VM, v object.Value, _ []object.Value, _ *Proc) object.Value { + d(name, 0, 0, func(_ *VM, v object.Value, _ []object.Value, _ *Proc) object.Value { return &Image{img: f(imgOf(v))} }) } @@ -139,7 +156,7 @@ func (vm *VM) registerImage() { // Operations with a scalar parameter, no error. scalar := func(name string, f func(image.Image, float64) *image.RGBA) { - d(name, func(_ *VM, v object.Value, args []object.Value, _ *Proc) object.Value { + d(name, 1, 1, func(_ *VM, v object.Value, args []object.Value, _ *Proc) object.Value { x, _ := toFloat(args[0]) return &Image{img: f(imgOf(v), x)} }) @@ -148,38 +165,38 @@ func (vm *VM) registerImage() { scalar("adjust_contrast", goimg.AdjustContrast) // Operations that can fail. - d("gaussian_blur", func(_ *VM, v object.Value, args []object.Value, _ *Proc) object.Value { + d("gaussian_blur", 1, 1, func(_ *VM, v object.Value, args []object.Value, _ *Proc) object.Value { s, _ := toFloat(args[0]) return mustImg(goimg.GaussianBlur(imgOf(v), s)) }) - d("box_blur", func(_ *VM, v object.Value, args []object.Value, _ *Proc) object.Value { + d("box_blur", 1, 1, func(_ *VM, v object.Value, args []object.Value, _ *Proc) object.Value { return mustImg(goimg.BoxBlur(imgOf(v), int(intArg(args[0])))) }) - d("median", func(_ *VM, v object.Value, args []object.Value, _ *Proc) object.Value { + d("median", 1, 1, func(_ *VM, v object.Value, args []object.Value, _ *Proc) object.Value { return mustImg(goimg.Median(imgOf(v), int(intArg(args[0])))) }) - d("erode", func(_ *VM, v object.Value, args []object.Value, _ *Proc) object.Value { + d("erode", 1, 1, func(_ *VM, v object.Value, args []object.Value, _ *Proc) object.Value { return mustImg(goimg.Erode(imgOf(v), int(intArg(args[0])))) }) - d("dilate", func(_ *VM, v object.Value, args []object.Value, _ *Proc) object.Value { + d("dilate", 1, 1, func(_ *VM, v object.Value, args []object.Value, _ *Proc) object.Value { return mustImg(goimg.Dilate(imgOf(v), int(intArg(args[0])))) }) - d("canny", func(_ *VM, v object.Value, args []object.Value, _ *Proc) object.Value { + d("canny", 3, 3, func(_ *VM, v object.Value, args []object.Value, _ *Proc) object.Value { sigma, _ := toFloat(args[0]) low, _ := toFloat(args[1]) high, _ := toFloat(args[2]) return mustImg(goimg.Canny(imgOf(v), sigma, low, high)) }) - d("resize", func(_ *VM, v object.Value, args []object.Value, _ *Proc) object.Value { + d("resize", 2, 2, func(_ *VM, v object.Value, args []object.Value, _ *Proc) object.Value { return mustImg(goimg.Resize(imgOf(v), int(intArg(args[0])), int(intArg(args[1])), goimg.Bilinear)) }) - d("crop", func(_ *VM, v object.Value, args []object.Value, _ *Proc) object.Value { + d("crop", 4, 4, func(_ *VM, v object.Value, args []object.Value, _ *Proc) object.Value { x, y := int(intArg(args[0])), int(intArg(args[1])) w, h := int(intArg(args[2])), int(intArg(args[3])) return mustImg(goimg.Crop(imgOf(v), image.Rect(x, y, x+w, y+h))) }) - d("inspect", func(_ *VM, v object.Value, _ []object.Value, _ *Proc) object.Value { + d("inspect", 0, 0, func(_ *VM, v object.Value, _ []object.Value, _ *Proc) object.Value { return object.NewString(v.(*Image).Inspect()) }) vm.cImage.define("to_s", vm.cImage.methods["inspect"].native) diff --git a/internal/vm/wave71_arity_test.go b/internal/vm/wave71_arity_test.go new file mode 100644 index 00000000..23b7e074 --- /dev/null +++ b/internal/vm/wave71_arity_test.go @@ -0,0 +1,134 @@ +// Copyright (c) the go-embedded-ruby/ruby authors +// +// SPDX-License-Identifier: BSD-3-Clause + +package vm + +import ( + "strings" + "testing" +) + +// TestFFTAndImageCheckTheirArity closes #820. +// +// Every method of both modules indexed args[0..3] directly with no count +// check, so a missing argument surfaced as +// +// FFT.fft() ArgumentError: fft: runtime error: index out of range [0] with length 0 +// +// a Go panic recovered and relabelled, where MRI says "wrong number of +// arguments (given 0, expected 1)". A census over rbgo found 31: all 17 of +// FFT's and 14 of Image's. NDArray was already clean because it uses ndArgc, +// which is what both now use. +// +// This is a CENSUS rather than a list of names, so a method added later is +// covered without being registered anywhere -- and it asserts the number +// WALKED as well as the empty result, so a census that reads nothing fails +// instead of passing. +// +// #save is the one method held out, because it writes a file; it is checked by +// name in TestFFTAndImageArityMessages below. +func TestFFTAndImageCheckTheirArity(t *testing.T) { + src := `def census(recv, names, skip) + bad = [] + n = 0 + names.sort.each do |m| + next if skip.include?(m.to_s) + n += 1 + begin + recv.send(m) + rescue => e + bad << m if e.message.include?("runtime error") + end + end + [n, bad] +end +a, abad = census(FFT, FFT.methods(false), []) +b, bbad = census(Image, Image.methods(false), []) +c, cbad = census(Image.new(2, 2), Image.instance_methods(false), ["save"]) +puts [a, b, c].join(" ") +puts (abad + bbad + cbad).join(" ")` + out := strings.SplitN(strings.TrimSpace(eval(t, src)), "\n", 2) + if len(out) == 0 || out[0] != "17 3 34" { + t.Fatalf("walked %q, want \"17 3 34\" (FFT module, Image module, Image instance) -- "+ + "a census that does not walk proves nothing", out[0]) + } + if len(out) > 1 && strings.TrimSpace(out[1]) != "" { + t.Errorf("these methods still leak a Go runtime fault when called with no arguments: %s", out[1]) + } +} + +// TestFFTAndImageArityMessages pins MRI's wording for one method of each shape, +// because the count is only half of it: a reader rescuing ArgumentError and +// printing e.message has to be told what it passed wrongly, and "runtime error: +// index out of range" does not. The range forms (min..max) matter too -- they +// are what an optional argument looks like to a caller. +func TestFFTAndImageArityMessages(t *testing.T) { + for _, tc := range []struct{ expr, want string }{ + // FFT: fixed one, two and three arguments, then both range forms. + {"FFT.fft", "wrong number of arguments (given 0, expected 1)"}, + {"FFT.irfft", "wrong number of arguments (given 0, expected 2)"}, + {"FFT.fft2", "wrong number of arguments (given 0, expected 3)"}, + {"FFT.fftfreq", "wrong number of arguments (given 0, expected 1..2)"}, + {"FFT.spectrogram", "wrong number of arguments (given 0, expected 4..5)"}, + {"FFT.hann", "wrong number of arguments (given 0, expected 1)"}, + {"FFT.fft([1.0], 2)", "wrong number of arguments (given 2, expected 1)"}, + // Image, module side. + {"Image.new", "wrong number of arguments (given 0, expected 2)"}, + {"Image.load", "wrong number of arguments (given 0, expected 1)"}, + {"Image.decode", "wrong number of arguments (given 0, expected 1)"}, + // Image, instance side, including the no-argument and range forms. + {"Image.new(2, 2).get", "wrong number of arguments (given 0, expected 2)"}, + {"Image.new(2, 2).set", "wrong number of arguments (given 0, expected 5..6)"}, + {"Image.new(2, 2).canny", "wrong number of arguments (given 0, expected 3)"}, + {"Image.new(2, 2).crop", "wrong number of arguments (given 0, expected 4)"}, + {"Image.new(2, 2).save", "wrong number of arguments (given 0, expected 1)"}, + {"Image.new(2, 2).width(1)", "wrong number of arguments (given 1, expected 0)"}, + {"Image.new(2, 2).grayscale(1)", "wrong number of arguments (given 1, expected 0)"}, + {"Image.new(2, 2).to_png(1)", "wrong number of arguments (given 1, expected 0)"}, + {"Image.new(2, 2).adjust_brightness", "wrong number of arguments (given 0, expected 1)"}, + } { + t.Run(tc.expr, func(t *testing.T) { + src := "begin\n " + tc.expr + "\n puts \"NO ERROR\"\nrescue ArgumentError => e\n puts e.message\nend" + if got := strings.TrimSpace(eval(t, src)); got != tc.want { + t.Errorf("%s raised %q, want %q", tc.expr, got, tc.want) + } + }) + } +} + +// TestFFTAndImageStillWork is the control for wrapping every method: the checks +// must not have changed any answer. Each row is a call that was correct before +// and has to stay so, including the optional arguments the new ranges allow +// (#set's alpha, #fftfreq's spacing) and #to_s, which is ALIASED from #inspect's +// native function and so inherits whatever wrapper #inspect got. +func TestFFTAndImageStillWork(t *testing.T) { + src := `p FFT.fft([1.0, 2.0, 3.0, 4.0]).map { |c| c.real.round(6) } +p FFT.fftfreq(4).map { |f| f.round(4) } +p FFT.fftfreq(4, 0.5).map { |f| f.round(4) } +p FFT.hann(3).map { |f| f.round(6) } +p FFT.irfft(FFT.rfft([1.0, 2.0, 3.0, 4.0]), 4).map { |f| f.round(6) } +i = Image.new(4, 4) +i.set(0, 0, 255, 0, 0) +p i.get(0, 0) +i.set(1, 1, 1, 2, 3, 128) +p i.get(1, 1) +p [i.width, i.height, i.grayscale.width, i.resize(2, 2).width, i.crop(0, 0, 2, 2).width] +p [i.adjust_brightness(1.0).width, i.gaussian_blur(0.5).width, i.canny(1.0, 0.1, 0.2).width] +p i.to_png.bytesize > 0 +p i.to_s == i.inspect` + const want = "[10.0, -2.0, -2.0, -2.0]\n" + + "[0.0, 0.25, -0.5, -0.25]\n" + + "[0.0, 0.5, -1.0, -0.5]\n" + + "[0.0, 1.0, 0.0]\n" + + "[1.0, 2.0, 3.0, 4.0]\n" + + "[255, 0, 0, 255]\n" + + "[1, 2, 3, 128]\n" + + "[4, 4, 4, 2, 2]\n" + + "[4, 4, 4]\n" + + "true\n" + + "true\n" + if got := eval(t, src); got != want { + t.Errorf("got\n%s\nwant\n%s", got, want) + } +} From ad20f3069f27dce1256b708eaa077be6d33c6e1f Mon Sep 17 00:00:00 2001 From: tannevaled Date: Sun, 18 Oct 2026 09:30:03 +0200 Subject: [PATCH 2/2] Split the arity tests by build tag: Image is not in the core build CI's rbgo_core lane failed on the tests, not the fix: `NameError: uninitialized constant Image`. Image is the first of the gems the core build leaves out -- image.go and image_test.go have carried `//go:build !rbgo_core` all along -- so a test naming Image fails under that tag for a reason that has nothing to do with arity. The file is split along the line the repository already draws: wave71_arity_test.go FFT only, UNTAGGED -- FFT is in core wave71_arity_image_test.go Image only, !rbgo_core Checked under BOTH tags rather than one, which is the whole point of a split like this: 23 tests run on the default build (my six plus every pre-existing FFT, Image, canvas and images test) and 6 under -tags rbgo_core (the three FFT arity ones and the three pre-existing FFT ones), with Image correctly absent and no "no tests to run" -- an `ok` that ran nothing would look identical otherwise. The FFT half also gained the rows the split made room for: rfftfreq and psd's 1..2 ranges, fftn's fixed 2, and psd called both with and without its optional spacing, so every range form FFT has is exercised in both directions. go vet is clean on the default build, on -tags rbgo_core, and on GOOS=js GOARCH=wasm -tags rbgo_core; the core wasm module builds at 23_199_022 bytes, well under the 29_000_000 bound. Co-Authored-By: Claude Opus 5 --- internal/vm/wave71_arity_image_test.go | 105 +++++++++++++++++++++++ internal/vm/wave71_arity_test.go | 110 +++++++++---------------- 2 files changed, 142 insertions(+), 73 deletions(-) create mode 100644 internal/vm/wave71_arity_image_test.go diff --git a/internal/vm/wave71_arity_image_test.go b/internal/vm/wave71_arity_image_test.go new file mode 100644 index 00000000..958133ae --- /dev/null +++ b/internal/vm/wave71_arity_image_test.go @@ -0,0 +1,105 @@ +// Copyright (c) the go-embedded-ruby/ruby authors +// +// SPDX-License-Identifier: BSD-3-Clause + +//go:build !rbgo_core + +package vm + +import ( + "strings" + "testing" +) + +// The Image half of #820. Tagged !rbgo_core, as image.go and image_test.go +// are: Image is the first of the gems the core build leaves out, so under +// -tags rbgo_core the constant does not exist and a test naming it would fail +// for the wrong reason. The FFT half is untagged and runs under both. + +// TestImageChecksItsArity is the census over both of Image's faces -- the +// module methods and the instance ones. It asserts the number WALKED (3 and +// 34) as well as the empty result, so a census that reads nothing fails +// instead of passing. #save is held out, because it writes a file; it is +// checked by name in TestImageArityMessages. +func TestImageChecksItsArity(t *testing.T) { + src := `def census(recv, names, skip) + bad = [] + n = 0 + names.sort.each do |m| + next if skip.include?(m.to_s) + n += 1 + begin + recv.send(m) + rescue => e + bad << m if e.message.include?("runtime error") + end + end + [n, bad] +end +a, abad = census(Image, Image.methods(false), []) +b, bbad = census(Image.new(2, 2), Image.instance_methods(false), ["save"]) +puts [a, b].join(" ") +puts (abad + bbad).join(" ")` + out := strings.SplitN(strings.TrimSpace(eval(t, src)), "\n", 2) + if out[0] != "3 34" { + t.Fatalf("walked %q, want \"3 34\" (Image module, Image instance) -- "+ + "a census that does not walk proves nothing", out[0]) + } + if len(out) > 1 && strings.TrimSpace(out[1]) != "" { + t.Errorf("these Image methods still leak a Go runtime fault when called with no arguments: %s", out[1]) + } +} + +// TestImageArityMessages pins MRI's wording for each shape Image has: fixed +// counts, the 5..6 range of #set's optional alpha, and the zero-argument form +// a caller trips by passing something. +func TestImageArityMessages(t *testing.T) { + for _, tc := range []struct{ expr, want string }{ + {"Image.new", "wrong number of arguments (given 0, expected 2)"}, + {"Image.load", "wrong number of arguments (given 0, expected 1)"}, + {"Image.decode", "wrong number of arguments (given 0, expected 1)"}, + {"Image.new(2, 2).get", "wrong number of arguments (given 0, expected 2)"}, + {"Image.new(2, 2).set", "wrong number of arguments (given 0, expected 5..6)"}, + {"Image.new(2, 2).canny", "wrong number of arguments (given 0, expected 3)"}, + {"Image.new(2, 2).crop", "wrong number of arguments (given 0, expected 4)"}, + {"Image.new(2, 2).resize", "wrong number of arguments (given 0, expected 2)"}, + {"Image.new(2, 2).save", "wrong number of arguments (given 0, expected 1)"}, + {"Image.new(2, 2).adjust_brightness", "wrong number of arguments (given 0, expected 1)"}, + {"Image.new(2, 2).gaussian_blur", "wrong number of arguments (given 0, expected 1)"}, + {"Image.new(2, 2).width(1)", "wrong number of arguments (given 1, expected 0)"}, + {"Image.new(2, 2).grayscale(1)", "wrong number of arguments (given 1, expected 0)"}, + {"Image.new(2, 2).to_png(1)", "wrong number of arguments (given 1, expected 0)"}, + } { + t.Run(tc.expr, func(t *testing.T) { + src := "begin\n " + tc.expr + "\n puts \"NO ERROR\"\nrescue ArgumentError => e\n puts e.message\nend" + if got := strings.TrimSpace(eval(t, src)); got != tc.want { + t.Errorf("%s raised %q, want %q", tc.expr, got, tc.want) + } + }) + } +} + +// TestImageStillWorks is the control for wrapping every method. #to_s earns its +// own row: it is registered from #inspect's native function, so it silently +// inherits whatever wrapper #inspect got, and only comparing the two says the +// alias still works. +func TestImageStillWorks(t *testing.T) { + src := `i = Image.new(4, 4) +i.set(0, 0, 255, 0, 0) +p i.get(0, 0) +i.set(1, 1, 1, 2, 3, 128) +p i.get(1, 1) +p [i.width, i.height, i.grayscale.width, i.resize(2, 2).width, i.crop(0, 0, 2, 2).width] +p [i.adjust_brightness(1.0).width, i.gaussian_blur(0.5).width, i.canny(1.0, 0.1, 0.2).width] +p i.to_png.bytesize > 0 +p i.to_s == i.inspect` + const want = "[255, 0, 0, 255]\n" + + "[1, 2, 3, 128]\n" + + "[4, 4, 4, 2, 2]\n" + + "[4, 4, 4]\n" + + "true\n" + + "true\n" + if got := eval(t, src); got != want { + t.Errorf("got\n%s\nwant\n%s", got, want) + } +} diff --git a/internal/vm/wave71_arity_test.go b/internal/vm/wave71_arity_test.go index 23b7e074..7e0c4224 100644 --- a/internal/vm/wave71_arity_test.go +++ b/internal/vm/wave71_arity_test.go @@ -9,84 +9,62 @@ import ( "testing" ) -// TestFFTAndImageCheckTheirArity closes #820. +// TestFFTChecksItsArity is half of #820; the Image half is in +// wave71_arity_image_test.go, which is tagged !rbgo_core because Image is the +// first of the gems the core build leaves out. FFT is in core, so this file is +// untagged and runs under both. // -// Every method of both modules indexed args[0..3] directly with no count -// check, so a missing argument surfaced as +// Every FFT method indexed args[0..3] with no count check, so a missing +// argument surfaced as // // FFT.fft() ArgumentError: fft: runtime error: index out of range [0] with length 0 // // a Go panic recovered and relabelled, where MRI says "wrong number of -// arguments (given 0, expected 1)". A census over rbgo found 31: all 17 of -// FFT's and 14 of Image's. NDArray was already clean because it uses ndArgc, -// which is what both now use. +// arguments (given 0, expected 1)". // // This is a CENSUS rather than a list of names, so a method added later is // covered without being registered anywhere -- and it asserts the number // WALKED as well as the empty result, so a census that reads nothing fails // instead of passing. -// -// #save is the one method held out, because it writes a file; it is checked by -// name in TestFFTAndImageArityMessages below. -func TestFFTAndImageCheckTheirArity(t *testing.T) { - src := `def census(recv, names, skip) - bad = [] - n = 0 - names.sort.each do |m| - next if skip.include?(m.to_s) - n += 1 - begin - recv.send(m) - rescue => e - bad << m if e.message.include?("runtime error") - end +func TestFFTChecksItsArity(t *testing.T) { + src := `bad = [] +n = 0 +FFT.methods(false).sort.each do |m| + n += 1 + begin + FFT.send(m) + rescue => e + bad << m if e.message.include?("runtime error") end - [n, bad] end -a, abad = census(FFT, FFT.methods(false), []) -b, bbad = census(Image, Image.methods(false), []) -c, cbad = census(Image.new(2, 2), Image.instance_methods(false), ["save"]) -puts [a, b, c].join(" ") -puts (abad + bbad + cbad).join(" ")` +puts n +puts bad.join(" ")` out := strings.SplitN(strings.TrimSpace(eval(t, src)), "\n", 2) - if len(out) == 0 || out[0] != "17 3 34" { - t.Fatalf("walked %q, want \"17 3 34\" (FFT module, Image module, Image instance) -- "+ - "a census that does not walk proves nothing", out[0]) + if out[0] != "17" { + t.Fatalf("walked %q FFT methods, want 17 -- a census that does not walk proves nothing", out[0]) } if len(out) > 1 && strings.TrimSpace(out[1]) != "" { - t.Errorf("these methods still leak a Go runtime fault when called with no arguments: %s", out[1]) + t.Errorf("these FFT methods still leak a Go runtime fault when called with no arguments: %s", out[1]) } } -// TestFFTAndImageArityMessages pins MRI's wording for one method of each shape, -// because the count is only half of it: a reader rescuing ArgumentError and -// printing e.message has to be told what it passed wrongly, and "runtime error: -// index out of range" does not. The range forms (min..max) matter too -- they -// are what an optional argument looks like to a caller. -func TestFFTAndImageArityMessages(t *testing.T) { +// TestFFTArityMessages pins MRI's wording for each shape FFT has, because the +// count is only half of it: a reader rescuing ArgumentError and printing +// e.message has to be told what they passed wrongly, and "runtime error: index +// out of range" does not. The range forms are what an optional argument looks +// like to a caller -- numpy's sample spacing d, and spectrogram's. +func TestFFTArityMessages(t *testing.T) { for _, tc := range []struct{ expr, want string }{ - // FFT: fixed one, two and three arguments, then both range forms. {"FFT.fft", "wrong number of arguments (given 0, expected 1)"}, {"FFT.irfft", "wrong number of arguments (given 0, expected 2)"}, {"FFT.fft2", "wrong number of arguments (given 0, expected 3)"}, {"FFT.fftfreq", "wrong number of arguments (given 0, expected 1..2)"}, + {"FFT.rfftfreq", "wrong number of arguments (given 0, expected 1..2)"}, + {"FFT.psd", "wrong number of arguments (given 0, expected 1..2)"}, {"FFT.spectrogram", "wrong number of arguments (given 0, expected 4..5)"}, {"FFT.hann", "wrong number of arguments (given 0, expected 1)"}, + {"FFT.fftn", "wrong number of arguments (given 0, expected 2)"}, {"FFT.fft([1.0], 2)", "wrong number of arguments (given 2, expected 1)"}, - // Image, module side. - {"Image.new", "wrong number of arguments (given 0, expected 2)"}, - {"Image.load", "wrong number of arguments (given 0, expected 1)"}, - {"Image.decode", "wrong number of arguments (given 0, expected 1)"}, - // Image, instance side, including the no-argument and range forms. - {"Image.new(2, 2).get", "wrong number of arguments (given 0, expected 2)"}, - {"Image.new(2, 2).set", "wrong number of arguments (given 0, expected 5..6)"}, - {"Image.new(2, 2).canny", "wrong number of arguments (given 0, expected 3)"}, - {"Image.new(2, 2).crop", "wrong number of arguments (given 0, expected 4)"}, - {"Image.new(2, 2).save", "wrong number of arguments (given 0, expected 1)"}, - {"Image.new(2, 2).width(1)", "wrong number of arguments (given 1, expected 0)"}, - {"Image.new(2, 2).grayscale(1)", "wrong number of arguments (given 1, expected 0)"}, - {"Image.new(2, 2).to_png(1)", "wrong number of arguments (given 1, expected 0)"}, - {"Image.new(2, 2).adjust_brightness", "wrong number of arguments (given 0, expected 1)"}, } { t.Run(tc.expr, func(t *testing.T) { src := "begin\n " + tc.expr + "\n puts \"NO ERROR\"\nrescue ArgumentError => e\n puts e.message\nend" @@ -97,37 +75,23 @@ func TestFFTAndImageArityMessages(t *testing.T) { } } -// TestFFTAndImageStillWork is the control for wrapping every method: the checks -// must not have changed any answer. Each row is a call that was correct before -// and has to stay so, including the optional arguments the new ranges allow -// (#set's alpha, #fftfreq's spacing) and #to_s, which is ALIASED from #inspect's -// native function and so inherits whatever wrapper #inspect got. -func TestFFTAndImageStillWork(t *testing.T) { +// TestFFTStillWorks is the control for wrapping every method: the checks must +// not have changed any answer. Each row was right before and has to stay so, +// including the optional spacing the new ranges allow. +func TestFFTStillWorks(t *testing.T) { src := `p FFT.fft([1.0, 2.0, 3.0, 4.0]).map { |c| c.real.round(6) } p FFT.fftfreq(4).map { |f| f.round(4) } p FFT.fftfreq(4, 0.5).map { |f| f.round(4) } p FFT.hann(3).map { |f| f.round(6) } p FFT.irfft(FFT.rfft([1.0, 2.0, 3.0, 4.0]), 4).map { |f| f.round(6) } -i = Image.new(4, 4) -i.set(0, 0, 255, 0, 0) -p i.get(0, 0) -i.set(1, 1, 1, 2, 3, 128) -p i.get(1, 1) -p [i.width, i.height, i.grayscale.width, i.resize(2, 2).width, i.crop(0, 0, 2, 2).width] -p [i.adjust_brightness(1.0).width, i.gaussian_blur(0.5).width, i.canny(1.0, 0.1, 0.2).width] -p i.to_png.bytesize > 0 -p i.to_s == i.inspect` +p FFT.psd([1.0, 2.0, 3.0, 4.0]).size +p FFT.psd([1.0, 2.0, 3.0, 4.0], 0.5).size` const want = "[10.0, -2.0, -2.0, -2.0]\n" + "[0.0, 0.25, -0.5, -0.25]\n" + "[0.0, 0.5, -1.0, -0.5]\n" + "[0.0, 1.0, 0.0]\n" + "[1.0, 2.0, 3.0, 4.0]\n" + - "[255, 0, 0, 255]\n" + - "[1, 2, 3, 128]\n" + - "[4, 4, 4, 2, 2]\n" + - "[4, 4, 4]\n" + - "true\n" + - "true\n" + "3\n3\n" if got := eval(t, src); got != want { t.Errorf("got\n%s\nwant\n%s", got, want) }