Refactor: move rewrite inside hugr, Rewrite -> Replace implementing new 'Rewrite' trait

src/hugr.rs

@@ -3,6 +3,7 @@
 mod hugrmut;
 
 pub mod multiportgraph;
+pub mod rewrite;
 pub mod serialize;
 pub mod validate;
 pub mod view;
@@ -11,13 +12,14 @@ pub use self::hugrmut::HugrMut;
 pub use self::validate::ValidationError;
 
 use derive_more::From;
+pub use rewrite::{Replace, ReplaceError, Rewrite};
+
 use portgraph::dot::{hier_graph_dot_string_with, DotEdgeStyle};
 use portgraph::{Hierarchy, PortGraph, UnmanagedDenseMap};
 use thiserror::Error;
 
 pub use self::view::HugrView;
 use crate::ops::{ModuleOp, OpType};
-use crate::rewrite::{Rewrite, RewriteError};
 use crate::types::EdgeKind;
 
 use html_escape::encode_text_to_string;
@@ -75,29 +77,8 @@ impl Hugr {
     }
 
     /// Applies a rewrite to the graph.
-    pub fn apply_rewrite(mut self, rewrite: Rewrite) -> Result<(), RewriteError> {
-        // Get the open graph for the rewrites, and a HUGR with the additional components.
-        let (rewrite, mut replacement, parents) = rewrite.into_parts();
-
-        // TODO: Use `parents` to update the hierarchy, and keep the internal hierarchy from `replacement`.
-        let _ = parents;
-
-        let node_inserted = |old, new| {
-            std::mem::swap(&mut self.op_types[new], &mut replacement.op_types[old]);
-            // TODO: metadata (Fn parameter ?)
-        };
-        rewrite.apply_with_callbacks(
-            &mut self.graph,
-            |_| {},
-            |_| {},
-            node_inserted,
-            |_, _| {},
-            |_, _| {},
-        )?;
-
-        // TODO: Check types
-
-        Ok(())
+    pub fn apply_rewrite<E>(&mut self, rw: impl Rewrite<E>) -> Result<(), E> {
+        rw.apply(self)
     }
 
     /// Return dot string showing underlying graph and hierarchy side by side.

src/hugr/rewrite.rs

@@ -0,0 +1,57 @@
+//! Replace operations on the HUGR.
+
+#[allow(clippy::module_inception)] // TODO: Rename?
+pub mod replace;
+use std::mem;
+
+use crate::Hugr;
+pub use replace::{OpenHugr, Replace, ReplaceError};
+
+/// An operation that can be applied to mutate a Hugr
+pub trait Rewrite<E> {
+    /// If `true`, [self.apply]'s of this rewrite guarantee that they do not mutate the Hugr when they return an Err.
+    /// If `false`, there is no guarantee; the Hugr should be assumed invalid when Err is returned.
+    const UNCHANGED_ON_FAILURE: bool;
+
+    /// Checks whether the rewrite would succeed on the specified Hugr.
+    /// If this call succeeds, [self.apply] should also succeed on the same `h`
+    /// If this calls fails, [self.apply] would fail with the same error.
+    fn verify(&self, h: &Hugr) -> Result<(), E>;
+
+    /// Mutate the specified Hugr, or fail with an error.
+    /// If [self.unchanged_on_failure] is true, then `h` must be unchanged if Err is returned.
+    /// See also [self.verify]
+    /// # Panics
+    /// May panic if-and-only-if `h` would have failed [Hugr::validate]; that is,
+    /// implementations may begin with `assert!(h.validate())`, and *should* begin with `debug_assert!(h.validate())`
+    fn apply(self, h: &mut Hugr) -> Result<(), E>;
+}
+
+/// Wraps any rewrite into a transaction (i.e. that has no effect upon failure)
+pub struct Transactional<R> {
+    underlying: R,
+}
+
+// Note we might like to constrain R to Rewrite<unchanged_on_failure=false> but this
+// is not yet supported, https://github.com/rust-lang/rust/issues/92827
+impl<E, R: Rewrite<E>> Rewrite<E> for Transactional<R> {
+    const UNCHANGED_ON_FAILURE: bool = true;
+
+    fn verify(&self, h: &Hugr) -> Result<(), E> {
+        self.underlying.verify(h)
+    }
+
+    fn apply(self, h: &mut Hugr) -> Result<(), E> {
+        if R::UNCHANGED_ON_FAILURE {
+            return self.underlying.apply(h);
+        }
+        // note that if underlying.may_fail_destructively(h) is false, we don't need a backup...
+        let backup = h.clone();
+        let r = self.underlying.apply(h);
+        if r.is_err() {
+            // drop the old h, it was undefined
+            let _ = mem::replace(h, backup);
+        }
+        r
+    }
+}

src/rewrite/rewrite.rs → src/hugr/rewrite/replace.rs

@@ -1,12 +1,14 @@
 #![allow(missing_docs)]
-//! Rewrite operations on Hugr graphs.
+//! Replace operations on Hugr graphs. This is a nonfunctional
+//! dummy implementation just to demonstrate design principles.
 
 use std::collections::HashMap;
 
 use portgraph::substitute::OpenGraph;
 use portgraph::{NodeIndex, PortIndex};
 use thiserror::Error;
 
+use super::Rewrite;
 use crate::Hugr;
 
 /// A subset of the nodes in a graph, and the ports that it is connected to.
@@ -76,7 +78,7 @@ pub type ParentsMap = HashMap<NodeIndex, NodeIndex>;
 /// A rewrite operation that replaces a subgraph with another graph.
 /// Includes the new weights for the nodes in the replacement graph.
 #[derive(Debug, Clone)]
-pub struct Rewrite {
+pub struct Replace {
     /// The subgraph to be replaced.
     subgraph: BoundedSubgraph,
     /// The replacement graph.
@@ -85,7 +87,7 @@ pub struct Rewrite {
     parents: ParentsMap,
 }
 
-impl Rewrite {
+impl Replace {
     /// Creates a new rewrite operation.
     pub fn new(
         subgraph: BoundedSubgraph,
@@ -112,30 +114,62 @@ impl Rewrite {
         )
     }
 
+    pub fn verify_convexity(&self) -> Result<(), ReplaceError> {
+        todo!()
+    }
+
+    pub fn verify_boundaries(&self) -> Result<(), ReplaceError> {
+        todo!()
+    }
+}
+
+impl Rewrite<ReplaceError> for Replace {
+    const UNCHANGED_ON_FAILURE: bool = false;
+
     /// Checks that the rewrite is valid.
     ///
     /// This includes having a convex subgraph (TODO: include definition), and
     /// having matching numbers of ports on the boundaries.
-    pub fn verify(&self) -> Result<(), RewriteError> {
+    /// TODO not clear this implementation really provides much guarantee about [self.apply]
+    /// but this class is not really working anyway.
+    fn verify(&self, _h: &Hugr) -> Result<(), ReplaceError> {
         self.verify_convexity()?;
         self.verify_boundaries()?;
         Ok(())
     }
 
-    pub fn verify_convexity(&self) -> Result<(), RewriteError> {
-        todo!()
-    }
-
-    pub fn verify_boundaries(&self) -> Result<(), RewriteError> {
-        todo!()
+    /// Performs a Replace operation on the graph.
+    fn apply(self, h: &mut Hugr) -> Result<(), ReplaceError> {
+        // Get the open graph for the rewrites, and a HUGR with the additional components.
+        let (rewrite, mut replacement, parents) = self.into_parts();
+
+        // TODO: Use `parents` to update the hierarchy, and keep the internal hierarchy from `replacement`.
+        let _ = parents;
+
+        let node_inserted = |old, new| {
+            std::mem::swap(&mut h.op_types[new], &mut replacement.op_types[old]);
+            // TODO: metadata (Fn parameter ?)
+        };
+        // unchanged_on_failure is false, so no guarantees here
+        rewrite.apply_with_callbacks(
+            &mut h.graph,
+            |_| {},
+            |_| {},
+            node_inserted,
+            |_, _| {},
+            |_, _| {},
+        )?;
+
+        // TODO: Check types
+        Ok(())
     }
 }
 
 /// Error generated when a rewrite fails.
 #[derive(Debug, Clone, Error, PartialEq, Eq)]
-pub enum RewriteError {
-    /// The rewrite failed because the boundary defined by the
-    /// [`Rewrite`] could not be matched to the dangling ports of the
+pub enum ReplaceError {
+    /// The replacement failed because the boundary defined by the
+    /// [`Replace`] could not be matched to the dangling ports of the
     /// [`OpenHugr`].
     #[error("The boundary defined by the rewrite could not be matched to the dangling ports of the OpenHugr")]
     BoundarySize(#[source] portgraph::substitute::RewriteError),
@@ -150,7 +184,7 @@ pub enum RewriteError {
     NotConvex(),
 }
 
-impl From<portgraph::substitute::RewriteError> for RewriteError {
+impl From<portgraph::substitute::RewriteError> for ReplaceError {
     fn from(e: portgraph::substitute::RewriteError) -> Self {
         match e {
             portgraph::substitute::RewriteError::BoundarySize => Self::BoundarySize(e),

src/lib.rs

@@ -16,10 +16,8 @@ pub mod hugr;
 pub mod macros;
 pub mod ops;
 pub mod resource;
-pub mod rewrite;
 pub mod types;
 mod utils;
 
-pub use crate::hugr::{Direction, Hugr, Node, Port, Wire};
+pub use crate::hugr::{Direction, Hugr, Node, Port, Replace, ReplaceError, Wire};
 pub use crate::resource::Resource;
-pub use crate::rewrite::{Rewrite, RewriteError};