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