Skip to content

Stdin:read_line does not need &mut self #26890

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.

Sign up for GitHub

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

Jump to bottom
Closed
RalfJung opened this issue Jul 8, 2015 · 5 comments · Fixed by #26897
Closed

Stdin:read_line does not need &mut self #26890

RalfJung opened this issue Jul 8, 2015 · 5 comments · Fixed by #26897
Labels
T-libs-api Relevant to the library API team, which will review and decide on the PR/issue.

Comments

@RalfJung
Copy link
Member

RalfJung commented Jul 8, 2015

Currently, Stdin::read_line takes self as &mut. Why is that? The implementation simply locks and calls read_line on the guard, and locking doesn't need a mutable self. So this is unnecessary, and the doc doesn't say why this was done. (Nor does the source.)

So I propose for this to be changed to &self. Looks like an oversight of some refactoring to me.
@sfackler
Copy link
Member

sfackler commented Jul 8, 2015

I imagine it's like that because the signature was copied off of the Read trait - changing it to take &self seems fine to me personally. cc @alexcrichton

@sfackler sfackler added A-io T-libs-api Relevant to the library API team, which will review and decide on the PR/issue. labels Jul 8, 2015
@alexcrichton
Copy link
Member

Yeah I think it'd be fine to switch to &self

@RalfJung
Copy link
Member Author

RalfJung commented Jul 8, 2015

Is it quicker for one of you to just do that, or should I submit a PR?

@sfackler
Copy link
Member

sfackler commented Jul 8, 2015

Go for it!

@RalfJung RalfJung mentioned this issue Jul 8, 2015
Merged
@RalfJung
Copy link
Member Author

RalfJung commented Jul 8, 2015

See #26897.

bors added a commit that referenced this issue Jul 31, 2015
@bors
Auto merge of #26897 - RalfJung:stdin-mut, r=alexcrichton
dc966ef 
This fixes #26890.

To be honest, the local compile-test is still running. This just takes so long. But this looks trivial enough...
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
T-libs-api Relevant to the library API team, which will review and decide on the PR/issue.
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

Stdin::read_line: read_line does not need a mutable borrow
3 participants
@alexcrichton @RalfJung @sfackler