Crystal: Reusing a constant's value in a macro

Created on 30 Mar 2016  路  14Comments  路  Source: crystal-lang/crystal

Say we have a type and we want to provide both JSON and YAML mapping. We can do this:

require "json"
require "yaml"

class Point
  JSON.mapping({
    x: Int32,
    y: Int32,
  })

  YAML.mapping({
    x: Int32,
    y: Int32,
  })
end

string = %({"x": 1, "y": 2})
p Point.from_json(string)
p Point.from_yaml(string)

In this case the mapping is exactly the same, so we'd want to refactor it like this:

class Point
  MAPPING = {
    x: Int32,
    y: Int32,
  }

  JSON.mapping(MAPPING)
  YAML.mapping(MAPPING)
end

However, that won't work. The problem is that the mapping macros expect a HashLiteral as an argument, but now they receive a Path (a Path is something like Foo::Bar::Baz or just Foo).

An alternative is to do this:

class Point
  {% for type in %w(JSON YAML) %}
    {{type.id}}.mapping({
      x: Int32,
      y: Int32,
    })
  {% end %}
end

but I'd say that's just a workaround. I'd really like the original refactor to work... somehow.

(A separate discussion is whether the way to define mappings is ok or not, maybe the mapping should be defined as attributes of instance variables... but it's a separate discussion, the point here is to reuse code in macros)

A different example: a macro that gets some info from a type, for example its instance variables:

macro instance_vars(type)
  {{type.instance_vars}}
end

instance_vars(SomeType)

Same problem: type is a Path, not a TypeNode denoting a type (the one you get if you use @type inside a macro, of if you mention a type inside a macro, like writing SomeType inside it, like if you do {{SomeType.instance_vars}}).

One solution is, if a method is not found in a Path (and right now Path has no macro methods), then it's solved to either its constant value or type. So, the above examples would work transparently, although it might be a bit confusing.

Another solution is to make this explicit with a resolve(...) macro method, but in the end every macro would have to invoke this "just in case" someone passes a Path instead of the required type.

I'm more inclined to implement the first solution, but do you know other solutions to this?

draft compiler

Most helpful comment

With the above commit this is now possible:

macro include_constants(t)
  {% for c in t.resolve.constants %}
    {{c}} = {{t}}::{{c}}
  {% end %}
end

enum E
  A, B, C
end

class T1
  include_constants E
end

p T1::A # works
p T1::B # works
p T1::C # works

All 14 comments

What about call side syntax for this? Something like JSON.mapping(MAPPING.value)?

@jhass It's a good idea. But it would have to be syntax that doesn't conflict with the current one: MAPPING.value is a Call, so what if you do want to pass something like Foo.value?

What about using {{...}} for forcing the evaluation of macro arguments so instead of passing a ASTNode the value is passed?

...
  JSON.mapping({{MAPPING}})
  YAML.mapping({{MAPPING}})
end

Or maybe #{...}.
Otherwise I see we will end with a clojure kind of quote char for stoping or resuming evaluation. But it is no easy to pick a char for this '`^ ?

...
  JSON.mapping(^MAPPING)
  YAML.mapping(^MAPPING)
end

Granted, it would require new syntax that then isn't passable to a macro.

I think what @bcardiff suggests might work. Right now that is passed as a MacroExpression to the macro, but the macro expander could in turn expand arguments that are macro expressions. And the best thing is that no new syntax will be needed. The only drawback is that you can't pass a macro expression to a macro, but I don't know if that's very useful...

I have some doubts about what will happen with nested macros, maybe there is an issue in that use case, but I think sit down to think about it. But the {{..}} syntax felt right for top level usages.

That's why delegating to the resolved type transparently will always work, even with nested macros. I think I'd like to try that solution and see what happens :-)

Ummm... well, as soon as I started implementing this I realized it won't work. For example all AST nodes have a stringify method, or class_name method. If you invoke one of those methods, would you want to invoke it on the Path or in the resolved constant/type? Not very nice. So yes, the {{...}} solution now convinces me more :-)

How about a new node, or flag, for pure literal-consts, instead of generic Path? When a Path is assigned a literal, it gets LiteralConst or something along those lines from the parser. Then macro can automatically use the value for those and generic Paths are passed as is?

This conversation was sparked when I found out that you can get a TypeNode only from the type that you're currently inside; there's no way for a macro to receive a TypeNode argument (which is only one of the use cases that this issue touches).

But I also found this silly workaround through module ...(T) and macro included which actually has access to the T as a TypeNode.

I just realized something: macros are usually not made for 2 kinds of arguments (Path / constant), so if you use a macro made for constants you always have to use these brackets when calling, which is annoying (as is making a wrapped macro as demonstrated in @asterite's PR).

Another solution is to make this explicit with a resolve(...) macro method, but in the end every macro would have to invoke this "just in case" someone passes a Path instead of the required type.

I don't understand this. How can you "pass a Path instead of the required type" when such solution presumes that you can ONLY pass a path (as it always was)?

@BlaXpirit Take for example the JSON.mapping macro. It expects a HashLiteral as an argument. If you have this literal inside a constant there's no way to pass the constant's value as an argument to the macro. With {{...}} you can do that.

For the other cases where the macro expects a type node, we can add a resolve macro method on Path. If I understand you correctly, you say there's no way to pass a TypeNode because right now you can only pass a Path. So here a resolve method would be very useful. However, the {{...}} is needed in case where you want to pass a constant's value. I talk about that in #2392, but now I want to include this resolve method in it too, for the case of the include_constants example. Is resolve a good name? What would be a better name?

@asterite Oh, of course, this all makes sense. I spaced out and forgot that you can pass literals.

resolve sounds great! Indeed, these two solutions complement each other.

With the above commit this is now possible:

macro include_constants(t)
  {% for c in t.resolve.constants %}
    {{c}} = {{t}}::{{c}}
  {% end %}
end

enum E
  A, B, C
end

class T1
  include_constants E
end

p T1::A # works
p T1::B # works
p T1::C # works
Was this page helpful?
0 / 5 - 0 ratings

Related issues

oprypin picture oprypin  路  3Comments

ArthurZ picture ArthurZ  路  3Comments

grosser picture grosser  路  3Comments

RX14 picture RX14  路  3Comments

jhass picture jhass  路  3Comments