Implement BufRead for VecDeque<u8>

[a1phyr]

Note: it would become insta-stable

[rustbot]

r? @Mark-Simulacrum

(rustbot has picked a reviewer for you, use r? to override)

[rustbot]

Hey! It looks like you've submitted a new PR for the library teams!

If this PR contains changes to any rust-lang/rust public library APIs then please comment with @rustbot label +T-libs-api -T-libs to tag it appropriately. If this PR contains changes to any unstable APIs please edit the PR description to add a link to the relevant API Change Proposal or create one if you haven't already. If you're unsure where your change falls no worries, just leave it as is and the reviewer will take a look and make a decision to forward on if necessary.

Examples of T-libs-api changes:

• Stabilizing library features
• Introducing insta-stable changes such as new implementations of existing stable traits on existing stable types
• Introducing new or changing existing unstable library APIs (excluding permanently unstable features / features without a tracking issue)
• Changing public documentation in ways that create new stability guarantees
• Changing observable runtime behavior of library APIs

[Mark-Simulacrum]

r? @joshtriplett for libs-api FCP

[workingjubilee]

r? libs-api

Need FCP for impl BufRead on VecDeque<u8>. It's needed for VecDeque<u8> instead of using the one for &[u8] because unlike Vec<u8>, the VecDeque can't simply deref to &[u8] (due to being internally disjoint).

[dtolnay]

r? dtolnay

[dtolnay]

This is great!

[dtolnay]

@rust-lang/libs-api:
@rfcbot fcp merge

Note that there is no impl BufRead for Vec<u8>. It wouldn't make sense -- .consume(n) could not be implemented efficiently. But for VecDeque<u8> it can!

For Vec<u8>, one must use a Cursor instead. That impl looks like this:

impl<T> BufRead for Cursor<T>
where
    T: AsRef<[u8]>

With or without the impl proposed by this PR, a Cursor does not work for VecDeque<u8> because it cannot be viewed as a single contiguous &[u8].

An alternative worth considering instead of this PR would be how to generalize that Cursor impl so that it works regardless of whether the &[u8] covers the whole buffer, or only a prefix of it.

pub trait PrefixBytes {
    fn prefix_bytes(&self) -> &[u8];
}

impl PrefixBytes for VecDeque<u8> {
    fn prefix_bytes(&self) -> &[u8] {
        let (front, _) = self.as_slices();
        front
    }
}

impl<T> PrefixBytes for T
where
    T: AsRef<[u8]>,
{
    fn prefix_bytes(&self) -> &[u8] {
        self.as_ref()
    }
}

impl<T> BufRead for Cursor<T>
where
    T: PrefixBytes  // instead of AsRef<[u8]>
...

[rfcbot]

Team member @dtolnay has proposed to merge this. The next step is review by the rest of the tagged team members:

• @Amanieu
• @BurntSushi
• @dtolnay
• @joshtriplett
• @m-ou-se

No concerns currently listed.

Once a majority of reviewers approve (and at most 2 approvals are outstanding), this will enter its final comment period. If you spot a major issue that hasn't been raised at any point in this process, please speak up!

See this document for info about what commands tagged team members can give me.

[rfcbot]

🔔 This is now entering its final comment period, as per the review above. 🔔

[rfcbot]

The final comment period, with a disposition to merge, as per the review above, is now complete.

As the automated representative of the governance process, I would like to thank the author for their work and everyone else who contributed.

This will be merged soon.

[dtolnay]

Thank you!

[dtolnay]

@bors r+

[bors]

📌 Commit a9ece12 has been approved by dtolnay

It is now in the queue for this repository.

[bors]

⌛ Testing commit a9ece12 with merge f70779b...

[bors]

☀️ Test successful - checks-actions
Approved by: dtolnay
Pushing f70779b to master...

[rust-timer]

Finished benchmarking commit (f70779b): comparison URL.

Overall result: no relevant changes - no action needed

@rustbot label: -perf-regression

Instruction count

This benchmark run did not return any relevant results for this metric.

Max RSS (memory usage)

This benchmark run did not return any relevant results for this metric.

Cycles

This benchmark run did not return any relevant results for this metric.

Binary size

This benchmark run did not return any relevant results for this metric.

Bootstrap: 625.359s -> 625.093s (-0.04%)
Artifact size: 305.52 MiB -> 305.52 MiB (-0.00%)

[wiseaidev]

Hello friends. While reviewing this feature, I realized that we can the accomplish the same using the as_slices. Here is an example using as_slices:

use std::collections::VecDeque;

fn main() {
    let mut vec_deque: VecDeque<u8> = VecDeque::new();
    vec_deque.push_back(1);
    vec_deque.push_back(2);
    vec_deque.push_back(3);

    let (buffer, _) = vec_deque.as_slices();
    println!("Buffer: {:?}", buffer);

    vec_deque.drain(..2);

    let (new_buffer, _) = vec_deque.as_slices();
    println!("New Buffer: {:?}", new_buffer);
}

This would accomplish the same as BufRead:

use std::collections::VecDeque;
use std::io::BufRead;

fn main() {
    let mut vec_deque: VecDeque<u8> = VecDeque::new();

    vec_deque.push_back(1);
    vec_deque.push_back(2);
    vec_deque.push_back(3);

    let buf_reader: &mut dyn BufRead = &mut vec_deque;

    let buffer = buf_reader.fill_buf().expect("Error reading buffer");

    println!("Buffer: {:?}", buffer);

    buf_reader.consume(2);

    let new_buffer = buf_reader.fill_buf().expect("Error reading buffer after consume");

    println!("New Buffer: {:?}", new_buffer);
}

So, I am wondering when this feature could be useful. Thanks!

Edit: I just read the documentation:

    /// Returns the contents of the "front" slice as returned by
    /// [`as_slices`][`VecDeque::as_slices`]. If the contained byte slices of the `VecDeque` are
    /// discontiguous, multiple calls to `fill_buf` will be needed to read the entire content.

Now I see what's the difference. Thanks