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

in Channel `put!`, convert value to Channel type #29092

Merged
merged 1 commit into from Oct 20, 2018

Conversation

5 participants
@tanmaykm
Copy link
Member

commented Sep 8, 2018

put!(ch::Channel{T}, v) should convert v to type T.

This can prevent errors like:

julia> c = Channel{Int}(0)
Channel{Int64}(sz_max:0,sz_curr:0)

julia> @async put!(c, :a)
Task (runnable) @0x00007ff5b9d79270

julia> isready(c) && take!(c)
ERROR: TypeError: in take_unbuffered, in typeassert, expected Int64, got Symbol
Stacktrace:
 [1] take_unbuffered(::Channel{Int64}) at ./channels.jl:323
 [2] take!(::Channel{Int64}) at ./channels.jl:306
 [3] top-level scope at none:0

@vchuravy vchuravy added the needs tests label Sep 8, 2018

check_channel_state(c)
isbuffered(c) ? put_buffered(c,v) : put_unbuffered(c,v)
vT = convert(T, v)

This comment has been minimized.

Copy link
@StefanKarpinski

StefanKarpinski Sep 10, 2018

Member

It's no longer a performance issue to reuse the name v for this. Style is a different question, but I thought I'd mention it.

This comment has been minimized.

Copy link
@tanmaykm

tanmaykm Sep 11, 2018

Author Member

Thanks. I was partly skeptical when I did this. Will add a commit to reuse name.

in Channel `put!`, convert value to Channel type
`put!(ch::Channel{T}, v)` should convert `v` to type `T`.

This can prevent errors like:

```
julia> c = Channel{Int}(0)
Channel{Int64}(sz_max:0,sz_curr:0)

julia> @async put!(c, :a)
Task (runnable) @0x00007ff5b9d79270

julia> isready(c) && take!(c)
ERROR: TypeError: in take_unbuffered, in typeassert, expected Int64, got Symbol
Stacktrace:
 [1] take_unbuffered(::Channel{Int64}) at ./channels.jl:323
 [2] take!(::Channel{Int64}) at ./channels.jl:306
 [3] top-level scope at none:0
```

@tanmaykm tanmaykm force-pushed the tanmaykm:tan/chantype branch from 4fc7f69 to 63e84da Sep 11, 2018

@tanmaykm

This comment has been minimized.

Copy link
Member Author

commented Sep 11, 2018

added tests

@JeffBezanson

This comment has been minimized.

Copy link
Member

commented Sep 12, 2018

I think this is a bugfix, but will see what triage thinks about backporting.

@StefanKarpinski

This comment has been minimized.

Copy link
Member

commented Sep 13, 2018

I'd be curious to hear what @DatName's objection to this is.

@DatName

This comment has been minimized.

Copy link

commented Sep 13, 2018

@StefanKarpinski I think, that if you are explicitly creating a channel for Int, you should not be putting some other stuff in there silently. It is easy to do a convertion explicitly on user's side, if needed, so no need for this "behind-the-curtains-magic", so to speak.

@StefanKarpinski

This comment has been minimized.

Copy link
Member

commented Sep 13, 2018

I see your point of view. However, we do automatic conversion when values are put into typed contexts of all kinds already. Our conversions are quite strict about what conversions they will do without error: the value must be convertible exactly or an InexactError will be thrown.

@JeffBezanson

This comment has been minimized.

Copy link
Member

commented Sep 13, 2018

For values that aren't convertible (e.g. a string) this gives an error sooner than we did before.

@JeffBezanson JeffBezanson removed the bugfix label Sep 13, 2018

@JeffBezanson JeffBezanson added this to the 1.1 milestone Sep 13, 2018

@JeffBezanson JeffBezanson added minor change and removed triage labels Sep 13, 2018

@JeffBezanson JeffBezanson merged commit 74657ec into JuliaLang:master Oct 20, 2018

2 of 3 checks passed

ci/circleci No test commands were found
Details
continuous-integration/appveyor/pr AppVeyor build succeeded
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details

mortenpi added a commit to mortenpi/julia that referenced this pull request Dec 1, 2018

fredrikekre added a commit that referenced this pull request Dec 5, 2018

Addition of NEWS and compat admonitions for important
changes between Julia 1.0 and 1.1, including:

