Crystal: ICE using type parameter in top-level code

[Grabli66]

Input:

class Klass(T)
  Channel(T).new
end

Output:

BUG: called create_llvm_type for T
Error opening file 'crystal' with mode 'r': No such file or directory (Errno)
Failed to raise an exception: END_OF_STACK
[0x1a48236] ???
[0x11f6119] __crystal_raise +153
[0x11f7838] ???
[0x1208c07] ???
[0x12021c6] ???
[0x12014fe] ???
[0x11fe292] ???
[0x11fad36] main +438
[0x7fe6c7f62830] __libc_start_main +240
[0x11f35d9] _start +41
[0x0] ???

Crystal 0.24.2 (2018-05-16)
LLVM: 5.0.1
Default target: x86_64-unknown-linux-gnu

[RX14]

This should of course be a compile error because T is not available in that context.

I vote for disallowing any top-level code that's outside the root namespace. It's an anti-feature.

i.e.

puts "foo"

is allowed but

class Foo
  puts "foo"
end

is disallowed.

[straight-shoota]

And what about this:

class Foo
  property foo
end

[jwoertink]

Then what would happen with something like this? What if that was

class InitHandler(T)
  INSTANCE = new(T)
  ...
end

[RX14]

@straight-shoota that's a macro, and expands to some method definitions and an ivar def, no top-level code.

@jwoertink that's a constant definition, and isn't top-level code (the right hand size of constants is actually lazilly initialized and therefore completely out of sequence with the top-level code, for example you can do

puts Foo::CONST

class Foo
  CONST = "bar"
end

which makes it clear that constants aren't top-level code.

[jwoertink]

Ah, nice. How about something like this? @RX14

class Api < Base
  get "/whatever" do |env|
    "would this go away?"
  end
end

edit: thinking about it, I guess it would really depend on the implementation. If it were a macro that expanded to a method somehow called later, then it would be ok..

[RX14]

if get is a macro which doesn't generate top-level code, it'd be fine. If get is a normal method, i'd put it in a def routes method, which Base calls, otherwise it's magic and magic is bad.

You could make the get macro expand to something like

def routes
  previous_def

  get_impl {{str}} { {{block}} }
end

which is a really short, readable macro then implement the rest using just inheritance and a normal def get_impl method.

[straight-shoota]

@RX14 Yes, property is a macro call, but still a "top level" expression in that sense. And such a macro can expand to include top level expressions. Should that be forbidden?

[RX14]

@straight-shoota yes, that's what I meant. Ban all top-level not at the root namespace after macro expansion (conceptually, we don't have to implement it this way).