Check for acyclicity; remove boundedness requirement (#468)

Fixes #455 .
Fixes #447 .
Fixes #435 .

src/builder/build_traits.rs

@@ -210,9 +210,8 @@ pub trait Dataflow: Container {
  let num_outputs = hugr.get_optype(hugr.root()).signature().output_count();
  let node = self.add_hugr(hugr)?;
 
- let [inp, _] = self.io();
  let inputs = input_wires.into_iter().collect();
- wire_up_inputs(inputs, node, self, inp)?;
+ wire_up_inputs(inputs, node, self)?;
 
  Ok((node, num_outputs).into())
  }
@@ -232,9 +231,8 @@ pub trait Dataflow: Container {
  let num_outputs = hugr.get_optype(hugr.root()).signature().output_count();
  let node = self.add_hugr_view(hugr)?;
 
- let [inp, _] = self.io();
  let inputs = input_wires.into_iter().collect();
- wire_up_inputs(inputs, node, self, inp)?;
+ wire_up_inputs(inputs, node, self)?;
 
  Ok((node, num_outputs).into())
  }
@@ -248,8 +246,8 @@ pub trait Dataflow: Container {
  &mut self,
  output_wires: impl IntoIterator<Item = Wire>,
  ) -> Result<(), BuildError> {
- let [inp, out] = self.io();
- wire_up_inputs(output_wires.into_iter().collect_vec(), out, self, inp)
+ let [_, out] = self.io();
+ wire_up_inputs(output_wires.into_iter().collect_vec(), out, self)
  }
 
  /// Return an array of the input wires.
@@ -605,12 +603,10 @@ fn add_node_with_wires<T: Dataflow + ?Sized>(
  nodetype: NodeType,
  inputs: Vec<Wire>,
 ) -> Result<(Node, usize), BuildError> {
- let [inp, _] = data_builder.io();
-
  let op_node = data_builder.add_child_node(nodetype.clone())?;
  let sig = nodetype.op_signature();
 
- wire_up_inputs(inputs, op_node, data_builder, inp)?;
+ wire_up_inputs(inputs, op_node, data_builder)?;
 
  Ok((op_node, sig.output().len()))
 }
@@ -619,26 +615,16 @@ fn wire_up_inputs<T: Dataflow + ?Sized>(
  inputs: Vec<Wire>,
  op_node: Node,
  data_builder: &mut T,
- inp: Node,
 ) -> Result<(), BuildError> {
- let mut any_local_df_inputs = false;
  for (dst_port, wire) in inputs.into_iter().enumerate() {
- any_local_df_inputs |= wire_up(
+ wire_up(
  data_builder,
  wire.node(),
  wire.source().index(),
  op_node,
  dst_port,
  )?;
  }
- let base = data_builder.hugr_mut();
- let op = base.get_optype(op_node);
- let some_df_outputs = !op.signature().output.is_empty();
- if !any_local_df_inputs && some_df_outputs {
- // If op has no inputs add a StateOrder edge from input to place in
- // causal cone of Input node
- data_builder.add_other_wire(inp, op_node)?;
- };
  Ok(())
 }
 

src/hugr/validate.rs

@@ -5,7 +5,7 @@ use std::iter;
 
 use itertools::Itertools;
 use petgraph::algo::dominators::{self, Dominators};
-use petgraph::visit::{DfsPostOrder, Walker};
+use petgraph::visit::{Topo, Walker};
 use portgraph::{LinkView, PortView};
 use thiserror::Error;
 
@@ -341,9 +341,7 @@ impl<'a> ValidationContext<'a> {
  Ok(())
  }
 
- /// Ensure that the children of a node form a direct acyclic graph with a
- /// single source and source. That is, their edges do not form cycles in the
- /// graph and there are no dangling nodes.
+ /// Ensure that the children of a node form a directed acyclic graph.
  ///
  /// Inter-graph edges are ignored. Only internal dataflow, constant, or
  /// state order edges are considered.
@@ -354,18 +352,11 @@ impl<'a> ValidationContext<'a> {
  };
 
  let region: SiblingGraph = SiblingGraph::new(self.hugr, parent);
- let entry_node = self.hugr.children(parent).next().unwrap();
-
- let postorder = DfsPostOrder::new(&region, entry_node);
+ let postorder = Topo::new(&region);
  let nodes_visited = postorder.iter(&region).filter(|n| *n != parent).count();
- // Local ScopedDefn's should not be reachable from the Input node, so discount them
- let non_defn_count = self
- .hugr
- .children(parent)
- .filter(|n| !OpTag::ScopedDefn.is_superset(self.hugr.get_optype(*n).tag()))
- .count();
- if nodes_visited != non_defn_count {
- return Err(ValidationError::NotABoundedDag {
+ let node_count = self.hugr.children(parent).count();
+ if nodes_visited != node_count {
+ return Err(ValidationError::NotADag {
  node: parent,
  optype: op_type.clone(),
  });
@@ -603,9 +594,9 @@ pub enum ValidationError {
  /// The node must have children, but has none.
  #[error("The node {node:?} with optype {optype:?} must have children, but has none.")]
  ContainerWithoutChildren { node: Node, optype: OpType },
- /// The children of a node do not form a dag with single source and sink.
- #[error("The children of an operation {optype:?} must form a dag with single source and sink. Loops are not allowed, nor are dangling nodes not in the path between the input and output. In node {node:?}.")]
- NotABoundedDag { node: Node, optype: OpType },
+ /// The children of a node do not form a DAG.
+ #[error("The children of an operation {optype:?} must form a DAG. Loops are not allowed. In node {node:?}.")]
+ NotADag { node: Node, optype: OpType },
  /// There are invalid inter-graph edges.
  #[error(transparent)]
  InterGraphEdgeError(#[from] InterGraphEdgeError),
@@ -699,7 +690,7 @@ mod test {
  use crate::ops::dataflow::IOTrait;
  use crate::ops::{self, LeafOp, OpType};
  use crate::std_extensions::logic;
- use crate::std_extensions::logic::test::and_op;
+ use crate::std_extensions::logic::test::{and_op, not_op};
  use crate::types::{FunctionType, Type};
  use crate::Direction;
  use crate::{type_row, Node};
@@ -1096,10 +1087,7 @@ mod test {
  let lcst = h.add_op_with_parent(h.root(), ops::LoadConstant { datatype: BOOL_T })?;
  h.connect(cst, 0, lcst, 0)?;
  h.connect(lcst, 0, and, 1)?;
- // We are missing the edge from Input to LoadConstant, hence:
- assert_matches!(h.validate(), Err(ValidationError::NotABoundedDag { .. }));
- // Now include the LoadConstant node in the causal cone
- h.add_other_edge(input, lcst)?;
+ // There is no edge from Input to LoadConstant, but that's OK:
  h.validate().unwrap();
  Ok(())
  }
@@ -1256,4 +1244,24 @@ mod test {
  );
  Ok(())
  }
+
+ #[test]
+ fn dfg_with_cycles() -> Result<(), HugrError> {
+ let mut h = Hugr::new(NodeType::pure(ops::DFG {
+ signature: FunctionType::new(type_row![BOOL_T, BOOL_T], type_row![BOOL_T]),
+ }));
+ let input = h.add_op_with_parent(h.root(), ops::Input::new(type_row![BOOL_T, BOOL_T]))?;
+ let output = h.add_op_with_parent(h.root(), ops::Output::new(type_row![BOOL_T]))?;
+ let and = h.add_op_with_parent(h.root(), and_op())?;
+ let not1 = h.add_op_with_parent(h.root(), not_op())?;
+ let not2 = h.add_op_with_parent(h.root(), not_op())?;
+ h.connect(input, 0, and, 0)?;
+ h.connect(and, 0, not1, 0)?;
+ h.connect(not1, 0, and, 1)?;
+ h.connect(input, 1, not2, 0)?;
+ h.connect(not2, 0, output, 0)?;
+ // The graph contains a cycle:
+ assert_matches!(h.validate(), Err(ValidationError::NotADag { .. }));
+ Ok(())
+ }
 }

src/std_extensions/logic.rs

@@ -110,7 +110,7 @@ lazy_static! {
 pub(crate) mod test {
  use crate::{extension::prelude::BOOL_T, ops::LeafOp, types::type_param::TypeArg, Extension};
 
- use super::{extension, AND_NAME, EXTENSION, FALSE_NAME, TRUE_NAME};
+ use super::{extension, AND_NAME, EXTENSION, FALSE_NAME, NOT_NAME, TRUE_NAME};
 
  #[test]
  fn test_logic_extension() {
@@ -131,11 +131,19 @@ pub(crate) mod test {
  }
  }
 
- /// Generate a logic extension and operation over [`crate::prelude::BOOL_T`]
+ /// Generate a logic extension and "and" operation over [`crate::prelude::BOOL_T`]
  pub(crate) fn and_op() -> LeafOp {
  EXTENSION
  .instantiate_extension_op(AND_NAME, [TypeArg::BoundedNat(2)])
  .unwrap()
  .into()
  }
+
+ /// Generate a logic extension and "not" operation over [`crate::prelude::BOOL_T`]
+ pub(crate) fn not_op() -> LeafOp {
+ EXTENSION
+ .instantiate_extension_op(NOT_NAME, [])
+ .unwrap()
+ .into()
+ }
 }