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

AbstractChannel is currently not parametric #28124

Closed
donm opened this issue Jul 15, 2018 · 4 comments · Fixed by #28174
Closed

AbstractChannel is currently not parametric #28124

donm opened this issue Jul 15, 2018 · 4 comments · Fixed by #28174

Comments

@donm
Copy link
Contributor

donm commented Jul 15, 2018

The AbstractChannel type is not parametric; it's defined as

abstract type AbstractChannel end # channels.jl: 3

But should it be parametric? Will something with the Channel interface ever require more flexibility than would be allowed by this?:

abstract type AbstractChannel{T} end

The reason why this came up is because Channel and RemoteChannel do not share a common supertype below Any, and I'd like to create a type union for functions that accept anything with the channel interface. This is valid and catches most (all?) of the built-in cases:

 AnyChannel{T} = Union{Channel{T}, RemoteChannel{Channel{T}}}  

but trying to expand that definition to AbstractChannels is not valid

 AnyChannel{T} = Union{AbstractChannel{T}, RemoteChannel{AbstractChannel{T}}}  # -> TypeError
@StefanKarpinski
Copy link
Sponsor Member

This seems like a good idea; also shouldn't AbstractChannel be a supertype of RemoteChannel?

@donm
Copy link
Contributor Author

donm commented Jul 16, 2018

That would be excellent, and it would also be great if RemoteChannel supported what seems to be the full Channel interface (it's missing iteration, isopen, ....). But currently RemoteChannel is a Base.Distributed.AbstractRemoteRef. It could instead just have a RemoteRef, but be an AbstractChannel. But that's a bigger change and I haven't thought through the implications.

@StefanKarpinski
Copy link
Sponsor Member

The immediate change to Base (as opposed to stdlib/Distributed) is just adding the type parameter.

@StefanKarpinski StefanKarpinski added the status:triage This should be discussed on a triage call label Jul 16, 2018
@JeffBezanson
Copy link
Sponsor Member

Sounds good.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants