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
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)?), ) {