D6773: rust-hgpath: add HgPath and HgPathBuf structs to encapsulate handling of paths

[Alphare (Raphaël Gomès)]

Alphare added a comment.


  In D6773#99433 <https://phab.mercurial-scm.org/D6773#99433>, @yuja wrote:
  
  >> +#[derive(Eq, Ord, PartialEq, PartialOrd, Debug, Hash)]
  >> +pub struct HgPath {
  >> +    inner: [u8],
  >> +}
  >
  > I found `std::path::Path` has a well-written inline comment about
  > the unsafe pointer cast. Let's copy it so we won't introduce a
  > memory issue.
  
  I'm not 100% sure which comment you're referring to, sorry.
  
  In D6773#99434 <https://phab.mercurial-scm.org/D6773#99434>, @yuja wrote:
  
  > I'm not sure if this `HgPath` type is designed to be always validated or not.
  > If yes, these functions can be private, and we'll need a conservative
  > `HgPath::from_bytes(s) -> Result<&HgPath>` and `unsafe fn from_bytes_unchecked(s)`
  > functions. If not, maybe `HgPath::new()` shouldn't validate the bytes at all.
  > I feel `debug_assert()` implies that the condition must always be met,
  > so the caller has to guarantee that.
  
  I feel like both the performance implications and the ergonomics of checking everything are going to be a problem. @kevincox's idea of using `debug_assert!` seemed like a decent enough check to ease development. These functions could also be used during conversions to `OsString` and the like.
  
  >> +impl Index<usize> for HgPath {
  >> +    type Output = u8;
  >> +
  >> +    fn index(&self, i: usize) -> &Self::Output {
  >> +        &self.inner[i]
  >> +    }
  >> +}
  >
  > [...]
  > Better to not implement `path[...]` since it can be considered returning
  > a path component at the specified index. We can always do
  > `path.as_bytes()[...]` explicitly.
  
  I'd leave `Index<RangeFull>`since it it not ambiguous and is a neat shorthand.

REPOSITORY
  rHG Mercurial

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

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

To: Alphare, #hg-reviewers, kevincox
Cc: yuja, durin42, kevincox, mercurial-devel