Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Can't receive from channel of abstract type #8216

Closed
carlhoerberg opened this issue Sep 24, 2019 · 8 comments · Fixed by #8221

Comments

@carlhoerberg
Copy link
Contributor

commented Sep 24, 2019

Following code:

abstract class A; end
class B < A; end
ch = Channel(A).new(1)
ch.send B.new
ch.receive

Results in:

✗ crystal run ch_receive_abstract.cr --error-trace
In ch_receive_abstract.cr:5:4

 5 | ch.receive
        ^------
Error: instantiating 'Channel(A)#receive()'


In /usr/local/Cellar/crystal/0.31.0/src/channel.cr:149:5

 149 | receive_impl { raise ClosedError.new }
       ^-----------
Error: instantiating 'receive_impl()'


In /usr/local/Cellar/crystal/0.31.0/src/channel.cr:161:11

 161 | @lock.sync do
             ^---
Error: instantiating 'Crystal::SpinLock#sync()'


In /usr/local/Cellar/crystal/0.31.0/src/channel.cr:161:11

 161 | @lock.sync do
             ^---
Error: instantiating 'Crystal::SpinLock#sync()'


In /usr/local/Cellar/crystal/0.31.0/src/channel.cr:162:7

 162 | receive_internal do
       ^---------------
Error: instantiating 'receive_internal()'


In /usr/local/Cellar/crystal/0.31.0/src/channel.cr:162:7

 162 | receive_internal do
       ^---------------
Error: instantiating 'receive_internal()'


In /usr/local/Cellar/crystal/0.31.0/src/channel.cr:167:20

 167 | @receivers << {Fiber.current, pointerof(value), pointerof(state), nil}
                  ^-
Error: no overload matches 'Deque(Tuple(Fiber, Pointer(A), Pointer(Channel::DeliveryState), Channel::SelectContext(A) | Nil))#<<' with type Tuple(Fiber, Pointer(A), Pointer(Channel::DeliveryState), Nil)

Overloads are:
 - Deque(T)#<<(value : T)
@carlhoerberg

This comment has been minimized.

Copy link
Contributor Author

commented Sep 24, 2019

Currently a workaround is to use Channel.select(ch.receive_select_action)

abstract class A; end
class B < A; end
ch = Channel(A).new(1)
ch.send B.new
_, b = Channel.select(ch.receive_select_action)
@asterite

This comment has been minimized.

Copy link
Member

commented Sep 24, 2019

@bcardiff @waj Could we update the code to use record instead of Tuple? It will fix the error and it will make the code clearer. Right now we have receiver[1], sender[0], etc., which is very hard to understand. Use a record with names instead of indices and it becomes much better (and it will compile too!).

@bcardiff

This comment has been minimized.

Copy link
Member

commented Sep 24, 2019

That can be changed, but I would be more interested in finding how to use the tuples for directly. Either a subtyping is missing somewhere in the compiler or an upcast is missing in the Channels code.

@asterite

This comment has been minimized.

Copy link
Member

commented Sep 24, 2019

Hm, the problem still happens when using a record. It seems to be a compiler bug.

@asterite

This comment has been minimized.

Copy link
Member

commented Sep 24, 2019

Reduced:

class Base
end

class Sub < Base
end

u = uninitialized Base

x : Pointer(Base)
x = pointerof(u)

Something related to pointerof...

Workaround:

class Base
end

class Sub < Base
end

u = uninitialized Base

x : Pointer(Base)
x = pointerof(u).as(Base*)

Almost sure pointerof isn't creating a virtual type when needed. If that's the case I'll send a fix later today. It should also fix it for tuples.

@RX14

This comment has been minimized.

Copy link
Member

commented Sep 24, 2019

Still, that ugly channel code using tuples and meaningful indexes instead of names (either NamedTuple or record) should be changed.

I would have asked for that if I'd been in time to review, actually.

kalinon added a commit to kalinon/amqp-client.cr that referenced this issue Sep 24, 2019
workaround for crystal-lang/crystal#8216
@andrius

This comment has been minimized.

Copy link

commented Sep 25, 2019

Also get this error by just upgrading crystal, with AMQP library: https://github.com/cloudamqp/amqp-client.cr

In lib/amqp-client/src/amqp-client/connection.cr:51:37

 51 | ch = @channels[i] = Channel.new(self, i)
                                  ^--
Error: instantiating 'AMQP::Client::Channel.class#new(AMQP::Client::Connection, UInt16)'


In lib/amqp-client/src/amqp-client/channel.cr:21:7

 21 | spawn read_loop, name: "Channel #{@id} read_loop"
      ^
Error: expanding macro


There was a problem expanding macro 'spawn'

Called macro defined in /usr/share/crystal/src/concurrent.cr:100:1

 100 | macro spawn(call, *, name = nil, same_thread = false, &block)
@asterite

This comment has been minimized.

Copy link
Member

commented Sep 25, 2019

@andrius That seems like a different error. Could you provide the full error trace?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
5 participants
You can’t perform that action at this time.