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

Implement BufReader #8934

Closed
wants to merge 1 commit into from
Closed

Implement BufReader #8934

wants to merge 1 commit into from

Conversation

sfackler
Copy link
Member

@sfackler sfackler commented Sep 2, 2013

No description provided.

@alexcrichton
Copy link
Member

Hmm, I'm actually curious why we have BufReader and MemReader. It seems like you would never actually need to have MemReader, and if we did it looks like it should be implemented on top of BufReader.

That being said, even with both MemWriter and BufWriter, I'm not entirely sure why one would want BufWriter...

Anyway, I'm curious what @brson has to say about the status of these apis. It seems to me that MemReader shouldn't exist, and perhaps one of the writers also shouldn't exist...

@sfackler
Copy link
Member Author

sfackler commented Sep 3, 2013

Since MemReader owns its buffer it can be passed around a lot more easily than BufReader. I'd like to implement MemReader in terms of BufReader but it's not clear to me that it's possible. I don't think an arrangement like

struct MemReader<'self> {
    buf: ~[u8],
    reader: MemReader<'self>
}

is possible since it can't (I think) be constructed such that the lifetimes work out. After construction, it couldn't be moved. We could use a macro to create the implementations.

I agree that BufWriter seems useless.

The other weird thing in here is that everything implements Seek but the seek impl fails. Would it make sense to split Seek in to separate Seek and Tell traits?

@alexcrichton
Copy link
Member

Oh well that's unfortunate, I hadn't thought of that. I actually suppose that with destructors it may not even be sound to have something like that. Regardless, it looks to be that the code is very similar between MemReader and BufReader, so could they be implemented in a helper method taking a &[u8]?

I don't really see why we can't implement seek on these buffers. It should be pretty simple, right? I think that it's currently failing just because it's not implemented yet.

@sfackler
Copy link
Member Author

sfackler commented Sep 3, 2013

Now that I think about it, BufWriter could be useful if you're trying to avoid repeatedly allocating.

@alexcrichton
Copy link
Member

I suppose they do all have their uses. Code duplication is sad though :(

@brson
Copy link
Contributor

brson commented Sep 3, 2013

I don't have a particularly strong opinion on how these should be designed, as long as all the use cases are covered. seek is easy to implement, just needs to be done.

@brson
Copy link
Contributor

brson commented Sep 3, 2013

Thanks for the I/O improvements.

bors added a commit that referenced this pull request Sep 3, 2013
@bors bors closed this Sep 3, 2013
flip1995 pushed a commit to flip1995/rust that referenced this pull request Jun 4, 2022
add [`as_underscore`] lint

closes rust-lang#8847

detect usage of `as _` and enforce the usage of explicit type like
```rust
fn foo(n: usize) {}
let n: u16 = 256;
foo(n as _);
```
will suggest to change to
```rust
fn foo(n: usize) {}
let n: u16 = 256;
foo(n as usize);
```

changelog: add [`as_underscore`] lint
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 this pull request may close these issues.

4 participants