From a1ffe6f22f223953cbf72518af6c7468c768723b Mon Sep 17 00:00:00 2001 From: Douglas Wilson Date: Thu, 13 Mar 2025 14:16:29 +0000 Subject: [PATCH 1/2] test: Failing testcase for #1974 --- hugr-core/src/hugr/views/sibling_subgraph.rs | 20 ++++++++++++++++++++ 1 file changed, 20 insertions(+) diff --git a/hugr-core/src/hugr/views/sibling_subgraph.rs b/hugr-core/src/hugr/views/sibling_subgraph.rs index de3bf22c1a..805ca5cdc4 100644 --- a/hugr-core/src/hugr/views/sibling_subgraph.rs +++ b/hugr-core/src/hugr/views/sibling_subgraph.rs @@ -831,6 +831,7 @@ mod tests { use cool_asserts::assert_matches; use crate::builder::inout_sig; + use crate::extension::prelude::Noop; use crate::hugr::Rewrite; use crate::ops::Const; use crate::std_extensions::arithmetic::float_types::{self, ConstF64}; @@ -1239,4 +1240,23 @@ mod tests { let rep = subg.create_simple_replacement(&h, replacement).unwrap(); rep.apply(&mut h).unwrap(); } + + #[test] + #[should_panic] + fn order_edge() { + let (hugr, nop) = { + let mut b = DFGBuilder::new( + Signature::new_endo(Type::UNIT).with_prelude() + ).unwrap(); + let input = b.input(); + let nop = b.add_dataflow_op(Noop(Type::UNIT), [input.out_wire(0)]).unwrap(); + let output = b.output(); + b.add_other_wire(input.node(), nop.node()); + b.add_other_wire(nop.node(), output.node()); + (b.finish_hugr_with_outputs([nop.out_wire(0)]).unwrap(), nop.node()) + }; + + let view = SiblingSubgraph::from_node(nop, &hugr); + assert_eq!(view.signature(&hugr), Signature::new_endo(Type::UNIT).with_prelude()); + } } From 29ca080483e1492c7a86a664472a3415a60dd1a2 Mon Sep 17 00:00:00 2001 From: Seyon Sivarajah Date: Thu, 13 Mar 2025 17:13:50 +0000 Subject: [PATCH 2/2] fix: only allow value edges on SiblingSubgraph::from_node --- hugr-core/src/hugr/views/sibling_subgraph.rs | 47 +++++++++++--------- 1 file changed, 26 insertions(+), 21 deletions(-) diff --git a/hugr-core/src/hugr/views/sibling_subgraph.rs b/hugr-core/src/hugr/views/sibling_subgraph.rs index 805ca5cdc4..3bd2ae048f 100644 --- a/hugr-core/src/hugr/views/sibling_subgraph.rs +++ b/hugr-core/src/hugr/views/sibling_subgraph.rs @@ -276,25 +276,24 @@ impl SiblingSubgraph { /// /// The subgraph signature will be given by signature of the node. pub fn from_node(node: N, hugr: &impl HugrView) -> Self { + let is_value_port = |p: Port| -> bool { + hugr.get_optype(node) + .port_kind(p) + .is_some_and(|k| k.is_value()) + }; + let nodes = vec![node]; let inputs = hugr .node_inputs(node) - .filter(|&p| hugr.is_linked(node, p)) - .map(|p| vec![(node, p)]) + // accept linked value inputs + .filter_map(|p| { + (hugr.is_linked(node, p) && is_value_port(p.into())).then_some(vec![(node, p)]) + }) .collect_vec(); let outputs = hugr .node_outputs(node) - .filter_map(|p| { - // accept linked outputs or unlinked value outputs - { - hugr.is_linked(node, p) - || hugr - .get_optype(node) - .port_kind(p) - .is_some_and(|k| k.is_value()) - } - .then_some((node, p)) - }) + // accept linked or unlinked value outputs + .filter_map(|p| is_value_port(p.into()).then_some((node, p))) .collect_vec(); Self { @@ -1242,21 +1241,27 @@ mod tests { } #[test] - #[should_panic] + // #[should_panic] fn order_edge() { let (hugr, nop) = { - let mut b = DFGBuilder::new( - Signature::new_endo(Type::UNIT).with_prelude() - ).unwrap(); + let mut b = DFGBuilder::new(Signature::new_endo(Type::UNIT).with_prelude()).unwrap(); let input = b.input(); - let nop = b.add_dataflow_op(Noop(Type::UNIT), [input.out_wire(0)]).unwrap(); + let nop = b + .add_dataflow_op(Noop(Type::UNIT), [input.out_wire(0)]) + .unwrap(); let output = b.output(); b.add_other_wire(input.node(), nop.node()); b.add_other_wire(nop.node(), output.node()); - (b.finish_hugr_with_outputs([nop.out_wire(0)]).unwrap(), nop.node()) + ( + b.finish_hugr_with_outputs([nop.out_wire(0)]).unwrap(), + nop.node(), + ) }; - + println!("{}", hugr.mermaid_string()); let view = SiblingSubgraph::from_node(nop, &hugr); - assert_eq!(view.signature(&hugr), Signature::new_endo(Type::UNIT).with_prelude()); + assert_eq!( + view.signature(&hugr), + Signature::new_endo(Type::UNIT).with_prelude() + ); } }