From 361ee87d37d189386d82dcf69dbc0f5a57c1e405 Mon Sep 17 00:00:00 2001 From: konstin Date: Fri, 8 Nov 2024 10:24:43 +0100 Subject: [PATCH 1/2] Document invariant on Ranges --- version-ranges/src/lib.rs | 12 ++++++++++++ 1 file changed, 12 insertions(+) diff --git a/version-ranges/src/lib.rs b/version-ranges/src/lib.rs index 0e33bd5c..289d63c1 100644 --- a/version-ranges/src/lib.rs +++ b/version-ranges/src/lib.rs @@ -46,6 +46,17 @@ use proptest::prelude::*; use smallvec::{smallvec, SmallVec}; /// Ranges represents multiple intervals of a continuous range of monotone increasing values. +/// +/// Internally, [`Ranges`] are an ordered list of segments, where segment is a bounds pair. +/// +/// Invariants: +/// 1. The segments are sorted, from lowest to highest (through `Ord`). +/// 2. Each segment contains at least one version (start < end). +/// 3. There is at least one version between two segments. +/// +/// These ensure that equivalent instances have an identical representation, which is important +/// for `Eq` and `Hash`. Given that this type doesn't know the lower bound, upper bound, granularity +/// or actual instances of `V`, this applies only to equality under pubgrub's model. #[derive(Debug, Clone, Eq, PartialEq, Hash)] #[cfg_attr(feature = "serde", derive(serde::Serialize))] #[cfg_attr(feature = "serde", serde(transparent))] @@ -283,6 +294,7 @@ impl Ranges { } } + /// See [`Ranges`] for the invariants checked. fn check_invariants(self) -> Self { if cfg!(debug_assertions) { for p in self.segments.as_slice().windows(2) { From c739b41928f2d24be825395f63778012759b08be Mon Sep 17 00:00:00 2001 From: konstin Date: Sun, 10 Nov 2024 11:16:08 +0100 Subject: [PATCH 2/2] More explanantion --- version-ranges/src/lib.rs | 12 ++++++++++-- 1 file changed, 10 insertions(+), 2 deletions(-) diff --git a/version-ranges/src/lib.rs b/version-ranges/src/lib.rs index 289d63c1..9d3b9959 100644 --- a/version-ranges/src/lib.rs +++ b/version-ranges/src/lib.rs @@ -55,8 +55,16 @@ use smallvec::{smallvec, SmallVec}; /// 3. There is at least one version between two segments. /// /// These ensure that equivalent instances have an identical representation, which is important -/// for `Eq` and `Hash`. Given that this type doesn't know the lower bound, upper bound, granularity -/// or actual instances of `V`, this applies only to equality under pubgrub's model. +/// for `Eq` and `Hash`. Note that this representation cannot strictly guaranty equality of +/// [`Ranges`] with equality of its representation without also knowing the nature of the underlying +/// versions. In particular, if the version space is discrete, different representations, using +/// different types of bounds (exclusive/inclusive) may correspond to the same set of existing +/// versions. It is a tradeoff we acknowledge, but which makes representations of continuous version +/// sets more accessible, to better handle features like pre-releases and other types of version +/// modifiers. For example, `[(Included(3u32), Excluded(7u32))]` and +/// `[(Included(3u32), Included(6u32))]` refer to the same version set, since there is no version +/// between 6 and 7, which this crate doesn't know about. + #[derive(Debug, Clone, Eq, PartialEq, Hash)] #[cfg_attr(feature = "serde", derive(serde::Serialize))] #[cfg_attr(feature = "serde", serde(transparent))]