crystal-lang / crystal

The Crystal Programming Language
https://crystal-lang.org
Apache License 2.0
19.42k stars 1.62k forks source link

Misleading error "Unexpected token" when using macros #13733

Open KCErb opened 1 year ago

KCErb commented 1 year ago

The following made me think I was using %w wrong!

class Foo
  PROPS = %w(one two three)
  {% for prop in PROPS %}
    puts "oops. Forgot to call id on {{prop}}"
  {% end %}
end

Error

syntax error in eval:2
Error: unexpected token: "one"

Turns out I forgot to call prop.id down on line 4 so the macro expands to "oops. Forgot to call id on "one"". In my actual use case the error and callsite were dozens of lines apart. So don't let the fact they are so close here fool you. The compiler actually seems to think the issue is up on the PROPS declaration not the output of the macro expansion.


 $ crystal -v
Crystal 1.9.2 [1908c816f] (2023-07-19)

LLVM: 15.0.7
Default target: aarch64-apple-darwin```
devnote-dev commented 1 year ago

While the error message is a bit cryptic, I think this is fair from the compiler as it can't really make assumptions with what you're doing in the macro land the same way it can in the normal land. An easy way to view the macro land is like a template (and in essence, they operate on almost the same levels). The template doesn't care what you're doing or embedding in the actual code, that's not its job. It only needs to worry about you using it correctly — that is, you abiding by macro land rules.

KCErb commented 1 year ago

@devnote-dev that makes sense to me. I guess my question is why does the error point to line 2 (which is valid) instead of to the generated invalid line?

devnote-dev commented 1 year ago

You can think of the real land (normal Crystal code) as having no knowledge of the macro land, so while we see the macro code, the normal land sees this:

class Foo
  PROPS = %w(one two three)
    puts "oops. Forgot to call id on ""one""
end

So from its perspective, that error location is correct. I'm not sure if there is a way to make the normal land aware of macro land contexts without it breaking language features.

KCErb commented 1 year ago

I think there is a miscommunication between us @devnote-dev. The issue is not that it throws an error on line 3 where the mistake is. The issue is that it throws an error on line 2 where the mistake isn't. I thought originally that maybe I should make my example longer so that it would be more clear that I'm getting an error line number on the wrong line! Try this for example:

class Foo
  PROPS = %w(one two three)
  def initialize
    puts "I'm initializing and taking up more vertical space!"
  end

  def bar
  {% for prop in PROPS %}
    puts "oops. Forgot to call id on {{prop}}"
  {% end %}
  end
end

Foo.new.bar

The compiler still complains about line 2 even though the error is on line 8 in the expanded code.

https://play.crystal-lang.org/#/r/flhf

For completeness, here is the expanded code and the error I get

class Foo
  PROPS = %w(one two three)
  def initialize
    puts "I'm initializing and taking up more vertical space!"
  end

  def bar
    puts "oops. Forgot to call id on "one""
    puts "oops. Forgot to call id on "two""
    puts "oops. Forgot to call id on "three""
  end
end

Foo.new.bar

Error

syntax error in eval:8
Error: unexpected token: "one"

https://play.crystal-lang.org/#/r/flhg

This is the error I would expect (though it would be nice if the compiler could tell the error was in expanded code and show it to me).

devnote-dev commented 1 year ago

Ah yes, you're right. I don't know much about how the compiler parses macro code on a technical level so I'll wait for someone else to respond here.

straight-shoota commented 1 year ago

I believe the AST node "one" carries the original location of its definition in line 2. In some cases this might be useful and expected, but here it doesn't do very well. It could be better to always use the location of the var instead. Or maybe we could inject two location frames, for both the var and the value? Probably need to consider more examples to fully take the effects into consideration.

I have an experimental branch at https://github.com/straight-shoota/crystal/tree/feature/macro-var-location where the value takes the location of the var. Feel free to experiment with it.

There is a side effect of duplicating the value (no other way to give it different locations) and that could cause some negative effects. Maybe this relocating should only happen when the macro value is stringified.