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

runtime: chanrecv funcs don't actually use the *chantype parameter #19591

Closed
mvdan opened this issue Mar 17, 2017 · 7 comments

Comments

Projects
None yet
4 participants
@mvdan
Copy link
Member

commented Mar 17, 2017

func chanrecv(t *chantype, c *hchan, ep unsafe.Pointer, block bool) (selected, received bool)

The t *chantype parameter isn't actually used. I assume it could be removed, but the entry point magic scares me a bit and I don't dare do a CL myself.

Unless the parameter is actually used/needed for a reason that's not obvious given the implementation?

Found with https://github.com/mvdan/unparam.

@mvdan

This comment has been minimized.

Copy link
Member Author

commented Mar 17, 2017

Also worth noting that chansend does use its t *chantype parameter, but only if either raceenabled or msanenabled are true. Would either of those need the type parameter at some point in the future?

@mdempsky

This comment has been minimized.

Copy link
Member

commented Mar 17, 2017

The chantype is stored as a field in the hchan anyway. I think we can get rid of the parameters.

Do you want to send a CL?

@mvdan

This comment has been minimized.

Copy link
Member Author

commented Mar 17, 2017

Sure, I'll get on that.

The chantype is stored as a field in the hchan anyway.

So you mean get rid of it in chansend too?

@mdempsky

This comment has been minimized.

Copy link
Member

commented Mar 17, 2017

So you mean get rid of it in chansend too?

Correct.

@mvdan

This comment has been minimized.

Copy link
Member Author

commented Mar 18, 2017

Having some issues with panics, I'm likely missing some change to make the compiler and/or runtime fully aware of the new func signature. I'll post the CL on monday in any case, asking for pointers if I'm still stuck.

@mvdan

This comment has been minimized.

Copy link
Member Author

commented Mar 18, 2017

Ah, found it - a missing mkcall1 tweak in gc.

@gopherbot

This comment has been minimized.

Copy link

commented Mar 18, 2017

CL https://golang.org/cl/38351 mentions this issue.

@bradfitz bradfitz added this to the Go1.9 milestone Mar 21, 2017

@gopherbot gopherbot closed this in 2e29eb5 Mar 21, 2017

@golang golang locked and limited conversation to collaborators Mar 21, 2018

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
You can’t perform that action at this time.