Skip to content

Commit

Permalink
feat!: turn entry::Stage into an enum.
Browse files Browse the repository at this point in the history
This way, it's much clearer what possible values are.
For performance, it also adds `Entry::stage_raw()` to avoid having
to match on a number.
  • Loading branch information
Byron committed Apr 3, 2024
1 parent 16dc027 commit a6cc781
Show file tree
Hide file tree
Showing 5 changed files with 69 additions and 25 deletions.
19 changes: 10 additions & 9 deletions gix-index/src/access/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@ use std::{cmp::Ordering, ops::Range};
use bstr::{BStr, ByteSlice, ByteVec};
use filetime::FileTime;

use crate::entry::{Stage, StageRaw};
use crate::{entry, extension, AccelerateLookup, Entry, PathStorage, PathStorageRef, State, Version};

// TODO: integrate this somehow, somewhere, depending on later usage.
Expand Down Expand Up @@ -81,7 +82,7 @@ impl State {
res
})
.ok()?;
self.entry_index_by_idx_and_stage(path, idx, stage, stage_cmp)
self.entry_index_by_idx_and_stage(path, idx, stage as StageRaw, stage_cmp)
}

/// Walk as far in `direction` as possible, with [`Ordering::Greater`] towards higher stages, and [`Ordering::Less`]
Expand Down Expand Up @@ -112,7 +113,7 @@ impl State {
&self,
path: &BStr,
idx: usize,
wanted_stage: entry::Stage,
wanted_stage: entry::StageRaw,
stage_cmp: Ordering,
) -> Option<usize> {
match stage_cmp {
Expand All @@ -121,15 +122,15 @@ impl State {
.enumerate()
.rev()
.take_while(|(_, e)| e.path(self) == path)
.find_map(|(idx, e)| (e.stage() == wanted_stage).then_some(idx)),
.find_map(|(idx, e)| (e.stage_raw() == wanted_stage).then_some(idx)),
Ordering::Equal => Some(idx),
Ordering::Less => self
.entries
.get(idx + 1..)?
.iter()
.enumerate()
.take_while(|(_, e)| e.path(self) == path)
.find_map(|(ofs, e)| (e.stage() == wanted_stage).then_some(idx + ofs + 1)),
.find_map(|(ofs, e)| (e.stage_raw() == wanted_stage).then_some(idx + ofs + 1)),
}
}

Expand Down Expand Up @@ -291,15 +292,15 @@ impl State {
.binary_search_by(|e| {
let res = e.path(self).cmp(path);
if res.is_eq() {
stage_at_index = e.stage();
stage_at_index = e.stage_raw();
}
res
})
.ok()?;
let idx = if stage_at_index == 0 || stage_at_index == 2 {
idx
} else {
self.entry_index_by_idx_and_stage(path, idx, 2, stage_at_index.cmp(&2))?
self.entry_index_by_idx_and_stage(path, idx, Stage::Ours as StageRaw, stage_at_index.cmp(&2))?
};
Some(&self.entries[idx])
}
Expand Down Expand Up @@ -334,13 +335,13 @@ impl State {
+ self.entries[low..].partition_point(|e| e.path(self).get(..prefix_len).map_or(false, |p| p <= prefix));

let low_entry = &self.entries.get(low)?;
if low_entry.stage() != 0 {
if low_entry.stage_raw() != 0 {
low = self
.walk_entry_stages(low_entry.path(self), low, Ordering::Less)
.unwrap_or(low);
}
if let Some(high_entry) = self.entries.get(high) {
if high_entry.stage() != 0 {
if high_entry.stage_raw() != 0 {
high = self
.walk_entry_stages(high_entry.path(self), high, Ordering::Less)
.unwrap_or(high);
Expand Down Expand Up @@ -374,7 +375,7 @@ impl State {
.binary_search_by(|e| {
let res = e.path(self).cmp(path);
if res.is_eq() {
stage_at_index = e.stage();
stage_at_index = e.stage_raw();
}
res
})
Expand Down
17 changes: 17 additions & 0 deletions gix-index/src/entry/flags.rs
Original file line number Diff line number Diff line change
Expand Up @@ -62,6 +62,23 @@ bitflags! {
impl Flags {
/// Return the stage as extracted from the bits of this instance.
pub fn stage(&self) -> Stage {
match self.stage_raw() {
0 => Stage::Unconflicted,
1 => Stage::Base,
2 => Stage::Ours,
3 => Stage::Theirs,
_ => unreachable!("BUG: Flags::STAGE_MASK is two bits, whose 4 possible values we have covered"),
}
}

/// Return an entry's stage as raw number between 0 and 4.
/// Possible values are:
///
/// * 0 = no conflict,
/// * 1 = base,
/// * 2 = ours,
/// * 3 = theirs
pub fn stage_raw(&self) -> u32 {
(*self & Flags::STAGE_MASK).bits() >> 12
}

Expand Down
29 changes: 27 additions & 2 deletions gix-index/src/entry/mod.rs
Original file line number Diff line number Diff line change
@@ -1,9 +1,23 @@
/// The stage of an entry, one of…
/// The stage of an entry.
#[derive(Default, Copy, Clone, Debug, PartialEq, Eq, Ord, PartialOrd, Hash)]
pub enum Stage {
/// This is the default, and most entries are in this stage.
#[default]
Unconflicted = 0,
/// The entry is the common base between 'our' change and 'their' change, for comparison.
Base = 1,
/// The entry represents our change.
Ours = 2,
/// The entry represents their change.
Theirs = 3,
}

// The stage of an entry, one of…
/// * 0 = no conflict,
/// * 1 = base,
/// * 2 = ours,
/// * 3 = theirs
pub type Stage = u32;
pub type StageRaw = u32;

///
#[allow(clippy::empty_docs)]
Expand Down Expand Up @@ -78,6 +92,17 @@ mod access {
pub fn stage(&self) -> entry::Stage {
self.flags.stage()
}

/// Return an entry's stage as raw number between 0 and 4.
/// Possible values are:
///
/// * 0 = no conflict,
/// * 1 = base,
/// * 2 = ours,
/// * 3 = theirs
pub fn stage_raw(&self) -> u32 {
self.flags.stage_raw()
}
}
}

Expand Down
10 changes: 5 additions & 5 deletions gix-index/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -144,6 +144,7 @@ pub struct State {
}

mod impls {
use crate::entry::Stage;
use std::fmt::{Debug, Formatter};

use crate::State;
Expand All @@ -155,11 +156,10 @@ mod impls {
f,
"{} {}{:?} {} {}",
match entry.flags.stage() {
0 => " ",
1 => "BASE ",
2 => "OURS ",
3 => "THEIRS ",
_ => "UNKNOWN",
Stage::Unconflicted => " ",
Stage::Base => "BASE ",
Stage::Ours => "OURS ",
Stage::Theirs => "THEIRS ",
},
if entry.flags.is_empty() {
"".to_string()
Expand Down
19 changes: 10 additions & 9 deletions gix-index/tests/index/access.rs
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
use crate::index::Fixture;
use bstr::{BString, ByteSlice};
use gix_index::entry::Stage;

fn icase_fixture() -> gix_index::File {
Fixture::Generated("v2_icase_name_clashes").open()
Expand All @@ -11,7 +12,7 @@ fn entry_by_path() {
for entry in file.entries() {
let path = entry.path(&file);
assert_eq!(file.entry_by_path(path), Some(entry));
assert_eq!(file.entry_by_path_and_stage(path, 0), Some(entry));
assert_eq!(file.entry_by_path_and_stage(path, Stage::Unconflicted), Some(entry));
}
}

Expand Down Expand Up @@ -140,27 +141,27 @@ fn entry_by_path_and_stage() {
for entry in file.entries() {
let path = entry.path(&file);
assert_eq!(
file.entry_index_by_path_and_stage(path, 0)
file.entry_index_by_path_and_stage(path, Stage::Unconflicted)
.map(|idx| &file.entries()[idx]),
Some(entry)
);
assert_eq!(file.entry_by_path_and_stage(path, 0), Some(entry));
assert_eq!(file.entry_by_path_and_stage(path, Stage::Unconflicted), Some(entry));
}
}

#[test]
fn entry_by_path_with_conflicting_file() {
let file = Fixture::Loose("conflicting-file").open();
for expected_stage in [1 /* common ancestor */, 2 /* ours */, 3 /* theirs */] {
for expected_stage in [Stage::Base,Stage::Ours, Stage::Theirs] {
assert!(
file.entry_by_path_and_stage("file".into(), expected_stage).is_some(),
"we have no stage 0 during a conflict, but all other ones. Missed {expected_stage}"
"we have no stage 0 during a conflict, but all other ones. Missed {expected_stage:?}"
);
}

assert_eq!(
file.entry_by_path("file".into()).expect("found").stage(),
2,
Stage::Ours,
"we always find our stage while in a merge"
);
}
Expand Down Expand Up @@ -226,13 +227,13 @@ fn sort_entries() {

for (idx, entry) in file.entries()[..valid_entries].iter().enumerate() {
assert_eq!(
file.entry_index_by_path_and_stage_bounded(entry.path(&file), 0, valid_entries),
file.entry_index_by_path_and_stage_bounded(entry.path(&file), Stage::Unconflicted, valid_entries),
Some(idx),
"we can still find entries in the correctly sorted region"
);
}
assert_eq!(
file.entry_by_path_and_stage(new_entry_path, 0),
file.entry_by_path_and_stage(new_entry_path, Stage::Unconflicted),
None,
"new entry can't be found due to incorrect order"
);
Expand All @@ -241,7 +242,7 @@ fn sort_entries() {
assert!(file.verify_entries().is_ok(), "sorting of entries restores invariants");

assert_eq!(
file.entry_by_path_and_stage(new_entry_path, 0)
file.entry_by_path_and_stage(new_entry_path, Stage::Unconflicted)
.expect("can be found")
.path(&file),
new_entry_path,
Expand Down

0 comments on commit a6cc781

Please sign in to comment.