D7790: rust-node: handling binary Node prefix

gracinet (Georges Racinet) phabricator at mercurial-scm.org
Mon Jan 27 10:51:06 EST 2020


gracinet added a comment.


  > Depends on the definition of NodePrefixRef. I don't think there would be any extra allocation if you define it like this:
  
  
  
    pub enum NodePrefixRef<'a> {
        Full(&'a Node),
        Prefix(&'a NodePrefix),
    }
  
  That's an interesting idea, another possibility would be to define a trait (essentially for `get_nybble`) and implement it for `&Node` and `&NodePrefix`. We'll see when we're pass preliminary tests, thanks.

INLINE COMMENTS

> martinvonz wrote in node.rs:79
> Why not use `(len + 1) / 2` as capacity?

Just thought 20 would be the simplest one-size-fit-all. With the preparations for different hash length (and potentially even somewhat dynamic), it's really obsolete now (switched to actual len based, yes).

> martinvonz wrote in node.rs:89
> Is this lifetime parameter needed?

ah yes indeed the compiler seems to be able to infer that `&self` and `Nodeprefix.buf` have the same lifteime, and that it must then be equal to the lifetime parameter of `NodePrefix` itself.

> martinvonz wrote in node.rs:136
> What does the `&*` do? Specifically, what's different if you drop that part?

These are conversions that need to be explicit when the compiler doesn't insert them magically. Usually, I try to avoid them, but it can happen that they become non necessary in the final form after some changes.

Not needed in the new form with a real struct.

REPOSITORY
  rHG Mercurial

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

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

To: gracinet, #hg-reviewers
Cc: martinvonz, durin42, kevincox, mercurial-devel


More information about the Mercurial-devel mailing list