Bug in amberframework/optarg or latest Crystal?

Help & Support
1

I have an amber project that fails to compile after upgrading Crystal.

The error in question:

There was a problem expanding macro '__concrete'

Code in lib/optarg/src/lib/value_types/string_array.cr:3:5

 3 | __concrete Array(::String), et: ::String
     ^
Called macro defined in lib/optarg/src/lib/value_types/base.cr:5:5

 5 | macro __concrete(t, et = nil)

Which expanded to:

 > 10 |       
 > 11 |         alias ElementType = ::String
 > 12 |         alias ElementValue = ::Optarg::ValueTypes::::String::Value

(code links)

It’s obvious that the problem is that {{et.id}} expands to ::String rather than String, but is this intended behaviour? Is there a way to get the type unprefixed with ::?

2

This is a bug in the lib, that was caused by fixing a bug in the compiler: Fix `Crystal::Path#to_macro_id` for global path by straight-shoota · Pull Request #14490 · crystal-lang/crystal · GitHub. Previously ::String when you use ASTNode#id would not respect the global :: prefix whereas it now respects if its global or not. Fix for this would be to probably make it et: String and move the :: to alias ElementType = ::{{et.id}}.

3

Thanks!

I’ll make a PR. Though I think it’s a bit sad it looses the symmetry in __concrete Array(::String), et: String.

4

Might be possible to just refactor things a bit more to remove the need to have et entirely. Seems a bit redundant given you can extract String from the Array(::String). Something like t.type_vars.first.resolve.name.id. This assumes that given Array(T) that et always is T tho.

If symmetry is that impt, could prob do the opposite and remove the :: within the macro via like ::Optarg::ValueTypes::{{et.resolve.name.id}}::Value.

5

Thanks again, I’ve added your suggestions to the PR, I think the maintainers has more of an opinion on the subject than I have.

6

:+1:. Prob would have to ask @crimson-knight. AFAIK he’s the only active amber maintainer these days.

1 Like