Filtering methods in a derived class's macro can result in empty MacroIds if splatted in a push call

[refi64]

The title makes no sense, but I have no idea how else to explain it. In short:

annotation Ann
end

class Parent
  def initialize
  {% begin %}
    {% a = [] of Def %}
    {% b = @type.methods.select &.annotation(Ann) %}
    {% a.push *b %}
    {% puts "a: #{a} (size: #{a.size}), a[0]: '#{a[0]}', class: #{a[0].class_name}; b: #{b} (size: #{b.size})" %}
  {% end %}
  end

  @[Ann]
  def func
  end
end

class Derived < Parent
end

Derived.new

This prints:

a: [] (size: 1), a[0]: '', class: "MacroId"; b: [] (size: 0)

In other words:

• b.size == 0 and a.size == 0
• a.push *b is used to append b's zero elements onto a
• Now a.size == 1, and it contains a single, empty MacroId

Even if I more methods annotated with @[Ann] in Parent are added, only a single MacroId will ever appear.

[asterite]

The problem is that the splat operator doesn't work in macro calls. I forgot to implement this (Oops!).

Right now * (splat) will turn antyhing into a macro id of the elements joined by comma. If there are no elements, an empty macro id will be returned.

Reduced:

{% begin %}
  {% a = [] of Nil %}
  {% b = [] of Nil %}
  {% a.push *b %}
  {% puts a %}
  {% puts a.size %}
  {% puts a[0].class_name %}
{% end %}

Output:

[]
1
MacroId

[asterite]

Also push in macros just accepts one argument, so that would have to be fixed too...

[refi64]

Ohh, that makes sense, when I first stumbled upon this splat behavior I guess I didn't realize it and thought it was just a cool trick for joining arrays inside macros. 😅

On Sun, Mar 1, 2020, 5:51 PM Ary Borenszweig [email protected] wrote:

Also push in macros just accepts one argument, so that would have to be fixed too...

— You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub https://github.com/crystal-lang/crystal/issues/8871?email_source=notifications&email_token=AAM4YSNWDLLKP6O6TBLJY2TRFLYGXA5CNFSM4K7JPZNKYY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGOENNQCCA#issuecomment-593166600, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAM4YSN5U2PZKXWCSYAQFU3RFLYGXANCNFSM4K7JPZNA .

[HertzDevil]

*b simply delegates to b.splat. IMO it is better if we distinguish between splatting of AST nodes in the macro language (*) and splatting interpolations (#splat).

That said, I wouldn't expect ArrayLiterals to be splattable even in the macro language; instead TupleLiterals can be splatted. The appropriate solution here is to define ArrayLiteral#concat.

[Sija]

The appropriate solution here is to define ArrayLiteral#concat.

fwiw, there's already ArrayLiteral#+.

[HertzDevil]

#+ is non-mutating, #concat would be. ArrayLiterals have reference semantics.

[HertzDevil]

The splat operator is now deprecated in #13939, so all that remains is whether ArrayLiteral#concat is still needed, or {% b.each { |m| a.push m } %} is good enough.