Auto merge of #83465 - michaelwoerister:safe-read_raw_bytes, r=cjgillot

Allow for reading raw bytes from rustc_serialize::Decoder without unsafe code

The current `read_raw_bytes` method requires using `MaybeUninit` and `unsafe`. I don't think this is necessary. Let's see if a safe interface has any performance drawbacks.

This is a followup to #83273 and will make it easier to rebase #82183.

r? `@cjgillot`

Author: bors

compiler/rustc_data_structures/src/fingerprint.rs

@@ -1,7 +1,7 @@
 use crate::stable_hasher;
 use rustc_serialize::{Decodable, Encodable};
+use std::convert::TryInto;
 use std::hash::{Hash, Hasher};
-use std::mem::{self, MaybeUninit};
 
 #[derive(Eq, PartialEq, Ord, PartialOrd, Debug, Clone, Copy)]
 #[repr(C)]
@@ -60,6 +60,29 @@ impl Fingerprint {
     pub fn to_hex(&self) -> String {
         format!("{:x}{:x}", self.0, self.1)
     }
+
+    #[inline]
+    pub fn to_le_bytes(&self) -> [u8; 16] {
+        // This seems to optimize to the same machine code as
+        // `unsafe { mem::transmute(*k) }`. Well done, LLVM! :)
+        let mut result = [0u8; 16];
+
+        let first_half: &mut [u8; 8] = (&mut result[0..8]).try_into().unwrap();
+        *first_half = self.0.to_le_bytes();
+
+        let second_half: &mut [u8; 8] = (&mut result[8..16]).try_into().unwrap();
+        *second_half = self.1.to_le_bytes();
+
+        result
+    }
+
+    #[inline]
+    pub fn from_le_bytes(bytes: [u8; 16]) -> Fingerprint {
+        Fingerprint(
+            u64::from_le_bytes(bytes[0..8].try_into().unwrap()),
+            u64::from_le_bytes(bytes[8..16].try_into().unwrap()),
+        )
+    }
 }
 
 impl std::fmt::Display for Fingerprint {
@@ -119,20 +142,17 @@ impl_stable_hash_via_hash!(Fingerprint);
 impl<E: rustc_serialize::Encoder> Encodable<E> for Fingerprint {
     #[inline]
     fn encode(&self, s: &mut E) -> Result<(), E::Error> {
-        let bytes: [u8; 16] = unsafe { mem::transmute([self.0.to_le(), self.1.to_le()]) };
-        s.emit_raw_bytes(&bytes)?;
+        s.emit_raw_bytes(&self.to_le_bytes()[..])?;
         Ok(())
     }
 }
 
 impl<D: rustc_serialize::Decoder> Decodable<D> for Fingerprint {
     #[inline]
     fn decode(d: &mut D) -> Result<Self, D::Error> {
-        let mut bytes: [MaybeUninit<u8>; 16] = MaybeUninit::uninit_array();
-        d.read_raw_bytes(&mut bytes)?;
-
-        let [l, r]: [u64; 2] = unsafe { mem::transmute(bytes) };
-        Ok(Fingerprint(u64::from_le(l), u64::from_le(r)))
+        let mut bytes = [0u8; 16];
+        d.read_raw_bytes_into(&mut bytes[..])?;
+        Ok(Fingerprint::from_le_bytes(bytes))
     }
 }
 

compiler/rustc_middle/src/ty/codec.rs

@@ -473,8 +473,8 @@ macro_rules! implement_ty_decoder {
                 }
 
                 #[inline]
-                fn read_raw_bytes(&mut self, bytes: &mut [std::mem::MaybeUninit<u8>]) -> Result<(), Self::Error> {
-                    self.opaque.read_raw_bytes(bytes)
+                fn read_raw_bytes_into(&mut self, bytes: &mut [u8]) -> Result<(), Self::Error> {
+                    self.opaque.read_raw_bytes_into(bytes)
                 }
 
                 fn error(&mut self, err: &str) -> Self::Error {

compiler/rustc_serialize/src/json.rs

@@ -188,7 +188,6 @@ use std::collections::{BTreeMap, HashMap};
 use std::io;
 use std::io::prelude::*;
 use std::mem::swap;
-use std::mem::MaybeUninit;
 use std::num::FpCategory as Fp;
 use std::ops::Index;
 use std::str::FromStr;
@@ -2367,10 +2366,9 @@ impl crate::Decoder for Decoder {
         expect!(self.pop(), String).map(Cow::Owned)
     }
 
-    fn read_raw_bytes(&mut self, s: &mut [MaybeUninit<u8>]) -> Result<(), Self::Error> {
+    fn read_raw_bytes_into(&mut self, s: &mut [u8]) -> Result<(), Self::Error> {
         for c in s.iter_mut() {
-            let h = self.read_u8()?;
-            unsafe { *c.as_mut_ptr() = h };
+            *c = self.read_u8()?;
         }
         Ok(())
     }

compiler/rustc_serialize/src/opaque.rs

@@ -1,6 +1,7 @@
 use crate::leb128::{self, max_leb128_len};
-use crate::serialize::{self, Decoder as _, Encoder as _};
+use crate::serialize::{self, Encoder as _};
 use std::borrow::Cow;
+use std::convert::TryInto;
 use std::fs::File;
 use std::io::{self, Write};
 use std::mem::MaybeUninit;
@@ -539,6 +540,13 @@ impl<'a> Decoder<'a> {
     pub fn advance(&mut self, bytes: usize) {
         self.position += bytes;
     }
+
+    #[inline]
+    pub fn read_raw_bytes(&mut self, bytes: usize) -> &'a [u8] {
+        let start = self.position;
+        self.position += bytes;
+        &self.data[start..self.position]
+    }
 }
 
 macro_rules! read_leb128 {
@@ -659,22 +667,10 @@ impl<'a> serialize::Decoder for Decoder<'a> {
     }
 
     #[inline]
-    fn read_raw_bytes(&mut self, s: &mut [MaybeUninit<u8>]) -> Result<(), String> {
+    fn read_raw_bytes_into(&mut self, s: &mut [u8]) -> Result<(), String> {
         let start = self.position;
-        let end = start + s.len();
-        assert!(end <= self.data.len());
-
-        // SAFETY: Both `src` and `dst` point to at least `s.len()` elements:
-        // `src` points to at least `s.len()` elements by above assert, and
-        // `dst` points to `s.len()` elements by derivation from `s`.
-        unsafe {
-            let src = self.data.as_ptr().add(start);
-            let dst = s.as_mut_ptr() as *mut u8;
-            ptr::copy_nonoverlapping(src, dst, s.len());
-        }
-
-        self.position = end;
-
+        self.position += s.len();
+        s.copy_from_slice(&self.data[start..self.position]);
         Ok(())
     }
 }
@@ -705,16 +701,7 @@ impl serialize::Encodable<FileEncoder> for [u8] {
 impl<'a> serialize::Decodable<Decoder<'a>> for Vec<u8> {
     fn decode(d: &mut Decoder<'a>) -> Result<Self, String> {
         let len = serialize::Decoder::read_usize(d)?;
-
-        let mut v = Vec::with_capacity(len);
-        let buf = &mut v.spare_capacity_mut()[..len];
-        d.read_raw_bytes(buf)?;
-
-        unsafe {
-            v.set_len(len);
-        }
-
-        Ok(v)
+        Ok(d.read_raw_bytes(len).to_owned())
     }
 }
 
@@ -750,13 +737,12 @@ impl serialize::Encodable<FileEncoder> for IntEncodedWithFixedSize {
 impl<'a> serialize::Decodable<Decoder<'a>> for IntEncodedWithFixedSize {
     #[inline]
     fn decode(decoder: &mut Decoder<'a>) -> Result<IntEncodedWithFixedSize, String> {
-        let mut bytes = MaybeUninit::uninit_array();
         let _start_pos = decoder.position();
-        decoder.read_raw_bytes(&mut bytes)?;
+        let bytes = decoder.read_raw_bytes(IntEncodedWithFixedSize::ENCODED_SIZE);
         let _end_pos = decoder.position();
         debug_assert_eq!((_end_pos - _start_pos), IntEncodedWithFixedSize::ENCODED_SIZE);
 
-        let value = u64::from_le_bytes(unsafe { MaybeUninit::array_assume_init(bytes) });
+        let value = u64::from_le_bytes(bytes.try_into().unwrap());
         Ok(IntEncodedWithFixedSize(value))
     }
 }

compiler/rustc_serialize/src/serialize.rs

@@ -7,7 +7,6 @@ Core encoding and decoding interfaces.
 use std::borrow::Cow;
 use std::cell::{Cell, RefCell};
 use std::marker::PhantomData;
-use std::mem::MaybeUninit;
 use std::path;
 use std::rc::Rc;
 use std::sync::Arc;
@@ -226,7 +225,7 @@ pub trait Decoder {
     fn read_f32(&mut self) -> Result<f32, Self::Error>;
     fn read_char(&mut self) -> Result<char, Self::Error>;
     fn read_str(&mut self) -> Result<Cow<'_, str>, Self::Error>;
-    fn read_raw_bytes(&mut self, s: &mut [MaybeUninit<u8>]) -> Result<(), Self::Error>;
+    fn read_raw_bytes_into(&mut self, s: &mut [u8]) -> Result<(), Self::Error>;
 
     // Compound types:
     #[inline]