Skip to content

Commit

Permalink
Enforce invariant that ranges are well-formed
Browse files Browse the repository at this point in the history
This is in contrast to `std::ops::Range`, which are not guaranteed to
be proper. For this reason, we have to lose `From` impls as well.
  • Loading branch information
matklad committed Mar 9, 2020
1 parent dea4c9c commit cd1f821
Show file tree
Hide file tree
Showing 2 changed files with 21 additions and 69 deletions.
88 changes: 20 additions & 68 deletions src/range.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2,9 +2,9 @@ use {
crate::TextSize,
std::{
cmp,
convert::{TryFrom, TryInto},
convert::TryInto,
fmt,
ops::{Bound, Index, IndexMut, Range, RangeBounds},
ops::{Bound, Index, IndexMut, RangeBounds},
},
};

Expand All @@ -30,15 +30,11 @@ use {
/// † See the note on [`TextRange::len`] for differing behavior for incorrect reverse ranges.
#[derive(Copy, Clone, Eq, PartialEq, Hash)]
pub struct TextRange {
// Invariant: start <= end
start: TextSize,
end: TextSize,
}

#[allow(non_snake_case)]
pub(crate) const fn TextRange(start: TextSize, end: TextSize) -> TextRange {
TextRange { start, end }
}

impl fmt::Debug for TextRange {
fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
write!(f, "[{}..{})", self.start(), self.end())
Expand All @@ -47,6 +43,16 @@ impl fmt::Debug for TextRange {

/// Identity methods.
impl TextRange {
/// Creates a new `TextRange` with given `start` and `end.
///
/// # Panics
///
/// Panics if `end < start`.
pub fn new(start: TextSize, end: TextSize) -> TextRange {
assert!(start <= end);
TextRange { start, end }
}

/// The start point of this range.
pub const fn start(self) -> TextSize {
self.start
Expand All @@ -58,11 +64,6 @@ impl TextRange {
}

/// The size of this range.
///
/// # Panics
///
/// When `end() < start()`, triggers a subtraction overflow.
/// This will panic with debug assertions, and overflow without.
pub const fn len(self) -> TextSize {
// HACK for const fn: math on primitives only
TextSize(self.end().raw - self.start().raw)
Expand All @@ -71,11 +72,10 @@ impl TextRange {
/// Check if this range empty or reversed.
///
/// When `end() < start()`, this returns false.
/// Code should prefer `is_empty()` to `len() == 0`,
/// as this safeguards against incorrect reverse ranges.
/// Code should prefer `is_empty()` to `len() == 0`.
pub const fn is_empty(self) -> bool {
// HACK for const fn: math on primitives only
self.start().raw >= self.end().raw
self.start().raw == self.end().raw
}
}

Expand All @@ -91,14 +91,17 @@ impl TextRange {
pub fn intersection(lhs: TextRange, rhs: TextRange) -> Option<TextRange> {
let start = cmp::max(lhs.start(), rhs.start());
let end = cmp::min(lhs.end(), rhs.end());
Some(TextRange(start, end)).filter(|_| start <= end)
if end < start {
return None;
}
Some(TextRange::new(start, end))
}

/// The smallest range that completely contains both ranges.
pub fn covering(lhs: TextRange, rhs: TextRange) -> TextRange {
let start = cmp::min(lhs.start(), rhs.start());
let end = cmp::max(lhs.end(), rhs.end());
TextRange(start, end)
TextRange::new(start, end)
}

/// Check if this range contains a point.
Expand Down Expand Up @@ -145,54 +148,3 @@ impl RangeBounds<TextSize> for TextRange {
Bound::Excluded(&self.end)
}
}

macro_rules! conversions {
(From<$lte:ident> for TextRange) => {
impl From<Range<$lte>> for TextRange {
fn from(value: Range<$lte>) -> TextRange {
TextRange(value.start.into(), value.end.into())
}
}
// Just support `start..end` for now, not `..end`, `start..=end`, `..=end`.
};
(TryFrom<$gt:ident> for TextRange) => {
impl TryFrom<Range<$gt>> for TextRange {
type Error = <$gt as TryInto<u32>>::Error;
fn try_from(value: Range<$gt>) -> Result<TextRange, Self::Error> {
Ok(TextRange(value.start.try_into()?, value.end.try_into()?))
}
}
// Just support `start..end` for now, not `..end`, `start..=end`, `..=end`.
};
{
lt TextSize [$($lt:ident)*]
eq TextSize [$($eq:ident)*]
gt TextSize [$($gt:ident)*]
varries [$($var:ident)*]
} => {
$(
conversions!(From<$lt> for TextRange);
// unlike TextSize, we do not provide conversions in the "out" direction.
)*

$(
conversions!(From<$eq> for TextRange);
)*

$(
conversions!(TryFrom<$gt> for TextRange);
)*

$(
conversions!(TryFrom<$var> for TextRange);
)*
};
}

// FIXME: when `default impl` is usable, change to blanket impls for [Try]Into<TextSize> instead
conversions! {
lt TextSize [u8 u16]
eq TextSize [u32 TextSize]
gt TextSize [u64]
varries [usize]
}
2 changes: 1 addition & 1 deletion tests/main.rs
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@ fn size(x: u32) -> TextSize {
}

fn range(x: ops::Range<u32>) -> TextRange {
TextRange::from(x)
TextRange::new(x.start.into(), x.end.into())
}

#[test]
Expand Down

0 comments on commit cd1f821

Please sign in to comment.