[Commented On] D12429: rust: fix unsound `OwningDirstateMap`

[martinvonz (Martin von Zweigbergk)]

martinvonz added inline comments.

INLINE COMMENTS

> dirstate_map.rs:989
> +    /// we need to borrow from `Self`.
> +    pub fn status<'a, R>(
>          &'a mut self,

this lifetime doesn't seem to be needed anymore

> owning.rs:75
>  
> -    pub fn get_map<'a>(&'a self) -> &'a DirstateMap<'a> {
> -        // SAFETY: same reasoning as in `get_pair_mut` above.
> -        let ptr: *mut DirstateMap<'a> = self.ptr.cast();
> -        unsafe { &*ptr }
> +    pub fn get_map_mut<'a, R>(
> +        &mut self,

delete unused lifetime

> owning.rs:75
>  
> -    pub fn get_map<'a>(&'a self) -> &'a DirstateMap<'a> {
> -        // SAFETY: same reasoning as in `get_pair_mut` above.
> -        let ptr: *mut DirstateMap<'a> = self.ptr.cast();
> -        unsafe { &*ptr }
> +    pub fn get_map_mut<'a, R>(
> +        &mut self,

call this `with_map_mut()` now that it takes a callback?

> owning.rs:86
>  
>      pub fn on_disk<'a>(&'a self) -> &'a [u8] {
> +        self.borrow_on_disk()

nit: can drop this explicit lifetime, right?

> status.rs:43
>  #[timed]
> -pub fn status<'tree, 'on_disk: 'tree>(
> -    dmap: &'tree mut DirstateMap<'on_disk>,
> +pub fn status<'a>(
> +    dmap: &'a mut DirstateMap,

nit: call the lifetime something useful (like `'dirstate` or `'map`)?

> status.rs:237
> +
> +    type StatusReturn<'a> =
> +        Result<(DirstateStatus<'a>, Vec<PatternFileWarning>), StatusError>;

nit: maybe call it `StatusResult` instead? it's a `Result`, and it's not returned from anywhere

maybe also rename `OwningDirstateMap::status()` to `with_status()`

REPOSITORY
  rHG Mercurial

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

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

To: Alphare, #hg-reviewers
Cc: martinvonz, mercurial-patches
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.mercurial-scm.org/pipermail/mercurial-patches/attachments/20220404/0f4ce465/attachment-0002.html>