Crystal: Type of instance vars generated with macros cannot be inferred

[denysvitali]

Version: Crystal 0.20.5 (2017-01-25)
OS: Arch Linux (4.9.9-1-ARCH)

When I do the following:

macro bindValue(attributes)
      {% for name in attributes %}
      attr = node["{{name}}"]
      if attr.is_a?(String)
        @{{name}} = attr
      else
        @{{name}} = ""
      end
      {% end %}
end

def initialize(node : XML::Node)
      bindValue [group, coordinator]
end

I get this error:

Error in src/sonos.cr:6: instantiating 'Sonos::Discovery#topology()'

  puts discovery.topology
                 ^~~~~~~~

in src/sonos/discovery.cr:23: instantiating 'Sonos::TopologyNode:Class#new(XML::Node)'

        tn = TopologyNode.new(node)
                          ^~~

in src/sonos/topology/node.cr:19: expanding macro

      bindValue [group, coordinator]
      ^

in macro 'bindValue' /home/dvitali/Documents/git/sonos-web/src/sonos/topology/node.cr:7, line 4:

   1.       
   2.       attr = node["group"]
   3.       if attr.is_a?(String)
>  4.         @group = attr
   5.       else
   6.         @group = ""
   7.       end
   8.       
   9.       attr = node["coordinator"]
  10.       if attr.is_a?(String)
  11.         @coordinator = attr
  12.       else
  13.         @coordinator = ""
  14.       end
  15.       
  16.     

Can't infer the type of instance variable '@group' of Sonos::TopologyNode

The type of a instance variable, if not declared explicitly with
`@group : Type`, is inferred from assignments to it across
the whole program.

The assignments must look like this:

  1. `@group = 1` (or other literals), inferred to the literal's type
  2. `@group = Type.new`, type is inferred to be Type
  3. `@group = Type.method`, where `method` has a return type
     annotation, type is inferred from it
  4. `@group = arg`, with 'arg' being a method argument with a
     type restriction 'Type', type is inferred to be Type
  5. `@group = arg`, with 'arg' being a method argument with a
     default value, type is inferred using rules 1, 2 and 3 from it
  6. `@group = uninitialized Type`, type is inferred to be Type
  7. `@group = LibSome.func`, and `LibSome` is a `lib`, type
     is inferred from that fun.
  8. `LibSome.func(out @group)`, and `LibSome` is a `lib`, type
     is inferred from that fun argument.

Other assignments have no effect on its type.

But if I wrtie the expanded macro, I get the expected result (no errors):

def initialize(node : XML::Node)
      attr = node["group"]
      if attr.is_a?(String)
        @group = attr
      else
        @group = ""
      end

      attr = node["coordinator"]
      if attr.is_a?(String)
        @coordinator = attr
      else
        @coordinator = ""
      end
end

I wonder if this is a Crystal bug, or a comprehension fault on my side. Pardon me in advance if this isn't a bug, I'm still taking my first steps in Crystal

[spalladino]

Good catch. I've reduced it to this code, seems like a compiler issue.

macro define(var)
  @{{var}} = "VALUE"
end

class Foo
  def initialize
    define myvar
  end
end

Foo.new

[asterite]

Macro calls inside methods are only expanded when the method is invoked. This happens after the compiler gathered the type of all instance variables. So I don't consider this a bug. One way you can solve this is by having a macro that generates the initialize method. This way the macro is invoked in the first pass of the compiler. This is how JSON.mapping and YAML.mapping work, by the way. It also looks like you need something like XML.mapping here (something that's currently missing)

[ezrast]

One benefit of expanding macros up front is that you can use them to define complex types without presenting that complexity to the user - like dynamic aliases, sort of.

class Counter
  getter value = 0

  def increment
    @value += 1
  end
end

class Collection(LabelType)
  # All the magic happens in this macro, which forces LabelType to be a
  # NamedTuple with only String values.
  macro [](*labels)
    Collection(
      NamedTuple(
        {% for label in labels %}
          {{ label.id }}: String,
        {% end %}
      )
    )
  end

  def initialize
    @values = Hash(LabelType, Counter).new do |hash ,key|
      hash[key] = Counter.new
    end
  end

  def [](**labels : **LabelType)
    @values[labels]
  end
end

hits = Collection[:method, :path].new
hits[method: "GET", path: "/"].increment
hits[method: "GET", path: "/"].increment
hits[method: "GET", path: "/about"].increment

pp hits[method: "GET", path: "/"].value # => 2
pp hits[method: "GET", path: "/about"].value # => 1
pp hits[method: "POST", path: "/about"].value # => 0

# Compiler saves me from this typo:
# hits[mtehod: "GET", path: "/"].increment

# But it also prevents me from instantiating this:
class Server
  def initialize
    @counter = Collection[:method, :path].new
  end
end

I could get rid of the LabelType parameter and just use Hash(Symbol, String) everywhere, but it would make #[] uglier and I'd have to implement additional checks at run-time to make up for the ones I lose at compile-time.

Unfortunately, instantiating Collection without the macro requires typing Collection(NamedTuple(method: String, path: String)).new. Since values for the NamedTuple are required by other parts of the library to be String, there's a lot of boilerplate there.

[ezrast]

As a side note, if you do not find the above use case compelling, please consider breaking that macro functionality altogether. I have a fair amount of library code I'm going to have to rewrite because I just figured out that none of it works when assigned to instance variables. It's sort of my fault for not being extra-careful with that level of type system abuse, but still a bit frustrating.

[asterite]

I'm going to close this. Macros inside methods are only expanded at the semantic phase, before guessing the type of instance variables. Plus I'd actually prefer less guessing and more explicit typing, and guessing from macros goes in the other direction.