Skip to content

Commit

Permalink
feat!: Replace GATs with impl Iterator returns (RPITIT) on `HugrVie…
Browse files Browse the repository at this point in the history
…w` (#1660)

This will simplify the update to the next portgraph release (which also
moved to RPITIT), avoiding the need to use boxes around iterators to be
able to name them.

- drops the dependency on `context-iterators`
- drive-by: Cleanup the implementations of `SiblingGraph` / `SiblingMut`
to avoid `collect_vec().into_iter()`s.

BREAKING CHANGE: Removed `HugrView` associated iterator types, replaced
with `impl Iterator` returns.
  • Loading branch information
aborgna-q authored and ss2165 committed Nov 22, 2024
1 parent 0243af4 commit 3bba01a
Show file tree
Hide file tree
Showing 16 changed files with 366 additions and 336 deletions.
1 change: 0 additions & 1 deletion Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,6 @@ portgraph = { version = "0.12.2" }
insta = { version = "1.34.0" }
bitvec = "1.0.1"
cgmath = "0.18.0"
context-iterators = "0.2.0"
cool_asserts = "2.0.3"
criterion = "0.5.1"
delegate = "0.13.0"
Expand Down
1 change: 0 additions & 1 deletion hugr-core/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -47,7 +47,6 @@ bitvec = { workspace = true, features = ["serde"] }
enum_dispatch = { workspace = true }
lazy_static = { workspace = true }
petgraph = { workspace = true }
context-iterators = { workspace = true }
serde_json = { workspace = true }
delegate = { workspace = true }
paste = { workspace = true }
Expand Down
22 changes: 11 additions & 11 deletions hugr-core/src/export.rs
Original file line number Diff line number Diff line change
Expand Up @@ -69,7 +69,7 @@ impl<'a> Context<'a> {
/// Exports the root module of the HUGR graph.
pub fn export_root(&mut self) {
let hugr_children = self.hugr.children(self.hugr.root());
let mut children = Vec::with_capacity(hugr_children.len());
let mut children = Vec::with_capacity(hugr_children.size_hint().0);

for child in self.hugr.children(self.hugr.root()) {
children.push(self.export_node(child));
Expand Down Expand Up @@ -110,7 +110,7 @@ impl<'a> Context<'a> {
num_ports: usize,
) -> &'a [model::LinkRef<'a>] {
let ports = self.hugr.node_ports(node, direction);
let mut links = BumpVec::with_capacity_in(ports.len(), self.bump);
let mut links = BumpVec::with_capacity_in(ports.size_hint().0, self.bump);

for port in ports.take(num_ports) {
links.push(model::LinkRef::Id(self.get_link_id(node, port)));
Expand Down Expand Up @@ -579,7 +579,7 @@ impl<'a> Context<'a> {
let targets = self.make_ports(output_node, Direction::Incoming, output_op.types.len());

// Export the remaining children of the node.
let mut region_children = BumpVec::with_capacity_in(children.len(), self.bump);
let mut region_children = BumpVec::with_capacity_in(children.size_hint().0, self.bump);

for child in children {
region_children.push(self.export_node(child));
Expand Down Expand Up @@ -609,7 +609,7 @@ impl<'a> Context<'a> {
/// Creates a control flow region from the given node's children.
pub fn export_cfg(&mut self, node: Node) -> model::RegionId {
let mut children = self.hugr.children(node);
let mut region_children = BumpVec::with_capacity_in(children.len() + 1, self.bump);
let mut region_children = BumpVec::with_capacity_in(children.size_hint().0 + 1, self.bump);

// The first child is the entry block.
// We create a source port on the control flow region and connect it to the
Expand All @@ -623,16 +623,16 @@ impl<'a> Context<'a> {
let source = model::LinkRef::Id(self.get_link_id(entry_block, IncomingPort::from(0)));
region_children.push(self.export_node(entry_block));

// Export the remaining children of the node, except for the last one.
for _ in 0..children.len() - 1 {
region_children.push(self.export_node(children.next().unwrap()));
}

// The last child is the exit block.
// Contrary to the entry block, the exit block does not have a dataflow subgraph.
// We therefore do not export the block itself, but simply use its output ports
// as the target ports of the control flow region.
let exit_block = children.next().unwrap();
let exit_block = children.next_back().unwrap();

// Export the remaining children of the node, except for the last one.
for child in children {
region_children.push(self.export_node(child));
}

let OpType::ExitBlock(_) = self.hugr.get_optype(exit_block) else {
panic!("expected an `ExitBlock` node as the last child node");
Expand All @@ -657,7 +657,7 @@ impl<'a> Context<'a> {
/// Export the `Case` node children of a `Conditional` node as data flow regions.
pub fn export_conditional_regions(&mut self, node: Node) -> &'a [model::RegionId] {
let children = self.hugr.children(node);
let mut regions = BumpVec::with_capacity_in(children.len(), self.bump);
let mut regions = BumpVec::with_capacity_in(children.size_hint().0, self.bump);

for child in children {
let OpType::Case(case_op) = self.hugr.get_optype(child) else {
Expand Down
1 change: 1 addition & 0 deletions hugr-core/src/extension/op_def.rs
Original file line number Diff line number Diff line change
Expand Up @@ -436,6 +436,7 @@ impl OpDef {
}

/// Iterate over all miscellaneous data in the [OpDef].
#[allow(unused)] // Unused when no features are enabled
pub(crate) fn iter_misc(&self) -> impl ExactSizeIterator<Item = (&str, &serde_json::Value)> {
self.misc.iter().map(|(k, v)| (k.as_str(), v))
}
Expand Down
12 changes: 6 additions & 6 deletions hugr-core/src/hugr/rewrite/inline_dfg.rs
Original file line number Diff line number Diff line change
Expand Up @@ -208,7 +208,7 @@ mod test {

// Sanity checks
assert_eq!(
outer.children(inner.node()).len(),
outer.children(inner.node()).count(),
if nonlocal { 3 } else { 6 }
); // Input, Output, add; + const, load_const, lift
assert_eq!(find_dfgs(&outer), vec![outer.root(), inner.node()]);
Expand All @@ -217,7 +217,7 @@ mod test {
outer.get_parent(outer.get_parent(add).unwrap()),
outer.get_parent(sub)
);
assert_eq!(outer.nodes().len(), 11); // 6 above + inner DFG + outer (DFG + Input + Output + sub)
assert_eq!(outer.nodes().count(), 11); // 6 above + inner DFG + outer (DFG + Input + Output + sub)
{
// Check we can't inline the outer DFG
let mut h = outer.clone();
Expand All @@ -230,7 +230,7 @@ mod test {

outer.apply_rewrite(InlineDFG(*inner.handle()))?;
outer.validate(&reg)?;
assert_eq!(outer.nodes().len(), 8);
assert_eq!(outer.nodes().count(), 8);
assert_eq!(find_dfgs(&outer), vec![outer.root()]);
let [_lift, add, sub] = extension_ops(&outer).try_into().unwrap();
assert_eq!(outer.get_parent(add), Some(outer.root()));
Expand Down Expand Up @@ -265,8 +265,8 @@ mod test {

let mut h = h.finish_hugr_with_outputs(cx.outputs(), &reg)?;
assert_eq!(find_dfgs(&h), vec![h.root(), swap.node()]);
assert_eq!(h.nodes().len(), 8); // Dfg+I+O, H, CX, Dfg+I+O
// No permutation outside the swap DFG:
assert_eq!(h.nodes().count(), 8); // Dfg+I+O, H, CX, Dfg+I+O
// No permutation outside the swap DFG:
assert_eq!(
h.node_connections(p_h.node(), swap.node())
.collect::<Vec<_>>(),
Expand All @@ -292,7 +292,7 @@ mod test {

h.apply_rewrite(InlineDFG(*swap.handle()))?;
assert_eq!(find_dfgs(&h), vec![h.root()]);
assert_eq!(h.nodes().len(), 5); // Dfg+I+O
assert_eq!(h.nodes().count(), 5); // Dfg+I+O
let mut ops = extension_ops(&h);
ops.sort_by_key(|n| h.num_outputs(*n)); // Put H before CX
let [h_gate, cx] = ops.try_into().unwrap();
Expand Down
4 changes: 2 additions & 2 deletions hugr-core/src/hugr/rewrite/outline_cfg.rs
Original file line number Diff line number Diff line change
Expand Up @@ -453,8 +453,8 @@ mod test {
// `add_hugr_with_wires` does not return an InsertionResult, so recover the nodes manually:
let cfg = cfg.node();
let exit_node = h.children(cfg).nth(1).unwrap();
let tail = h.input_neighbours(exit_node).exactly_one().unwrap();
let head = h.input_neighbours(tail).exactly_one().unwrap();
let tail = h.input_neighbours(exit_node).exactly_one().ok().unwrap();
let head = h.input_neighbours(tail).exactly_one().ok().unwrap();
// Just sanity-check we have the correct nodes
assert!(h.get_optype(exit_node).is_exit_block());
assert_eq!(
Expand Down
Loading

0 comments on commit 3bba01a

Please sign in to comment.