Make std::io::Buffer object-safe.

[ricky26]

This moves chars() and lines() out of Buffer and into separate traits (CharsBuffer and LinesBuffer respectively) - this matches the pattern used for bytes() on Reader (with BytesReader).

(I came across this when I wanted a trait object of a Buffer, so that I could use read_line(); rustc errors about std::io::Buffer not being object-safe.)

[breaking-change]
Any uses of Buffer::lines() will need to use the new trait std::io::LinesBuffer.
The same is true for Buffer::chars() with std::io::CharsBuffer.

[rust-highfive]

Thanks for the pull request, and welcome! The Rust team is excited to review your changes, and you should hear from @huonw (or someone else) soon.

[sfackler]

cc @aturon

[huonw]

Could you add a test/function to ensure that a Buffer trait object can be created, e.g. add the following to the tests module:

fn _ensure_buffer_is_object_safe<T: Buffer>(x: &T) -> &Buffer {
    x as &Buffer
}

(The leading _ is important, to ensure that the compiler linting doesn't warn about it being unused.)

[huonw]

I also wonder if the two extra traits could be collapsed into a single trait (don't know a good name off the top of my head). Also, the Buffer trait is in the prelude, so maybe the new ones should be too? (@aturon would know best.)

[ricky26]

I did consider making them a single trait (though I also couldn't think of a good name), in the end I broke them up - in fairness perhaps CharsBuffer is sufficient, the lines are composed of chars so the one behaviour implies the other?

And although Reader and Buffer are in the prelude, it looks like BytesReader (for example) is not. Though I would agree that having lines() be available in the prelude would be nice.

I'll drop the test in and merge the traits.

[aturon]

@huonw I've recently proposed a convention for extension traits that would cover a combined trait. It's not the best but it gets the job done. But I do believe that in this case, having a combined extension trait is the right thing to do (given that the only role is to provide a blanket impl for T: Buffer).

I think it's fine to put the new trait(s) in the prelude as well (and BytesReader probably should have been.) We'll be more thoroughly evaluating criteria for prelude inclusion later this month.

Otherwise, @huonw feel free to r+ when the above and your concerns have been addressed.

Thanks for the PR!

[ricky26]

Okay - I've merged the traits and added the new trait to the prelude, do we want to rename this trait now (to BufferExt)?

[aturon]

@ricky26 Since this is going in the prelude, can you call it BufferPrelude? (there's a bit of an overlap in conventions here, but I'd say the Prelude suffix trumps.)

[ricky26]

Okay @aturon, it's now BufferPrelude.

[ricky26]

@huonw is this okay to merge now?

[aturon]

@ricky26 Sorry for the delay in getting this approved, I've sent it to bors. In general feel free to ping any of us on IRC if you're not getting a response; the rust devs have a very high email volume.