Proposal to rebase #58 onto 3.5.0 for a new release

changelog.md

@@ -6,8 +6,8 @@ Next
 
 **Compatibility**:
 
- * Ensures compatibility with Rust 1.36.0 through 1.72.1. This bumps the minimum
-   supported Rust version from 1.16 to 1.36.
+ * Ensures compatibility with Rust 1.40.0 through 1.72.1. This bumps the minimum
+   supported Rust version from 1.16 to 1.40.
 
 3.5.0
 -----

rust-toolchain.toml

@@ -2,4 +2,4 @@
 # Note, this is the Minimum Supported Rust Version. We build with this by
 # default to ensure we don't accidentally break it, but of course you should be
 # able to build with more recent versions.
-channel = "1.36.0"
+channel = "1.40.0"

src/read.rs

@@ -14,7 +14,6 @@ use std::cmp;
 use std::fs;
 use std::io;
 use std::marker;
-use std::mem;
 use std::path;
 use super::{Error, Result, Sample, SampleFormat, WavSpec, WavSpecEx};
 
@@ -29,8 +28,8 @@ pub trait ReadExt: io::Read {
     //  TODO: There is an RFC proposing a method like this for the standard library.
     fn read_into(&mut self, buf: &mut [u8]) -> io::Result<()>;
 
-    /// Reads `n` bytes and returns them in a vector.
-    fn read_bytes(&mut self, n: usize) -> io::Result<Vec<u8>>;
+    /// Reads 4 bytes and returns them in an array.
+    fn read_4_bytes(&mut self) -> io::Result<[u8; 4]>;
 
     /// Skip over `n` bytes.
     fn skip_bytes(&mut self, n: usize) -> io::Result<()>;
@@ -110,13 +109,8 @@ impl<R> ReadExt for R
     }
 
     #[inline(always)]
-    fn read_bytes(&mut self, n: usize) -> io::Result<Vec<u8>> {
-        // We allocate a runtime fixed size buffer, and we are going to read
-        // into it, so zeroing or filling the buffer is a waste. This method
-        // is safe, because the contents of the buffer are only exposed when
-        // they have been overwritten completely by the read.
-        let mut buf = Vec::with_capacity(n);
-        unsafe { buf.set_len(n); }
+    fn read_4_bytes(&mut self) -> io::Result<[u8; 4]> {
+        let mut buf = [0_u8; 4];
         try!(self.read_into(&mut buf[..]));
         Ok(buf)
     }
@@ -192,8 +186,10 @@ impl<R> ReadExt for R
     }
 
     #[inline(always)]
-    fn read_le_f32(&mut self) -> io::Result<f32> {
-        self.read_le_u32().map(|u| unsafe { mem::transmute(u) })
+    fn read_le_f32(&mut self) -> io::Result<f32> {    
+        let mut buf = [0u8; 4];
+        try!(self.read_into(&mut buf));
+        Ok(f32::from_le_bytes(buf))
     }
 }
 
