crystal icon indicating copy to clipboard operation
crystal copied to clipboard

`previous_def` seems to lose information regarding a double-splat when used in a macro.

Open icy-arctic-fox opened this issue 1 year ago • 1 comments

Bug Report

I'm attempting to redefine a method and wrap its previous implementation. A macro reconstructs the signature and uses previous_def to call the original implementation. This doesn't seem to work correctly when combined with a double-splat parameter. The compiler gives an error similar to:

Error: wrong number of arguments for 'Foo#test' (given 2, expected 0..1)

Reduced code:

class Foo
  macro gen_previous
    previous_def
  end

  def test(name = nil, **kwargs)
    p! name, kwargs
  end

  def test(name = nil, **kwargs)
    gen_previous
  end
end

Foo.new.test 10, blah: "Fred"

Crystal 1.7.3

icy-arctic-fox avatar Mar 10 '23 03:03 icy-arctic-fox

Because the call contains named arguments, by the same the normalizer sees the second Foo#test, it is actually looking at something like this:

def test:blah(name : Int32, blah __temp_1 : String)
  kwargs = {blah: __temp_1}
  gen_previous # becomes previous_def(name, __temp_1)
end

Normally this doesn't happen and the normalizer acts first, however macro expansion reverses this order. Thus, the named arguments get turned into positional ones, and adding a single splat will give incorrect results:

module Foo
  macro gen_previous
    previous_def
  end

  def self.foo(*args, **opts)
    {args, opts}
  end

  def self.foo(*args, **opts)
    gen_previous
  end
end

Foo.foo 1, 2, 3, a: 4, b: 5 # => {{1, 2, 3, 4, 5}, {}}

HertzDevil avatar Apr 06 '24 10:04 HertzDevil