Crystal: Macro raise doesn't keep location

[bew]

Extracted from #6125 (https://github.com/crystal-lang/crystal/issues/6125#issuecomment-423640385)

macro raise_on(node)
  {% node.raise "node '#{node}' is here!" %}
end

macro will_raise_soon(node)
  # this is a helper macro
  raise_on {{node}}
end

macro will_raise(node)
  # this is a helper macro
  will_raise_soon {{node}}
end

will_raise :this

I think this should report the error node ':this' is here! at the last line (will_raise :this), without any trace (but allow to show it with --error-trace)

(might need a change in how macro's raise is reported)

---

_thought on this by @MakeNowJust:_ https://github.com/crystal-lang/crystal/pull/7008#issuecomment-434555247

The reason what #6125 (comment) does not work is for re-raising an error with node calling macro location here:

  def eval_macro(node)
    yield
  rescue ex : MacroRaiseException
    node.raise ex.message, exception_type: MacroRaiseException
  rescue ex : Crystal::Exception
    node.raise "expanding macro", ex
  end

[bew]

Actually it doesn't even work correctly (IMO) currently with a single macro call:

For exemple, with this (normal macro raise):

macro macro_raise
  {% raise "Raised in here" %}
end

macro_raise

We get:

Error in macro_raise.cr:5: Raised in here

macro_raise
^~~~~~~~~~~

Which is nice.

---

Now if we raise on a specific node in a macro (doesn't work as I would expect):

macro macro_raise_on(arg)
  {% arg.raise "Raised here on #{arg}" %}
end

macro_raise_on :this

We get:

Error in macro_raise.cr:5: Raised here on :this

macro_raise_on :this
^~~~~~~~~~~~~~

But I would expect:

Error in macro_raise.cr:5: Raised here on :this

macro_raise_on :this
               ^~~~~

Fixing this might fix the original code (https://github.com/crystal-lang/crystal/issues/7147#issue-387545421), but I'm not sure.

[asterite]

Yes, this is strange. I think it used to work in the past.

[franciscoadasme]

edit: I just realized that I meant to post this here instead of the other, related issue #7394. Original comment was deleted.

Another (more problematic) case is when requiring a file that calls a macro that raises, where the error message points to the require statement instead of the macro call location, e.g.,

# foo.cr
struct Foo
  macro foo
    {{ raise "error when calling foo" }}
  end
end

# bar.cr
require "./foo"

Foo.foo

# baz.cr
require "./bar.cr"

when running bar.cr, you get (expected behavior)

Error in bar.cr:3: error on foo

Foo.foo
    ^~~

but if you run baz.cr, you get

Error in baz.cr:1: error on foo

require "./bar"
^

If you have several nested files (or when bar.cr is very large), it'll be very difficult to find the aggravating line

[Blacksmoke16]

@bew, would you not expect the trace to show the line that was raised in macro land when using node.raise? I.e.

In test.cr:2:11

 2 | {% node.raise "node '#{node}' is here!" %}
             ^----
Error: node ':this' is here!

then using --error-trace would go up the tree:

ccrystal test.cr --error-trace
Using compiled compiler at .build/crystal
There was a problem expanding macro 'will_raise'

Called macro defined in test.cr:10:1

 10 | macro will_raise(node)

Which expanded to:

 > 1 | # this is a helper macro
 > 2 | will_raise_soon :this
Error: expanding macro


There was a problem expanding macro 'will_raise_soon'

Called macro defined in test.cr:5:1

 5 | macro will_raise_soon(node)

Which expanded to:

 > 1 | # this is a helper macro
 > 2 | raise_on :this
Error: expanding macro


In test.cr:2:11

 2 | {% node.raise "node '#{node}' is here!" %}
             ^----
Error: node ':this' is here!

[bew]

No, that's the whole point of this issue: when I use node.raise, I expect to see the error pointing to the node in question, where it is actually written in the original code.

In your trace I find it hard to find the node you raised on for example.

Do you see what I mean?

[asterite]

I agree with @bew

[Blacksmoke16]

Ah, you mean like:

Error in macro_raise.cr:5: Raised here on :this

macro_raise_on :this
               ^~~~~

like you mentioned earlier.

Yea that makes sense.

Maybe this is a slightly different issue then. In Athena I provide a fair bit of compile time errors by raising on specific nodes (like a specific class/method). However currently this is what happens:

Showing last frame. Use --error-trace for full trace.

In test.cr:26:18

 26 | pp RouteResolver.new
                       ^--
Error: Routes can only be defined as instance methods.  Did you mean 'RoutingController#user'?

https://play.crystal-lang.org/#/r/91b6

Which isn't really helpful at all.

IMO ideally it would be like:

In test.cr:8:7

 8 | def self.user : Nil
         ^--------
Error: Routes can only be defined as instance methods.  Did you mean 'RoutingController#user'?

[asterite]

You both want the same thing. The problem is that raise on a node doesn't retain that location and it's swallowed by the call that invokes the macro. That's the bug. If we fix that, both cases will be fixed.