@@ -273,14 +269,14 @@ pub fn read_wave_header<R: io::Read>(reader: &mut R) -> Result<u64> {
     // into it is more cumbersome, but also avoids a heap allocation. Is
     // the compiler smart enough to avoid the heap allocation anyway? I
     // would not expect it to be.
-    if b"RIFF" != &try!(reader.read_bytes(4))[..] {
+    if b"RIFF" != &try!(reader.read_4_bytes())[..] {
         return Err(Error::FormatError("no RIFF tag found"));
     }
 
     let file_len = try!(reader.read_le_u32());
 
     // Next four bytes indicate the file type, which should be WAVE.
-    if b"WAVE" != &try!(reader.read_bytes(4))[..] {
+    if b"WAVE" != &try!(reader.read_4_bytes())[..] {
         return Err(Error::FormatError("no WAVE tag found"));
     }
 

src/write.rs

@@ -14,6 +14,7 @@ use std::fs;
 use std::io;
 use std::mem;
 use std::io::{Seek, Write};
+use std::mem::MaybeUninit;
 use std::path;
 use super::{Error, Result, Sample, SampleFormat, WavSpec, WavSpecEx};
 use ::read;
@@ -114,7 +115,7 @@ impl<W> WriteExt for W
 
     #[inline(always)]
     fn write_le_f32(&mut self, x: f32) -> io::Result<()> {
-        let u = unsafe { mem::transmute(x) };
+        let u = unsafe { mem::transmute::<f32, u32>(x) };
         self.write_le_u32(u)
     }
 }
@@ -178,7 +179,7 @@ pub struct WavWriter<W>
 
     /// The buffer for the sample writer, which is recycled throughout calls to
     /// avoid allocating frequently.
-    sample_writer_buffer: Vec<u8>,
+    sample_writer_buffer: Vec<MaybeUninit<u8>>,
 
     /// The offset of the length field of the data chunk.
     ///
@@ -466,7 +467,7 @@ impl<W> WavWriter<W>
             // We need a bigger buffer. There is no point in growing the old
             // one, as we are going to overwrite the samples anyway, so just
             // allocate a new one.
-            let mut new_buffer = Vec::with_capacity(num_bytes);
+            let mut new_buffer = Vec::<MaybeUninit<u8>>::with_capacity(num_bytes);
 
             // The potentially garbage memory here will not be exposed: the
             // buffer is only exposed when flushing, but `flush()` asserts that
@@ -737,7 +738,7 @@ pub struct SampleWriter16<'parent, W> where W: io::Write + io::Seek + 'parent {
     writer: &'parent mut W,
 
     /// The internal buffer that samples are written to before they are flushed.
-    buffer: &'parent mut [u8],
+    buffer: &'parent mut [MaybeUninit<u8>],
 
     /// Reference to the `data_bytes_written` field of the writer.
     data_bytes_written: &'parent mut u32,
@@ -762,33 +763,20 @@ impl<'parent, W: io::Write + io::Seek> SampleWriter16<'parent, W> {
     /// Note that nothing is actually written until `flush()` is called.
     #[inline(always)]
     pub fn write_sample<S: Sample>(&mut self, sample: S) {
-        assert!((self.index as usize) <= self.buffer.len() - 2,
+        assert!((self.index as usize) + 2 <= self.buffer.len(),
           "Trying to write more samples than reserved for the sample writer.");
 
-        let s = sample.as_i16() as u16;
-
-        // Write the sample in little endian to the buffer.
-        self.buffer[self.index as usize] = s as u8;
-        self.buffer[self.index as usize + 1] = (s >> 8) as u8;
-
-        self.index += 2;
+        // SAFETY: We performed the bounds check in the above assertion.
+        unsafe { self.write_sample_unchecked(sample) };
     }
 
-    #[cfg(target_arch = "x86_64")]
     unsafe fn write_u16_le_unchecked(&mut self, value: u16) {
-        // x86_64 is little endian, so we do not need to shuffle bytes around;
-        // we can just store the 16-bit integer in the buffer directly.
-        let ptr: *mut u16 = mem::transmute(self.buffer.get_unchecked_mut(self.index as usize));
-        *ptr = value;
-    }
-
-    #[cfg(not(target_arch = "x86_64"))]
-    unsafe fn write_u16_le_unchecked(&mut self, value: u16) {
-        // Write a sample in little-endian to the buffer, independent of the
-        // endianness of the architecture we are running on.
-        let idx = self.index as usize;
-        *self.buffer.get_unchecked_mut(idx) = value as u8;
-        *self.buffer.get_unchecked_mut(idx + 1) = (value >> 8) as u8;
+        // On little endian machines the compiler produces assembly code
+        // that merges the following two lines into a single instruction.
+        *self.buffer.get_unchecked_mut(self.index as usize) = MaybeUninit::new(value as u8);
+        self.buffer.get_unchecked_mut(self.index as usize).assume_init();
+        *self.buffer.get_unchecked_mut(self.index as usize + 1) = MaybeUninit::new((value >> 8) as u8);
+        self.buffer.get_unchecked_mut(self.index as usize + 1).assume_init();
     }
 
     /// Like `write_sample()`, but does not perform a bounds check when writing
@@ -813,7 +801,15 @@ impl<'parent, W: io::Write + io::Seek> SampleWriter16<'parent, W> {
             panic!("Insufficient samples written to the sample writer.");
         }
 
-        try!(self.writer.write_all(&self.buffer));
+        // SAFETY: casting `self.buffer` to a `*const [MaybeUninit<u8>]` is safe since the caller guarantees that
+        // `self.buffer` is initialized, and `MaybeUninit<u8>` is guaranteed to have the same layout as `u8`.
+        // The pointer obtained is valid since it refers to memory owned by `self.buffer` which is a
+        // reference and thus guaranteed to be valid for reads.
+        // This is copied from the nightly implementation for slice_assume_init_ref.
+        let slice = unsafe { &*(self.buffer as *const [MaybeUninit<u8>] as *const [u8]) };
+
+        try!(self.writer.write_all(slice));
+
         *self.data_bytes_written += self.buffer.len() as u32;
         Ok(())
     }