- Custom .css-style for compat admonitions.

- Information about compat annotations to CONTRIBUTING.md.

- NEWS.md entry for PRs #30090, #30035, #30022, #29978,
  #29969, #29858, #29845, #29754, #29638, #29636, #29615,
  #29600, #29506, #29469, #29316, #29259, #29178, #29153,
  #29033, #28902, #28761, #28745, #28708, #28696, #29997,
  #28790, #29092, #29108, #29782

- Compat annotation for PRs #30090, #30013, #29978,
  #29890, #29858, #29827, #29754, #29679, #29636, #29623,
  #29600, #29440, #29316, #29259, #29178, #29157, #29153,
  #29033, #28902, #28878, #28761, #28708, #28156, #29733,
  #29670, #29997, #28790, #29092, #29108, #29782, #25278

- Documentation for broadcasting CartesianIndices (#30230).
- Documentation for Base.julia_cmd().
- Documentation for colon constructor of CartesianIndices (#29440).
- Documentation for ^(::Matrix, ::Number) and ^(::Number, ::Matrix).

- Run NEWS-update.jl.

Co-authored-by: Morten Piibeleht <morten.piibeleht@gmail.com>
Co-authored-by: Fredrik Ekre <ekrefredrik@gmail.com>

fredrikekre added a commit that referenced this pull request Dec 5, 2018

Addition of NEWS and compat admonitions for important
changes between Julia 1.0 and 1.1, including:

- Custom .css-style for compat admonitions.

- Information about compat annotations to CONTRIBUTING.md.

- NEWS.md entry for PRs #30090, #30035, #30022, #29978,
  #29969, #29858, #29845, #29754, #29638, #29636, #29615,
  #29600, #29506, #29469, #29316, #29259, #29178, #29153,
  #29033, #28902, #28761, #28745, #28708, #28696, #29997,
  #28790, #29092, #29108, #29782

- Compat annotation for PRs #30090, #30013, #29978,
  #29890, #29858, #29827, #29754, #29679, #29636, #29623,
  #29600, #29440, #29316, #29259, #29178, #29157, #29153,
  #29033, #28902, #28878, #28761, #28708, #28156, #29733,
  #29670, #29997, #28790, #29092, #29108, #29782, #25278

- Documentation for broadcasting CartesianIndices (#30230).
- Documentation for Base.julia_cmd().
- Documentation for colon constructor of CartesianIndices (#29440).
- Documentation for ^(::Matrix, ::Number) and ^(::Number, ::Matrix).

- Run NEWS-update.jl.

Co-authored-by: Morten Piibeleht <morten.piibeleht@gmail.com>
Co-authored-by: Fredrik Ekre <ekrefredrik@gmail.com>

fredrikekre added a commit that referenced this pull request Dec 5, 2018

Compat admonitions and NEWS for Julia 1.1 (#30230)
Addition of NEWS and compat admonitions for important changes between Julia 1.0 and 1.1, including:

- Custom .css-style for compat admonitions.

- Information about compat annotations to CONTRIBUTING.md.

- NEWS.md entry for PRs #30090, #30035, #30022, #29978,
  #29969, #29858, #29845, #29754, #29638, #29636, #29615,
  #29600, #29506, #29469, #29316, #29259, #29178, #29153,
  #29033, #28902, #28761, #28745, #28708, #28696, #29997,
  #28790, #29092, #29108, #29782

- Compat annotation for PRs #30090, #30013, #29978,
  #29890, #29858, #29827, #29754, #29679, #29636, #29623,
  #29600, #29440, #29316, #29259, #29178, #29157, #29153,
  #29033, #28902, #28878, #28761, #28708, #28156, #29733,
  #29670, #29997, #28790, #29092, #29108, #29782, #25278

- Documentation for broadcasting CartesianIndices (#30230).
- Documentation for Base.julia_cmd().
- Documentation for colon constructor of CartesianIndices (#29440).
- Documentation for ^(::Matrix, ::Number) and ^(::Number, ::Matrix).

- Run NEWS-update.jl.


Co-authored-by: Morten Piibeleht <morten.piibeleht@gmail.com>
Co-authored-by: Fredrik Ekre <ekrefredrik@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.