Skip to content

Retain original location for errors in included, extended hooks#13261

Merged
straight-shoota merged 2 commits intocrystal-lang:masterfrom
Blacksmoke16:more-accurate-macro-hook-errors
Nov 28, 2025
Merged

Retain original location for errors in included, extended hooks#13261
straight-shoota merged 2 commits intocrystal-lang:masterfrom
Blacksmoke16:more-accurate-macro-hook-errors

Conversation

@Blacksmoke16
Copy link
Member

@Blacksmoke16 Blacksmoke16 commented Apr 1, 2023

Depends on #13260.

Resolves #7394

module Foo
  macro included
    {% raise "noooo" %}
  end
end

include Foo

Before:

$ crystal run --error-trace test.cr
In test.cr:7:1

 7 | include Foo
     ^
Error: at 'Included' hook


In test.cr:3:8

 3 | {% raise "noooo" %}
        ^----
Error: noooo

After:

$ ccrystal run --error-trace test.cr
Using compiled compiler at .build/crystal
In test.cr:3:8

 3 | {% raise "noooo" %}
        ^----
Error: noooo


In test.cr:7:1

 7 | include Foo
     ^
Error: noooo

I'd like to have this one pointing at Foo itself vs include, but not sure how to do that. Can save it for another PR I guess.


abstract struct Parent
  macro inherited
    {% raise "nooo" %}
  end
end

struct Child < Parent
end

Before:

$ crystal run --error-trace test.cr
In test.cr:3:8

 3 | {% raise "nooo" %}
        ^----
Error: nooo

After:

$ ccrystal run --error-trace test.cr
Using compiled compiler at .build/crystal
In test.cr:3:8

 3 | {% raise "nooo" %}
        ^----
Error: nooo


In test.cr:7:8

 7 | struct Child < Parent
            ^
Error: nooo

module ExampleModule
  macro included
    {% verbatim do %}
      {%
        if method = @type.class.methods.find &.name.stringify.==("value")
          method.raise "Value method must be an instance method."
        else
          raise "BUG: Didn't find value method."
        end
      %}
    {% end %}
  end
end

class ExampleClass
  def self.value : Nil
  end

  include ExampleModule
end

Before:

$ crystal run --error-trace test.cr
In test.cr:19:3

 19 | include ExampleModule
      ^
Error: at 'Included' hook


In test.cr:19:3

 19 | include ExampleModule
      ^
Error: expanding macro


There was a problem expanding macro 'included'

Called macro defined in test.cr:2:3

 2 | macro included

Which expanded to:

 >  1 |
 >  2 |       {% if method = @type.class.methods.find do |__arg0|
 >  3 |   __arg0.name.stringify == "value"
 >  4 | end
 >  5 |   method.raise("Value method must be an instance method.")
 >  6 | else
 >  7 |   raise("BUG: Didn't find value method.")
 >  8 | end %}
 >  9 |
 > 10 |
Error: Value method must be an instance method.


In test.cr:16:12

 16 | def self.value : Nil
               ^----
Error: Value method must be an instance method.

After:

$ ccrystal run --error-trace test.cr
Using compiled compiler at .build/crystal
In test.cr:19:3

 19 | include ExampleModule
      ^
Error: at 'Included' hook


In test.cr:19:3

 19 | include ExampleModule
      ^
Error: expanding macro


There was a problem expanding macro 'included'

Called macro defined in test.cr:2:3

 2 | macro included

Which expanded to:

 >  1 |
 >  2 |       {% if method = @type.class.methods.find do |__arg0|
 >  3 |   __arg0.name.stringify == "value"
 >  4 | end
 >  5 |   method.raise("Value method must be an instance method.")
 >  6 | else
 >  7 |   raise("BUG: Didn't find value method.")
 >  8 | end %}
 >  9 |
 > 10 |
Error: Value method must be an instance method.


In test.cr:16:12

 16 | def self.value : Nil
               ^----
Error: Value method must be an instance method.

EDIT: Actually I'm not sure what the more proper error line is. E.g. is it pointing to the actual error if there isn't a specific node, or should it continue to be include Foo, but with the ^ pointing at the actual module name like the OP described?

@Blacksmoke16 Blacksmoke16 added kind:bug A bug in the code. Does not apply to documentation, specs, etc. topic:compiler labels Apr 1, 2023
@Blacksmoke16 Blacksmoke16 marked this pull request as draft April 1, 2023 04:18
@Blacksmoke16 Blacksmoke16 force-pushed the more-accurate-macro-hook-errors branch from 742a2c4 to 5d4d93e Compare December 11, 2023 04:47
@Blacksmoke16 Blacksmoke16 marked this pull request as ready for review December 11, 2023 04:47
@straight-shoota straight-shoota removed their request for review December 22, 2023 16:37
@straight-shoota straight-shoota added kind:feature and removed kind:bug A bug in the code. Does not apply to documentation, specs, etc. labels May 13, 2024
@Blacksmoke16
Copy link
Member Author

This is good for a review now. I think it makes sense we show the location of include/inherit that caused the error as the first frame. This way you know the specific context the error is in and could --error-trace for more details.

@straight-shoota straight-shoota added this to the 1.19.0 milestone Nov 26, 2025
@straight-shoota straight-shoota merged commit b609bb7 into crystal-lang:master Nov 28, 2025
40 checks passed
@straight-shoota straight-shoota changed the title Avoid raising a new exception when processing macro included/extended hooks to retain original location Retain original location for errors in included, extended hooks Nov 28, 2025
@Blacksmoke16 Blacksmoke16 deleted the more-accurate-macro-hook-errors branch December 16, 2025 14:42
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Keep type location when raising inside macro hooks

3 participants