Page MenuHomePhabricator

rust-dirstate-status: add `walk_explicit` implementation, use `Matcher` trait
Needs ReviewPublic

Authored by Alphare on Fri, Nov 29, 1:22 PM.

Details

Reviewers
kevincox
Group Reviewers
hg-reviewers
Summary

This is the first time we actually use the Matcher trait, still for a small
subset of all matchers defined in Python.

While I haven't yet actually measured the performance of this, I have tried
to avoid any unnecessary allocations. This forces the use of heavy lifetimes
annotations which I am not sure we can simplify, although I would be happy
to be proven wrong.

Diff Detail

Repository
rHG Mercurial
Branch
default
Lint
No Linters Available
Unit
No Unit Test Coverage

Event Timeline

Alphare created this revision.Fri, Nov 29, 1:22 PM
kevincox requested changes to this revision.Mon, Dec 2, 8:26 AM
kevincox added inline comments.
rust/hg-core/src/dirstate/status.rs
125

It seems to me that you can just use one lifetime here since the two input limit the output.

https://rust.godbolt.org/z/57bf6T But maybe I over-simplified the problem.

168

Isn't this the same as the first case? Can we just remove the first case?

240

It surprises me this annotation is required? I thought this would be the assumed meaning.

This revision now requires changes to proceed.Mon, Dec 2, 8:26 AM
Alphare updated this revision to Diff 18423.Mon, Dec 2, 9:20 AM
Alphare added inline comments.Mon, Dec 2, 9:21 AM
rust/hg-core/src/dirstate/status.rs
125

You're right, the complex annotations are only needed on status, this only returns values from files.

168

The if will be needed in a (semi-distant) future series, I'll remove it for now, thanks.

240

I thought so too, but it seems that the compiler's elision rules do not apply with Iterators. It's probable a more complicated problem to solve in a deterministic fashion than just a plain Vec.

kevincox accepted this revision.Mon, Dec 2, 10:16 AM