Patchwork [07,of,11] rust: simply use TryInto to convert slice to array

login
register
mail settings
Submitter Yuya Nishihara
Date Aug. 17, 2019, 12:12 p.m.
Message ID <d496a0a5d5f8ff45afde.1566043928@mimosa>
Download mbox | patch
Permalink /patch/41329/
State Accepted
Headers show

Comments

Yuya Nishihara - Aug. 17, 2019, 12:12 p.m.
# HG changeset patch
# User Yuya Nishihara <yuya@tcha.org>
# Date 1566009462 -32400
#      Sat Aug 17 11:37:42 2019 +0900
# Node ID d496a0a5d5f8ff45afde5a0a90b0917e507cbbbb
# Parent  a57c56ff9cec59f12780c83f5dc2ab679d2523e6
rust: simply use TryInto to convert slice to array

Since our rust module depends on TryInto, there's no point to avoid using it.

While rewriting copy_into_array(), I noticed CPython interface doesn't check
the length of the p1/p2 values, which is marked as TODO.
Raphaël Gomès - Aug. 21, 2019, 1:31 p.m.
Indeed. I think I wrote this function before TryInto was on Debian 
stable's Rust version.

On 8/17/19 2:12 PM, Yuya Nishihara wrote:

> # HG changeset patch
> # User Yuya Nishihara <yuya@tcha.org>
> # Date 1566009462 -32400
> #      Sat Aug 17 11:37:42 2019 +0900
> # Node ID d496a0a5d5f8ff45afde5a0a90b0917e507cbbbb
> # Parent  a57c56ff9cec59f12780c83f5dc2ab679d2523e6
> rust: simply use TryInto to convert slice to array
>
> Since our rust module depends on TryInto, there's no point to avoid using it.
>
> While rewriting copy_into_array(), I noticed CPython interface doesn't check
> the length of the p1/p2 values, which is marked as TODO.
>
> diff --git a/rust/hg-core/src/dirstate/dirstate_map.rs b/rust/hg-core/src/dirstate/dirstate_map.rs
> --- a/rust/hg-core/src/dirstate/dirstate_map.rs
> +++ b/rust/hg-core/src/dirstate/dirstate_map.rs
> @@ -7,13 +7,13 @@
>   
>   use crate::{
>       dirstate::{parsers::PARENT_SIZE, EntryState},
> -    pack_dirstate, parse_dirstate,
> -    utils::copy_into_array,
> -    CopyMap, DirsIterable, DirsMultiset, DirstateEntry, DirstateError,
> -    DirstateMapError, DirstateParents, DirstateParseError, StateMap,
> +    pack_dirstate, parse_dirstate, CopyMap, DirsIterable, DirsMultiset,
> +    DirstateEntry, DirstateError, DirstateMapError, DirstateParents,
> +    DirstateParseError, StateMap,
>   };
>   use core::borrow::Borrow;
>   use std::collections::{HashMap, HashSet};
> +use std::convert::TryInto;
>   use std::iter::FromIterator;
>   use std::ops::Deref;
>   use std::time::Duration;
> @@ -260,10 +260,10 @@ impl DirstateMap {
>           let parents;
>           if file_contents.len() == PARENT_SIZE * 2 {
>               parents = DirstateParents {
> -                p1: copy_into_array(&file_contents[..PARENT_SIZE]),
> -                p2: copy_into_array(
> -                    &file_contents[PARENT_SIZE..PARENT_SIZE * 2],
> -                ),
> +                p1: file_contents[..PARENT_SIZE].try_into().unwrap(),
> +                p2: file_contents[PARENT_SIZE..PARENT_SIZE * 2]
> +                    .try_into()
> +                    .unwrap(),
>               };
>           } else if file_contents.is_empty() {
>               parents = DirstateParents {
> diff --git a/rust/hg-core/src/dirstate/parsers.rs b/rust/hg-core/src/dirstate/parsers.rs
> --- a/rust/hg-core/src/dirstate/parsers.rs
> +++ b/rust/hg-core/src/dirstate/parsers.rs
> @@ -5,7 +5,6 @@
>   
>   use crate::{
>       dirstate::{CopyMap, EntryState, StateMap},
> -    utils::copy_into_array,
>       DirstateEntry, DirstatePackError, DirstateParents, DirstateParseError,
>   };
>   use byteorder::{BigEndian, ReadBytesExt, WriteBytesExt};
> @@ -31,8 +30,8 @@ pub fn parse_dirstate(
>   
>       let mut curr_pos = PARENT_SIZE * 2;
>       let parents = DirstateParents {
> -        p1: copy_into_array(&contents[..PARENT_SIZE]),
> -        p2: copy_into_array(&contents[PARENT_SIZE..curr_pos]),
> +        p1: contents[..PARENT_SIZE].try_into().unwrap(),
> +        p2: contents[PARENT_SIZE..curr_pos].try_into().unwrap(),
>       };
>   
>       while curr_pos < contents.len() {
> diff --git a/rust/hg-core/src/utils.rs b/rust/hg-core/src/utils.rs
> --- a/rust/hg-core/src/utils.rs
> +++ b/rust/hg-core/src/utils.rs
> @@ -9,23 +9,6 @@
>   
>   pub mod files;
>   
> -use std::convert::AsMut;
> -
> -/// Takes a slice and copies it into an array.
> -///
> -/// # Panics
> -///
> -/// Will panic if the slice and target array don't have the same length.
> -pub fn copy_into_array<A, T>(slice: &[T]) -> A
> -where
> -    A: Sized + Default + AsMut<[T]>,
> -    T: Copy,
> -{
> -    let mut a = Default::default();
> -    <A as AsMut<[T]>>::as_mut(&mut a).copy_from_slice(slice);
> -    a
> -}
> -
>   /// Replaces the `from` slice with the `to` slice inside the `buf` slice.
>   ///
>   /// # Examples
> diff --git a/rust/hg-cpython/src/dirstate/dirstate_map.rs b/rust/hg-cpython/src/dirstate/dirstate_map.rs
> --- a/rust/hg-cpython/src/dirstate/dirstate_map.rs
> +++ b/rust/hg-cpython/src/dirstate/dirstate_map.rs
> @@ -24,7 +24,7 @@ use crate::{
>       ref_sharing::PySharedState,
>   };
>   use hg::{
> -    utils::copy_into_array, DirsIterable, DirsMultiset, DirstateEntry,
> +    DirsIterable, DirsMultiset, DirstateEntry,
>       DirstateMap as RustDirstateMap, DirstateParents, DirstateParseError,
>       EntryState,
>   };
> @@ -239,8 +239,9 @@ py_class!(pub class DirstateMap |py| {
>       }
>   
>       def setparents(&self, p1: PyObject, p2: PyObject) -> PyResult<PyObject> {
> -        let p1 = copy_into_array(p1.extract::<PyBytes>(py)?.data(py));
> -        let p2 = copy_into_array(p2.extract::<PyBytes>(py)?.data(py));
> +        // TODO: don't panic; raise Python exception instead.
> +        let p1 = p1.extract::<PyBytes>(py)?.data(py).try_into().unwrap();
> +        let p2 = p2.extract::<PyBytes>(py)?.data(py).try_into().unwrap();
>   
>           self.inner(py)
>               .borrow_mut()
> @@ -274,8 +275,9 @@ py_class!(pub class DirstateMap |py| {
>       ) -> PyResult<PyBytes> {
>           let now = Duration::new(now.extract(py)?, 0);
>           let parents = DirstateParents {
> -            p1: copy_into_array(p1.extract::<PyBytes>(py)?.data(py)),
> -            p2: copy_into_array(p2.extract::<PyBytes>(py)?.data(py)),
> +            // TODO: don't panic; raise Python exception instead.
> +            p1: p1.extract::<PyBytes>(py)?.data(py).try_into().unwrap(),
> +            p2: p2.extract::<PyBytes>(py)?.data(py).try_into().unwrap(),
>           };
>   
>           match self.borrow_mut(py)?.pack(parents, now) {
> diff --git a/rust/hg-cpython/src/parsers.rs b/rust/hg-cpython/src/parsers.rs
> --- a/rust/hg-cpython/src/parsers.rs
> +++ b/rust/hg-cpython/src/parsers.rs
> @@ -15,10 +15,11 @@ use cpython::{
>       PythonObject, ToPyObject,
>   };
>   use hg::{
> -    pack_dirstate, parse_dirstate, utils::copy_into_array, DirstateEntry,
> +    pack_dirstate, parse_dirstate, DirstateEntry,
>       DirstatePackError, DirstateParents, DirstateParseError, PARENT_SIZE,
>   };
>   use std::collections::HashMap;
> +use std::convert::TryInto;
>   
>   use libc::c_char;
>   
> @@ -120,8 +121,8 @@ fn pack_dirstate_wrapper(
>           &mut dirstate_map,
>           &copies?,
>           DirstateParents {
> -            p1: copy_into_array(&p1),
> -            p2: copy_into_array(&p2),
> +            p1: p1.try_into().unwrap(),
> +            p2: p2.try_into().unwrap(),
>           },
>           Duration::from_secs(now.as_object().extract::<u64>(py)?),
>       ) {
> _______________________________________________
> Mercurial-devel mailing list
> Mercurial-devel@mercurial-scm.org
> https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel

Patch

diff --git a/rust/hg-core/src/dirstate/dirstate_map.rs b/rust/hg-core/src/dirstate/dirstate_map.rs
--- a/rust/hg-core/src/dirstate/dirstate_map.rs
+++ b/rust/hg-core/src/dirstate/dirstate_map.rs
@@ -7,13 +7,13 @@ 
 
 use crate::{
     dirstate::{parsers::PARENT_SIZE, EntryState},
-    pack_dirstate, parse_dirstate,
-    utils::copy_into_array,
-    CopyMap, DirsIterable, DirsMultiset, DirstateEntry, DirstateError,
-    DirstateMapError, DirstateParents, DirstateParseError, StateMap,
+    pack_dirstate, parse_dirstate, CopyMap, DirsIterable, DirsMultiset,
+    DirstateEntry, DirstateError, DirstateMapError, DirstateParents,
+    DirstateParseError, StateMap,
 };
 use core::borrow::Borrow;
 use std::collections::{HashMap, HashSet};
+use std::convert::TryInto;
 use std::iter::FromIterator;
 use std::ops::Deref;
 use std::time::Duration;
@@ -260,10 +260,10 @@  impl DirstateMap {
         let parents;
         if file_contents.len() == PARENT_SIZE * 2 {
             parents = DirstateParents {
-                p1: copy_into_array(&file_contents[..PARENT_SIZE]),
-                p2: copy_into_array(
-                    &file_contents[PARENT_SIZE..PARENT_SIZE * 2],
-                ),
+                p1: file_contents[..PARENT_SIZE].try_into().unwrap(),
+                p2: file_contents[PARENT_SIZE..PARENT_SIZE * 2]
+                    .try_into()
+                    .unwrap(),
             };
         } else if file_contents.is_empty() {
             parents = DirstateParents {
diff --git a/rust/hg-core/src/dirstate/parsers.rs b/rust/hg-core/src/dirstate/parsers.rs
--- a/rust/hg-core/src/dirstate/parsers.rs
+++ b/rust/hg-core/src/dirstate/parsers.rs
@@ -5,7 +5,6 @@ 
 
 use crate::{
     dirstate::{CopyMap, EntryState, StateMap},
-    utils::copy_into_array,
     DirstateEntry, DirstatePackError, DirstateParents, DirstateParseError,
 };
 use byteorder::{BigEndian, ReadBytesExt, WriteBytesExt};
@@ -31,8 +30,8 @@  pub fn parse_dirstate(
 
     let mut curr_pos = PARENT_SIZE * 2;
     let parents = DirstateParents {
-        p1: copy_into_array(&contents[..PARENT_SIZE]),
-        p2: copy_into_array(&contents[PARENT_SIZE..curr_pos]),
+        p1: contents[..PARENT_SIZE].try_into().unwrap(),
+        p2: contents[PARENT_SIZE..curr_pos].try_into().unwrap(),
     };
 
     while curr_pos < contents.len() {
diff --git a/rust/hg-core/src/utils.rs b/rust/hg-core/src/utils.rs
--- a/rust/hg-core/src/utils.rs
+++ b/rust/hg-core/src/utils.rs
@@ -9,23 +9,6 @@ 
 
 pub mod files;
 
-use std::convert::AsMut;
-
-/// Takes a slice and copies it into an array.
-///
-/// # Panics
-///
-/// Will panic if the slice and target array don't have the same length.
-pub fn copy_into_array<A, T>(slice: &[T]) -> A
-where
-    A: Sized + Default + AsMut<[T]>,
-    T: Copy,
-{
-    let mut a = Default::default();
-    <A as AsMut<[T]>>::as_mut(&mut a).copy_from_slice(slice);
-    a
-}
-
 /// Replaces the `from` slice with the `to` slice inside the `buf` slice.
 ///
 /// # Examples
diff --git a/rust/hg-cpython/src/dirstate/dirstate_map.rs b/rust/hg-cpython/src/dirstate/dirstate_map.rs
--- a/rust/hg-cpython/src/dirstate/dirstate_map.rs
+++ b/rust/hg-cpython/src/dirstate/dirstate_map.rs
@@ -24,7 +24,7 @@  use crate::{
     ref_sharing::PySharedState,
 };
 use hg::{
-    utils::copy_into_array, DirsIterable, DirsMultiset, DirstateEntry,
+    DirsIterable, DirsMultiset, DirstateEntry,
     DirstateMap as RustDirstateMap, DirstateParents, DirstateParseError,
     EntryState,
 };
@@ -239,8 +239,9 @@  py_class!(pub class DirstateMap |py| {
     }
 
     def setparents(&self, p1: PyObject, p2: PyObject) -> PyResult<PyObject> {
-        let p1 = copy_into_array(p1.extract::<PyBytes>(py)?.data(py));
-        let p2 = copy_into_array(p2.extract::<PyBytes>(py)?.data(py));
+        // TODO: don't panic; raise Python exception instead.
+        let p1 = p1.extract::<PyBytes>(py)?.data(py).try_into().unwrap();
+        let p2 = p2.extract::<PyBytes>(py)?.data(py).try_into().unwrap();
 
         self.inner(py)
             .borrow_mut()
@@ -274,8 +275,9 @@  py_class!(pub class DirstateMap |py| {
     ) -> PyResult<PyBytes> {
         let now = Duration::new(now.extract(py)?, 0);
         let parents = DirstateParents {
-            p1: copy_into_array(p1.extract::<PyBytes>(py)?.data(py)),
-            p2: copy_into_array(p2.extract::<PyBytes>(py)?.data(py)),
+            // TODO: don't panic; raise Python exception instead.
+            p1: p1.extract::<PyBytes>(py)?.data(py).try_into().unwrap(),
+            p2: p2.extract::<PyBytes>(py)?.data(py).try_into().unwrap(),
         };
 
         match self.borrow_mut(py)?.pack(parents, now) {
diff --git a/rust/hg-cpython/src/parsers.rs b/rust/hg-cpython/src/parsers.rs
--- a/rust/hg-cpython/src/parsers.rs
+++ b/rust/hg-cpython/src/parsers.rs
@@ -15,10 +15,11 @@  use cpython::{
     PythonObject, ToPyObject,
 };
 use hg::{
-    pack_dirstate, parse_dirstate, utils::copy_into_array, DirstateEntry,
+    pack_dirstate, parse_dirstate, DirstateEntry,
     DirstatePackError, DirstateParents, DirstateParseError, PARENT_SIZE,
 };
 use std::collections::HashMap;
+use std::convert::TryInto;
 
 use libc::c_char;
 
@@ -120,8 +121,8 @@  fn pack_dirstate_wrapper(
         &mut dirstate_map,
         &copies?,
         DirstateParents {
-            p1: copy_into_array(&p1),
-            p2: copy_into_array(&p2),
+            p1: p1.try_into().unwrap(),
+            p2: p2.try_into().unwrap(),
         },
         Duration::from_secs(now.as_object().extract::<u64>(py)?),
     ) {