Remove error type bound on Handle::execute() via new variant `Execu…

…teError::Handle`.

This is awkward because handle errors may also end up in the `Check`
variant, but in order to avoid that, we’d have to do one of these:

* Make `ExecuteError` take individual generics for its nested errors.
* Expose `TransactionInUniverse` and use its `Mismatch` type.
* Make `Transaction::check()` have an error variant for handle errors
  when they occur inside any transaction. (That might actually be a
  good idea, to distinguish “transient” from permanent errors.)

All of those are more complex, so we’ll stick with this for now.

all-is-cubes/src/transaction.rs

@@ -5,7 +5,7 @@ use alloc::sync::Arc;
 use core::any::type_name;
 use core::{fmt, mem};
 
-use crate::universe::{Handle, UTransactional, UniverseTransaction};
+use crate::universe::{Handle, HandleError, UTransactional, UniverseTransaction};
 use crate::util::ErrorIfStd;
 
 mod generic;
@@ -218,6 +218,19 @@ pub enum ExecuteError<Txn: Transaction = UniverseTransaction> {
     /// See the documentation of [`Transaction::commit()`] for the unfortunate
     /// implications of this.
     Commit(CommitError),
+
+    /// Executing the transaction required accessing a [`Handle`] that was unavailable.
+    ///
+    /// The [`HandleError`] will include the name of the problematic handle.
+    ///
+    /// This error may be transient, and
+    /// unlike [`ExecuteError::Commit`], does not indicate data corruption,
+    /// but code which triggers it should generally be considered incorrect.
+    ///
+    /// Note that this error is returned by [`Handle::execute()`], but transactions whose
+    /// `check` involves accessing handles will instead produce [`ExecuteError::Check`]s.
+    /// This may change in the future.
+    Handle(HandleError),
 }
 
 // Manual impl required to set proper associated type bounds.
@@ -230,6 +243,7 @@ where
             Self::Merge(e) => Self::Merge(e.clone()),
             Self::Check(e) => Self::Check(e.clone()),
             Self::Commit(e) => Self::Commit(e.clone()),
+            Self::Handle(e) => Self::Handle(e.clone()),
         }
     }
 }
@@ -244,6 +258,7 @@ crate::util::cfg_should_impl_error! {
                 ExecuteError::Merge(e) => e.source(),
                 ExecuteError::Check(e) => e.source(),
                 ExecuteError::Commit(e) => e.source(),
+                ExecuteError::Handle(e) => e.source(),
             }
         }
     }
@@ -258,6 +273,7 @@ where
             Self::Merge(e) => f.debug_tuple("Merge").field(e).finish(),
             Self::Check(e) => f.debug_tuple("Check").field(e).finish(),
             Self::Commit(e) => f.debug_tuple("Commit").field(e).finish(),
+            Self::Handle(e) => f.debug_tuple("Handle").field(e).finish(),
         }
     }
 }
@@ -271,6 +287,7 @@ where
             ExecuteError::Merge(e) => e.fmt(f),
             ExecuteError::Check(e) => e.fmt(f),
             ExecuteError::Commit(e) => e.fmt(f),
+            ExecuteError::Handle(e) => e.fmt(f),
         }
     }
 }

all-is-cubes/src/universe/handle.rs

@@ -216,33 +216,27 @@ impl<T: 'static> Handle<T> {
 
     /// Execute the given transaction on the referent.
     ///
-    /// Returns an error if the transaction's preconditions were not met, if the
-    /// referent was already borrowed (which is denoted as an [`ExecuteError::Check`]),
-    /// or if the transaction encountered an unexpected error.
+    /// Returns an error if the transaction's preconditions are not met,
+    /// if the transaction encountered an internal error, or if the referent
+    /// was already being read or written (which is expressed as an
+    /// [`ExecuteError::Commit`], because it is a shouldn’t-happen kind of error).
     #[inline(never)]
     pub fn execute(
         &self,
         transaction: &<T as Transactional>::Transaction,
     ) -> Result<(), ExecuteError<<T as Transactional>::Transaction>>
     where
         T: Transactional,
-        // TODO: relax `Mismatch` bound and use a wrapper type instead, when custom error types are actually in use
-        //
         // `Output = NoOutput` is required because, if there *were* outputs,
         // they would need to be directed to some destination in the `Universe`,
         // not the caller.
-        T::Transaction: Transaction<Mismatch = PreconditionFailed, Output = transaction::NoOutput>,
+        T::Transaction: Transaction<Output = transaction::NoOutput>,
     {
         let outcome: Result<
             Result<(), ExecuteError<<T as Transactional>::Transaction>>,
             HandleError,
         > = self.try_modify(|data| transaction.execute(data, &mut transaction::no_outputs));
-        outcome.map_err(|_| {
-            ExecuteError::Check(PreconditionFailed {
-                location: "Handle::execute()",
-                problem: "target is currently in use",
-            })
-        })?
+        outcome.map_err(ExecuteError::Handle)?
     }
 
     fn upgrade(&self) -> Result<StrongEntryRef<T>, HandleError> {