Merge pull request #516 from bkirwi/docs

Document various trait invariants / requirements

Author: frankmcsherry

timely/src/order.rs

@@ -5,12 +5,18 @@
 /// This trait is distinct from Rust's `PartialOrd` trait, because the implementation
 /// of that trait precludes a distinct `Ord` implementation. We need an independent
 /// trait if we want to have a partially ordered type that can also be sorted.
+///
+/// The partial order should be consistent with [Eq], in the sense that if `a == b` then
+/// `a.less_equal(b)` and `b.less_equal(a)`.
 pub trait PartialOrder<Rhs: ?Sized = Self>: PartialEq<Rhs> {
-    /// Returns `true` iff one element is strictly less than the other.
+    /// Returns true iff one element is strictly less than the other.
+    #[must_use]
     fn less_than(&self, other: &Rhs) -> bool {
         self.less_equal(other) && self != other
     }
-    /// Returns `true` iff one element is less than or equal to the other.
+
+    /// Returns true iff one element is less than or equal to the other.
+    #[must_use]
     fn less_equal(&self, other: &Rhs) -> bool;
 }
 

timely/src/progress/reachability.rs

@@ -205,7 +205,7 @@ impl<T: Timestamp> Builder<T> {
         Tracker::allocate_from(self, logger)
     }
 
-    /// Tests whether the graph a cycle of default path summaries.
+    /// Tests whether the graph includes a cycle of default path summaries.
     ///
     /// Graphs containing cycles of default path summaries will most likely
     /// not work well with progress tracking, as a timestamp can result in

timely/src/progress/timestamp.rs

@@ -9,10 +9,13 @@ use crate::ExchangeData;
 use crate::order::PartialOrder;
 
 /// A composite trait for types that serve as timestamps in timely dataflow.
+///
+/// By implementing this trait, you promise that the type's [PartialOrder] implementation
+/// is compatible with [Ord], such that if `a.less_equal(b)` then `a <= b`.
 pub trait Timestamp: Clone+Eq+PartialOrder+Debug+Send+Any+ExchangeData+Hash+Ord {
     /// A type summarizing action on a timestamp along a dataflow path.
     type Summary : PathSummary<Self> + 'static;
-    /// A minimum value suitable as a default.
+    /// A unique minimum value in our partial order, suitable as a default.
     fn minimum() -> Self;
 }
 
@@ -28,14 +31,21 @@ pub trait PathSummary<T> : Clone+'static+Eq+PartialOrder+Debug+Default {
     /// in computation, uses this method and will drop messages with timestamps that when advanced
     /// result in `None`. Ideally, all other timestamp manipulation should behave similarly.
     ///
+    /// This summary's partial order is expected to be compatible with the partial order of [T],
+    /// in the sense that if `s1.less_equal(s2)`, then `s1.results_in(&t)` is less than or equal to
+    /// `s2.results_in(&t)`.
+    ///
+    /// Note that `Self::default()` is expected to behave as an "empty" or "noop" summary, such that
+    /// `Self::default().results_in(&t) == Some(t)`.
+    ///
     /// # Examples
     /// ```
     /// use timely::progress::timestamp::PathSummary;
     ///
     /// let timestamp = 3;
     ///
     /// let summary1 = 5;
-    /// let summary2 = usize::max_value() - 2;
+    /// let summary2 = usize::MAX - 2;
     ///
     /// assert_eq!(summary1.results_in(&timestamp), Some(8));
     /// assert_eq!(summary2.results_in(&timestamp), None);
@@ -49,14 +59,27 @@ pub trait PathSummary<T> : Clone+'static+Eq+PartialOrder+Debug+Default {
     /// important that this not be used casually, as this does not prevent the actual movement of
     /// data.
     ///
+    /// Calling `results_in` on the composed summary should behave the same as though the two
+    /// summaries were applied to the argument in order.
+    ///
     /// # Examples
     /// ```
     /// use timely::progress::timestamp::PathSummary;
     ///
     /// let summary1 = 5;
-    /// let summary2 = usize::max_value() - 3;
+    /// let summary2 = usize::MAX - 3;
     ///
     /// assert_eq!(summary1.followed_by(&summary2), None);
+    ///
+    /// let time = 10;
+    /// let summary2 = 15;
+    /// assert_eq!(
+    ///     // Applying the composed summary...
+    ///     summary1.followed_by(&summary2).and_then(|s| s.results_in(&time)),
+    ///     // ...has the same result as applying the two summaries in sequence.
+    ///     summary1.results_in(&time).and_then(|t| summary2.results_in(&t)),
+    /// );
+    ///
     /// ```
     fn followed_by(&self, other: &Self) -> Option<Self>;
 }