D7794: rust-nodemap: generic NodeTreeVisitor

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


gracinet added inline comments.
gracinet marked 2 inline comments as done.

INLINE COMMENTS

> martinvonz wrote in nodemap.rs:264-269
> There will only be a valid result if this is a leaf, so it might be better to combine `leaf` and `opt` into one `Option<Option<Revision>>` so the caller cannot mistake a `opt=None` for "missing" when `leaf=false`.

I understand your concern, but `<Option<Option<Revision>>` would defeat the purpose of not having a dead match arm to fill with a "can't happen" comment in the future `insert()`.

Fortunately, @kevincox suggestion of having `NodeTreeVisitor` emit  a `struct` provides us with the best of both worlds, since it can have methods.

It should be fully transparent performance-wise, I just hope that is really true.

REPOSITORY
  rHG Mercurial

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

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

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


More information about the Mercurial-devel mailing list