Also move read-LEB to using iterators

Author: scottmcm

compiler/rustc_serialize/src/leb128.rs

@@ -49,25 +49,25 @@ impl_write_unsigned_leb128!(write_usize_leb128, usize);
 
 macro_rules! impl_read_unsigned_leb128 {
     ($fn_name:ident, $int_ty:ty) => {
+        // This returns `Option` to avoid needing to emit the panic paths here.
+        // Letting the caller do it instead helps keep our code size small.
         #[inline]
-        pub fn $fn_name(slice: &[u8], position: &mut usize) -> $int_ty {
+        pub fn $fn_name(slice: &mut std::slice::Iter<'_, u8>) -> Option<$int_ty> {
             // The first iteration of this loop is unpeeled. This is a
             // performance win because this code is hot and integer values less
             // than 128 are very common, typically occurring 50-80% or more of
             // the time, even for u64 and u128.
-            let byte = slice[*position];
-            *position += 1;
+            let byte = *(slice.next()?);
             if (byte & 0x80) == 0 {
-                return byte as $int_ty;
+                return Some(byte as $int_ty);
             }
             let mut result = (byte & 0x7F) as $int_ty;
             let mut shift = 7;
             loop {
-                let byte = slice[*position];
-                *position += 1;
+                let byte = *(slice.next()?);
                 if (byte & 0x80) == 0 {
                     result |= (byte as $int_ty) << shift;
-                    return result;
+                    return Some(result);
                 } else {
                     result |= ((byte & 0x7F) as $int_ty) << shift;
                 }
@@ -126,15 +126,16 @@ impl_write_signed_leb128!(write_isize_leb128, isize);
 
 macro_rules! impl_read_signed_leb128 {
     ($fn_name:ident, $int_ty:ty) => {
+        // This returns `Option` to avoid needing to emit the panic paths here.
+        // Letting the caller do it instead helps keep our code size small.
         #[inline]
-        pub fn $fn_name(slice: &[u8], position: &mut usize) -> $int_ty {
+        pub fn $fn_name(slice: &mut std::slice::Iter<'_, u8>) -> Option<$int_ty> {
             let mut result = 0;
             let mut shift = 0;
             let mut byte;
 
             loop {
-                byte = slice[*position];
-                *position += 1;
+                byte = *(slice.next()?);
                 result |= <$int_ty>::from(byte & 0x7F) << shift;
                 shift += 7;
 
@@ -148,7 +149,7 @@ macro_rules! impl_read_signed_leb128 {
                 result |= (!0 << shift);
             }
 
-            result
+            Some(result)
         }
     };
 }

compiler/rustc_serialize/src/opaque.rs

@@ -538,7 +538,7 @@ pub struct MemDecoder<'a> {
     // Previously this type stored `position: usize`, but because it's staying
     // safe code, that meant that reading `n` bytes meant a bounds check both
     // for `position + n` *and* `position`, since there's nothing saying that
-    // the additions didn't wrap.  Storing an iterator like this instead means
+    // the additions didn't wrap. Storing an iterator like this instead means
     // there's no offsetting needed to get to the data, and the iterator instead
     // of a slice means only increasing the start pointer on reads, rather than
     // also needing to decrease the count in a slice.
@@ -579,10 +579,11 @@ impl<'a> MemDecoder<'a> {
 
 macro_rules! read_leb128 {
     ($dec:expr, $fun:ident) => {{
-        let mut position = 0_usize;
-        let val = leb128::$fun($dec.reader.as_slice(), &mut position);
-        let _ = $dec.reader.advance_by(position);
-        val
+        if let Some(val) = leb128::$fun(&mut $dec.reader) {
+            val
+        } else {
+            $dec.panic_insufficient_data()
+        }
     }};
 }
 
@@ -685,9 +686,7 @@ impl<'a> Decoder for MemDecoder<'a> {
         let slice = self.reader.as_slice();
         assert!(slice[len] == STR_SENTINEL);
         self.reader.advance_by(len + 1).unwrap();
-        unsafe {
-            std::str::from_utf8_unchecked(&slice[..len])
-        }
+        unsafe { std::str::from_utf8_unchecked(&slice[..len]) }
     }
 
     #[inline]

compiler/rustc_serialize/tests/leb128.rs

@@ -28,12 +28,12 @@ macro_rules! impl_test_unsigned_leb128 {
                 stream.extend($write_fn_name(&mut buf, x));
             }
 
-            let mut position = 0;
+            let mut reader = stream.iter();
             for &expected in &values {
-                let actual = $read_fn_name(&stream, &mut position);
+                let actual = $read_fn_name(&mut reader).unwrap();
                 assert_eq!(expected, actual);
             }
-            assert_eq!(stream.len(), position);
+            assert_eq!(reader.len(), 0);
         }
     };
 }
@@ -74,12 +74,12 @@ macro_rules! impl_test_signed_leb128 {
                 stream.extend($write_fn_name(&mut buf, x));
             }
 
-            let mut position = 0;
+            let mut reader = stream.iter();
             for &expected in &values {
-                let actual = $read_fn_name(&stream, &mut position);
+                let actual = $read_fn_name(&mut reader).unwrap();
                 assert_eq!(expected, actual);
             }
-            assert_eq!(stream.len(), position);
+            assert_eq!(reader.len(), 0);
         }
     };
 }