Patchwork D6765: rustfilepatterns: shorter code for concatenating slices

login
register
mail settings
Submitter phabricator
Date Aug. 26, 2019, 4:16 a.m.
Message ID <differential-rev-PHID-DREV-t7nupojuptq4ngvo64wb-req@mercurial-scm.org>
Download mbox | patch
Permalink /patch/41409/
State Superseded
Headers show

Comments

phabricator - Aug. 26, 2019, 4:16 a.m.
valentin.gatienbaron created this revision.
Herald added subscribers: mercurial-devel, kevincox, durin42.
Herald added a reviewer: hg-reviewers.

REPOSITORY
  rHG Mercurial

REVISION DETAIL
  https://phab.mercurial-scm.org/D6765

AFFECTED FILES
  rust/hg-core/src/filepatterns.rs

CHANGE DETAILS




To: valentin.gatienbaron, #hg-reviewers
Cc: durin42, kevincox, mercurial-devel
phabricator - Aug. 28, 2019, 9:26 a.m.
kevincox added inline comments.
kevincox accepted this revision.

INLINE COMMENTS

> filepatterns.rs:161
>              }
> -            let mut res = b".*".to_vec();
> -            res.extend(pattern);
> -            res
> +            [&b".*"[..], pattern].concat()
>          }

I don't think you need the `&_[..]`. https://rust.godbolt.org/z/Wo-vza

REPOSITORY
  rHG Mercurial

CHANGES SINCE LAST ACTION
  https://phab.mercurial-scm.org/D6765/new/

REVISION DETAIL
  https://phab.mercurial-scm.org/D6765

To: valentin.gatienbaron, #hg-reviewers, Alphare, kevincox
Cc: durin42, kevincox, mercurial-devel
phabricator - Aug. 28, 2019, 11:44 a.m.
valentin.gatienbaron added inline comments.

INLINE COMMENTS

> kevincox wrote in filepatterns.rs:161
> I don't think you need the `&_[..]`. https://rust.godbolt.org/z/Wo-vza

Indeed, although it took a compiler upgrade.
I don't really understand why the compiler accepts the change here but not in the other places. It seems a bit random to remove the borrow only here. What do you think about the next commit? It would remove the `&_[..]` more predictably.

REPOSITORY
  rHG Mercurial

CHANGES SINCE LAST ACTION
  https://phab.mercurial-scm.org/D6765/new/

REVISION DETAIL
  https://phab.mercurial-scm.org/D6765

To: valentin.gatienbaron, #hg-reviewers, Alphare, kevincox
Cc: durin42, kevincox, mercurial-devel
phabricator - Aug. 28, 2019, 11:48 a.m.
kevincox added inline comments.

INLINE COMMENTS

> valentin.gatienbaron wrote in filepatterns.rs:161
> Indeed, although it took a compiler upgrade.
> I don't really understand why the compiler accepts the change here but not in the other places. It seems a bit random to remove the borrow only here. What do you think about the next commit? It would remove the `&_[..]` more predictably.

For the other ones I think the problem is that it gets the element type from the first element as `&Vec<u8>`. I think a better approach then creating the wrapper function is doing `[vec.as_slice(), b"foobar"].concat()`.

REPOSITORY
  rHG Mercurial

CHANGES SINCE LAST ACTION
  https://phab.mercurial-scm.org/D6765/new/

REVISION DETAIL
  https://phab.mercurial-scm.org/D6765

To: valentin.gatienbaron, #hg-reviewers, Alphare, kevincox
Cc: durin42, kevincox, mercurial-devel
phabricator - Aug. 28, 2019, 11:59 a.m.
valentin.gatienbaron added inline comments.
valentin.gatienbaron marked 2 inline comments as done.

INLINE COMMENTS

> kevincox wrote in filepatterns.rs:161
> For the other ones I think the problem is that it gets the element type from the first element as `&Vec<u8>`. I think a better approach then creating the wrapper function is doing `[vec.as_slice(), b"foobar"].concat()`.

I see. So the reason why the code above was accepted is because `pattern` was already a `&[u8]`, and not a `&Vec<u8>`. Thanks!

REPOSITORY
  rHG Mercurial

CHANGES SINCE LAST ACTION
  https://phab.mercurial-scm.org/D6765/new/

REVISION DETAIL
  https://phab.mercurial-scm.org/D6765

To: valentin.gatienbaron, #hg-reviewers, Alphare, kevincox
Cc: durin42, kevincox, mercurial-devel

Patch

diff --git a/rust/hg-core/src/filepatterns.rs b/rust/hg-core/src/filepatterns.rs
--- a/rust/hg-core/src/filepatterns.rs
+++ b/rust/hg-core/src/filepatterns.rs
@@ -158,26 +158,20 @@ 
             if pattern[0] == b'^' {
                 return pattern.to_owned();
             }
-            let mut res = b".*".to_vec();
-            res.extend(pattern);
-            res
+            [&b".*"[..], pattern].concat()
         }
         PatternSyntax::Path | PatternSyntax::RelPath => {
             if pattern == b"." {
                 return vec![];
             }
-            let mut pattern = escape_pattern(pattern);
-            pattern.extend(b"(?:/|$)");
-            pattern
+            [&escape_pattern(pattern), &b"(?:/|$)"[..]].concat()
         }
         PatternSyntax::RootFiles => {
             let mut res = if pattern == b"." {
                 vec![]
             } else {
                 // Pattern is a directory name.
-                let mut as_vec: Vec<u8> = escape_pattern(pattern);
-                as_vec.push(b'/');
-                as_vec
+                [&escape_pattern(pattern), &b"/"[..]].concat()
             };
 
             // Anything after the pattern must be a non-directory.
@@ -185,24 +179,16 @@ 
             res
         }
         PatternSyntax::RelGlob => {
-            let mut res: Vec<u8> = vec![];
             let glob_re = glob_to_re(pattern);
             if let Some(rest) = glob_re.chop_prefix(b"[^/]*") {
-                res.extend(b".*");
-                res.extend(rest);
+                [&b".*"[..], rest, globsuffix].concat()
             } else {
-                res.extend(b"(?:|.*/)");
-                res.extend(glob_re);
+                [&b"(?:|.*/)"[..], &glob_re, globsuffix].concat()
             }
-            res.extend(globsuffix.iter());
-            res
         }
         PatternSyntax::Glob
         | PatternSyntax::RootGlob => {
-            let mut res: Vec<u8> = vec![];
-            res.extend(glob_to_re(pattern));
-            res.extend(globsuffix.iter());
-            res
+            [&glob_to_re(pattern), globsuffix].concat()
         }
     }
 }