BTreeMap: lightly refactor range_search #81331
Conversation
rust-highfive
added
the
S-waiting-on-review
| let mut min_found = false; | ||
| let mut max_found = false; | ||
| let mut min_bound = Some(start); | ||
| let mut max_bound = Some(end); |
There was a problem hiding this comment.
Can we place a comment on what the meaning of Some/None here is?
There was a problem hiding this comment.
How about a commented type?
@rustbot modify labels: -S-waiting-on-author +S-waiting-on-review
Mark-Simulacrum
added
S-waiting-on-author
ssomers
force-pushed
the
btree_drainy_refactor_5
branch
from
04c09c6 to
0b191a2
Compare
rustbot
added
S-waiting-on-review
|
Looking a bit more at this, I am struggling with seeing this as an improvement. It seems like the encoding of the bit true/false into the Option makes it less clear what the code is doing rather than more by combining the paths for initial unbounded with "became unbounded" in a way that's confusing to me at least. Maybe we can find a different structure that works well? Maybe noting on the various branches why first/last/left/right edges are chosen would also help with making the intent clear, right now I at least have a bit of a hard time justifying without some manual working out why they're being made. |
|
I can only repeat I think it's slightly better in itself and that #81094 is the next step, which splits this large function up, and is also necessary for the |
|
The only thing I can think of that doesn/t thwart #81094 is the definition of |
ssomers
force-pushed
the
btree_drainy_refactor_5
branch
from
0b191a2 to
405b786
Compare
|
Can't keep updating this preparation of a preparation |
Ramp up to #81094 but IMHO useful in itself.
r? @Mark-Simulacrum