Simplify and test trait objects, implement object-safe OutputChecker.

dev_ext/src/task.rs

@@ -4,7 +4,7 @@ use std::io;
 use std::io::{Read, Write};
 use std::path::PathBuf;
 
-use pie::{Context, KeyBounds, OutputChecker, ResourceChecker, Task};
+use pie::{Context, Key, OutputChecker, ResourceChecker, Task};
 use pie::resource::file::{FsError, ModifiedChecker};
 use pie::task::{AlwaysConsistent, EqualsChecker};
 
@@ -49,7 +49,7 @@ impl<T, E> Constant<Result<T, E>> {
     Self(Ok(val.into()))
   }
 }
-impl<T: KeyBounds> Task for Constant<T> {
+impl<T: Key> Task for Constant<T> {
   type Output = T;
   #[inline]
   fn execute<C: Context>(&self, _context: &mut C) -> Self::Output {

pie/src/context/bottom_up.rs

@@ -9,7 +9,6 @@ use crate::dependency::ResourceDependencyObj;
 use crate::pie::{SessionInternal, Tracking};
 use crate::store::{Store, TaskNode};
 use crate::trait_object::{KeyObj, ValueObj};
-use crate::trait_object::base::CloneBox;
 use crate::trait_object::collection::TypeToAnyMap;
 use crate::trait_object::task::TaskObj;
 
@@ -61,7 +60,7 @@ impl<'p, 's> BottomUpContext<'p, 's> {
 
   /// Execute task `node` and potentially schedule new tasks based on the dependencies of the task.
   fn execute_and_schedule(&mut self, node: TaskNode) -> Box<dyn ValueObj> {
-    let task = self.session.store.get_task(&node).clone_box();
+    let task = self.session.store.get_task(&node).to_owned();
     let output = self.execute_obj(task.as_ref(), node);
 
     // Schedule tasks affected by task `node`'s resource writes.

pie/src/dependency.rs

@@ -16,7 +16,6 @@ pub struct TaskDependency<T, C, S> {
   checker: C,
   stamp: S,
 }
-
 impl<T: Task, C: OutputChecker<T::Output>> TaskDependency<T, C, C::Stamp> {
   #[inline]
   pub fn new(task: T, checker: C, stamp: C::Stamp) -> Self { Self { task, checker, stamp } }
@@ -29,7 +28,7 @@ impl<T: Task, C: OutputChecker<T::Output>> TaskDependency<T, C, C::Stamp> {
   pub fn stamp(&self) -> &C::Stamp { &self.stamp }
 
   #[inline]
-  pub fn check<'i>(&'i self, output: &'i T::Output) -> Option<C::Inconsistency<'i>> {
+  pub fn check<'i>(&'i self, output: &'i T::Output) -> Option<impl Debug + 'i> {
     self.checker.check(output, &self.stamp)
   }
 
@@ -48,7 +47,7 @@ pub trait TaskDependencyObj: DynClone + Debug {
   fn as_top_down_check(&self) -> &dyn TopDownCheck;
   fn is_consistent_bottom_up(&self, output: &dyn ValueObj, requiring_task: &dyn KeyObj, tracker: &mut Tracking) -> bool;
 }
-
+const_assert_object_safe!(dyn TaskDependencyObj);
 impl<T: Task, C: OutputChecker<T::Output>> TaskDependencyObj for TaskDependency<T, C, C::Stamp> {
   #[inline]
   fn task(&self) -> &dyn KeyObj { &self.task as &dyn KeyObj }
@@ -90,7 +89,6 @@ pub struct ResourceDependency<R, C, S> {
   checker: C,
   stamp: S,
 }
-
 impl<R: Resource, C: ResourceChecker<R>> ResourceDependency<R, C, C::Stamp> {
   #[inline]
   pub fn new(resource: R, checker: C, stamp: C::Stamp) -> Self { Self { resource, checker, stamp } }
@@ -106,7 +104,7 @@ impl<R: Resource, C: ResourceChecker<R>> ResourceDependency<R, C, C::Stamp> {
   pub fn check<'i, RS: ResourceState<R>>(
     &'i self,
     state: &'i mut RS,
-  ) -> Result<Option<C::Inconsistency<'i>>, C::Error> {
+  ) -> Result<Option<impl Debug + 'i>, C::Error> {
     self.checker.check(&self.resource, state, &self.stamp)
   }
 
@@ -151,6 +149,7 @@ pub trait ResourceDependencyObj: DynClone + Debug {
     tracker: &mut Tracking,
   ) -> Result<bool, Box<dyn Error>>;
 }
+const_assert_object_safe!(dyn ResourceDependencyObj);
 impl<R: Resource, C: ResourceChecker<R>> ResourceDependencyObj for ResourceDependency<R, C, C::Stamp> {
   #[inline]
   fn resource(&self) -> &dyn KeyObj { &self.resource as &dyn KeyObj }

pie/src/lib.rs

@@ -1,14 +1,14 @@
 //! # Trait bounds
 //!
-//! [`Task`] and [`Resource`] are bounded by [`KeyBounds`] so that we can store types of those traits as a key in a
+//! [`Task`] and [`Resource`] are bounded by [`Key`] so that we can store types of those traits as a key in a
 //! hashmap in trait object form. We need to store these types under a trait object to support arbitrary task and
 //! resource types. We also need to store an additional clone for a reverse hashmap.
 //!
-//! [`OutputChecker`] and [`ResourceChecker`] are also bounded by [`KeyBounds`], because types of these traits may be
-//! used as values in tasks, which would require them to be bounded by [`KeyBounds`] anyway. This reduces boilerplate in
-//! tasks that are generic over [`OutputChecker`] and [`ResourceChecker`], as the [`KeyBounds`] bound can be omitted.
+//! [`OutputChecker`] and [`ResourceChecker`] are also bounded by [`Key`], because types of these traits may be
+//! used as values in tasks, which would require them to be bounded by [`Key`] anyway. This reduces boilerplate in
+//! tasks that are generic over [`OutputChecker`] and [`ResourceChecker`], as the [`Key`] bound can be omitted.
 //!
-//! [`Task::Output`], [`OutputChecker::Stamp`], and [`ResourceChecker::Stamp`] are bounded by [`ValueBounds`] because
+//! [`Task::Output`], [`OutputChecker::Stamp`], and [`ResourceChecker::Stamp`] are bounded by [`Value`] because
 //! we need to store (cache) these values. We need to store these values for an indeterminate time, so non-`'static`
 //! references are ruled out. We need to clone outputs to store them. When checking dependencies, we need to clone the
 //! dependencies (due to lifetime/borrow complications). Since these types are used in dependencies, that is another
@@ -27,6 +27,7 @@ use crate::trait_object::KeyObj;
 pub mod task;
 pub mod resource;
 pub mod tracker;
+#[macro_use]
 pub mod trait_object;
 
 mod pie;
@@ -36,18 +37,18 @@ mod dependency;
 
 /// Trait alias for types that are used as values: types that can be cloned, debug formatted, and contain no
 /// non-`'static` references. We use this as an alias for trait bounds and super-traits.
-pub trait ValueBounds: Clone + Debug + 'static {}
-impl<T: Clone + Debug + 'static> ValueBounds for T {}
+pub trait Value: Clone + Debug + 'static {}
+impl<T: Clone + Debug + 'static> Value for T {}
 
-/// Trait alias for types that are used as keys: types that can be cloned, equality compared, hashed, debug formatted,
-/// and contain no non-`'static` references. We use this as an alias for trait bounds and super-traits.
-pub trait KeyBounds: ValueBounds + Eq + Hash {}
-impl<T: ValueBounds + Eq + Hash> KeyBounds for T {}
+/// Trait alias for types that are used as keys: types that are [values](Value) and that can be equality compared and
+/// hashed. We use this as an alias for trait bounds and super-traits.
+pub trait Key: Value + Eq + Hash {}
+impl<T: Value + Eq + Hash> Key for T {}
 
 /// A unit of computation in a programmatic incremental build system.
-pub trait Task: KeyBounds {
+pub trait Task: Key {
   /// Type of task outputs.
-  type Output: ValueBounds;
+  type Output: Value;
 
   /// Execute the task under `context`, returning an output.
   fn execute<C: Context>(&self, context: &mut C) -> Self::Output;
@@ -97,25 +98,22 @@ pub trait Context {
     H: ResourceChecker<R>;
 }
 
-/// Consistency checker for task outputs, producing and checking output stamps. For example, the equals checker uses the
-/// output of a task as stamp, and checks whether they are equal.
-pub trait OutputChecker<O>: KeyBounds {
+/// Consistency checker for task outputs of type `O`, producing and checking output stamps. For example, the
+/// [equals checker](task::EqualsChecker) uses the output of a task as stamp, and checks whether they are equal.
+pub trait OutputChecker<O>: Key {
   /// Type of stamps.
-  type Stamp: ValueBounds;
+  type Stamp: Value;
   /// Stamps `output`.
   fn stamp(&self, output: &O) -> Self::Stamp;
 
-  /// Type of inconsistency used for debugging/logging purposes. The `'i` lifetime represents this checker and the
-  /// lifetime of the `output` and `stamp` passed to [check](Self::check).
-  type Inconsistency<'i>: Debug where O: 'i;
   /// Checks whether `output` is inconsistent w.r.t. `stamp`, returning `Some(inconsistency)` if inconsistent, `None` if
-  /// consistent.
-  fn check<'i>(&'i self, output: &'i O, stamp: &'i Self::Stamp) -> Option<Self::Inconsistency<'i>>;
+  /// consistent. The returned inconsistency can be used for debugging purposes, such as logging what has changed.
+  fn check(&self, output: &O, stamp: &Self::Stamp) -> Option<impl Debug>;
 }
 
 
 /// A resource representing global (mutable) state, such as a path identifying a file on a filesystem.
-pub trait Resource: KeyBounds {
+pub trait Resource: Key {
   /// Type of readers returned from [read](Self::read), with `'rs` representing the lifetime of the resource state.
   type Reader<'rs>;
   /// Type of writers returned from [write](Self::write), with `'r` representing the lifetime of this resource.
@@ -160,9 +158,9 @@ pub trait ResourceState<R> {
 /// Consistency checker for resources, producing and checking resource stamps. For example, for filesystem resources, a
 /// last modified checker creates last modified stamps and checks whether they have changed, and a hash checker creates
 /// file content hash stamps and checks whether they have changed.
-pub trait ResourceChecker<R: Resource>: KeyBounds {
+pub trait ResourceChecker<R: Resource>: Key {
   /// Type of stamps returned from stamp methods.
-  type Stamp: ValueBounds;
+  type Stamp: Value;
   /// Type of errors returned from all methods.
   type Error: Error;
 
@@ -187,17 +185,15 @@ pub trait ResourceChecker<R: Resource>: KeyBounds {
   /// consistent with `resource`.
   fn stamp_writer(&self, resource: &R, writer: R::Writer<'_>) -> Result<Self::Stamp, Self::Error>;
 
-  /// Type of inconsistency used for debugging/logging purposes. The `'i` lifetime represents this checker and the
-  /// lifetime of the `resource`, `state`, and `stamp` passed to [Self::check].
-  type Inconsistency<'i>: Debug;
   /// Checks whether `resource` is inconsistent w.r.t. `stamp`, with access to `state`. Returns `Some(inconsistency)`
-  /// when inconsistent, `None` when consistent.
-  fn check<'i, RS: ResourceState<R>>(
-    &'i self,
-    resource: &'i R,
-    state: &'i mut RS,
-    stamp: &'i Self::Stamp,
-  ) -> Result<Option<Self::Inconsistency<'i>>, Self::Error>;
+  /// when inconsistent, `None` when consistent. The returned inconsistency can be used for debugging purposes, such as
+  /// logging what has changed.
+  fn check<RS: ResourceState<R>>(
+    &self,
+    resource: &R,
+    state: &mut RS,
+    stamp: &Self::Stamp,
+  ) -> Result<Option<impl Debug>, Self::Error>;
 
   /// Wraps a [resource `error`](Resource::Error) into [`Self::Error`].
   fn wrap_error(&self, error: R::Error) -> Self::Error;

pie/src/resource/file.rs

@@ -1,6 +1,6 @@
 use std::convert::Infallible;
 use std::error::Error;
-use std::fmt::{Display, Formatter};
+use std::fmt::{Debug, Display, Formatter};
 use std::fs::{self, File, Metadata, OpenOptions};
 use std::io::{self, BufReader, Seek};
 use std::path::{Path, PathBuf};
@@ -232,7 +232,7 @@ impl From<FsError> for io::Error {
 }
 impl Display for FsError {
   #[inline]
-  fn fmt(&self, f: &mut Formatter<'_>) -> std::fmt::Result { self.0.fmt(f) }
+  fn fmt(&self, f: &mut Formatter<'_>) -> std::fmt::Result { std::fmt::Display::fmt(&self.0, f) }
 }
 
 
@@ -264,8 +264,8 @@ impl ResourceChecker<PathBuf> for ModifiedChecker {
     Ok(Some(file.metadata()?.modified()?))
   }
 
-  type Inconsistency<'i> = Self::Stamp;
   #[inline]
+  #[allow(refining_impl_trait)]
   fn check<RS: ResourceState<PathBuf>>(
     &self,
     path: &PathBuf,
@@ -310,8 +310,8 @@ impl ResourceChecker<PathBuf> for ExistsChecker {
     Ok(exists)
   }
 
-  type Inconsistency<'i> = Self::Stamp;
   #[inline]
+  #[allow(refining_impl_trait)]
   fn check<RS: ResourceState<PathBuf>>(
     &self,
     path: &PathBuf,

pie/src/resource/file/hash_checker.rs

@@ -1,3 +1,4 @@
+use std::fmt::Debug;
 use std::io::Seek;
 
 use sha2::{Digest, Sha256};
@@ -34,8 +35,8 @@ impl ResourceChecker<PathBuf> for HashChecker {
     Ok(Some(hash))
   }
 
-  type Inconsistency<'i> = Self::Stamp;
   #[inline]
+  #[allow(refining_impl_trait)]
   fn check<RS: ResourceState<PathBuf>>(
     &self,
     path: &PathBuf,

pie/src/resource/map.rs

@@ -7,12 +7,12 @@ use std::hash::Hash;
 
 use dyn_clone::DynClone;
 
-use crate::{KeyBounds, Resource, ResourceChecker, ResourceState};
+use crate::{Key, Resource, ResourceChecker, ResourceState};
 use crate::trait_object::base::{AsAny, EqObj};
 use crate::trait_object::KeyObj;
 
 /// Key of globally shared [hash maps](HashMap) from [`Self`] to [`Self::Value`].
-pub trait MapKey: KeyBounds {
+pub trait MapKey: Key {
   type Value;
 }
 impl<K: MapKey> Resource for K {
@@ -99,14 +99,13 @@ impl<K: MapKey> ResourceChecker<K> for MapEqualsChecker where
     Ok(value)
   }
 
-  type Inconsistency<'i> = Option<&'i K::Value>;
   #[inline]
-  fn check<'i, RS: ResourceState<K>>(
+  fn check<RS: ResourceState<K>>(
     &self,
     key: &K,
-    state: &'i mut RS,
+    state: &mut RS,
     stamp: &Self::Stamp,
-  ) -> Result<Option<Self::Inconsistency<'i>>, Self::Error> {
+  ) -> Result<Option<impl Debug>, Self::Error> {
     let value = key.read(state)?;
     let inconsistency = if value != stamp.as_ref() {
       Some(value)
@@ -124,7 +123,7 @@ impl<K: MapKey> ResourceChecker<K> for MapEqualsChecker where
 #[derive(Copy, Clone, Ord, PartialOrd, Eq, PartialEq, Hash, Debug)]
 #[repr(transparent)]
 pub struct MapKeyToObj<K>(pub K);
-impl<K: KeyBounds> MapKey for MapKeyToObj<K> {
+impl<K: Key> MapKey for MapKeyToObj<K> {
   type Value = Box<dyn MapValueObj>;
 }
 impl<K> MapKeyToObj<K> {

pie/src/store.rs

@@ -5,7 +5,6 @@ use pie_graph::{DAG, Node};
 
 use crate::dependency::{Dependency, ResourceDependencyObj, TaskDependencyObj};
 use crate::trait_object::{KeyObj, ValueObj};
-use crate::trait_object::base::CloneBox;
 use crate::trait_object::task::TaskObj;
 
 pub struct Store {
@@ -57,11 +56,11 @@ impl Store {
       *node
     } else {
       let node = self.graph.add_node(NodeData::Task {
-        task: task.clone_box(),
+        task: task.to_owned(),
         output: None,
       });
       let node = TaskNode(node);
-      self.task_to_node.insert(task.clone_box(), node);
+      self.task_to_node.insert(task.to_owned(), node);
       node
     }
   }
@@ -85,9 +84,9 @@ impl Store {
     if let Some(node) = self.resource_to_node.get(resource) {
       *node
     } else {
-      let node = self.graph.add_node(NodeData::Resource(resource.clone_box()));
+      let node = self.graph.add_node(NodeData::Resource(resource.to_owned()));
       let node = ResourceNode(node);
-      self.resource_to_node.insert(resource.clone_box(), node);
+      self.resource_to_node.insert(resource.to_owned(), node);
       node
     }
   }