From 2f572aa40ae5b0cf1522d1cd9367f69aaf26a7c6 Mon Sep 17 00:00:00 2001
From: Alan Lawrence <alan.lawrence@cambridgequantum.com>
Date: Mon, 9 Oct 2023 11:43:12 +0100
Subject: [PATCH 1/3] Add test of broken SiblingMut covariance

---
 src/hugr/views/sibling.rs | 31 ++++++++++++++++++++++++++++++-
 1 file changed, 30 insertions(+), 1 deletion(-)

diff --git a/src/hugr/views/sibling.rs b/src/hugr/views/sibling.rs
index dcb773efb..853617942 100644
--- a/src/hugr/views/sibling.rs
+++ b/src/hugr/views/sibling.rs
@@ -366,7 +366,7 @@ mod test {
     use crate::builder::{Container, Dataflow, DataflowSubContainer, HugrBuilder, ModuleBuilder};
     use crate::extension::PRELUDE_REGISTRY;
     use crate::hugr::NodeType;
-    use crate::ops::handle::{CfgID, DfgID, FuncID, ModuleRootID};
+    use crate::ops::handle::{CfgID, DataflowParentID, DfgID, FuncID, ModuleRootID};
     use crate::ops::{dataflow::IOTrait, Input, OpTag, Output};
     use crate::type_row;
     use crate::types::{FunctionType, Type};
@@ -462,4 +462,33 @@ mod test {
         simple_dfg_hugr.replace_op(root, bad_nodetype).unwrap();
         assert!(simple_dfg_hugr.validate(&PRELUDE_REGISTRY).is_err());
     }
+
+    #[rstest]
+    //#[should_panic] // See commented out line in test
+    fn sibling_mut_covariance(mut simple_dfg_hugr: Hugr) {
+        let root = simple_dfg_hugr.root();
+        let case_nodetype = NodeType::open_extensions(crate::ops::Case {
+            signature: simple_dfg_hugr.root_type().op_signature(),
+        });
+        let mut sib_mut = SiblingMut::<DfgID>::try_new(&mut simple_dfg_hugr, root).unwrap();
+        // As expected, we cannot replace the root with a Case
+        assert_eq!(
+            sib_mut.replace_op(root, case_nodetype.clone()),
+            Err(HugrError::InvalidTag {
+                required: OpTag::Dfg,
+                actual: OpTag::Case
+            })
+        );
+
+        let mut nested_sib_mut =
+            SiblingMut::<DataflowParentID>::try_new(&mut sib_mut, root).unwrap();
+        nested_sib_mut
+            .replace_op(root, case_nodetype.clone())
+            .unwrap();
+        // The following line panics:
+        // assert_eq!(sib_mut.root_type(), &case_nodetype);
+        // But it'd have been much better to have stopped things getting to this point,
+        // is if we don't call root_type we can just carry on:
+        assert!(!DfgID::TAG.is_superset(case_nodetype.tag()));
+    }
 }

From 4242c657afdead543762d03ac5f75a2ce27ef6e3 Mon Sep 17 00:00:00 2001
From: Alan Lawrence <alan.lawrence@cambridgequantum.com>
Date: Mon, 9 Oct 2023 11:43:49 +0100
Subject: [PATCH 2/3] Check in SiblingMut::try_new, update test

---
 src/hugr/views/sibling.rs | 20 +++++++++-----------
 1 file changed, 9 insertions(+), 11 deletions(-)

diff --git a/src/hugr/views/sibling.rs b/src/hugr/views/sibling.rs
index 853617942..f36089c2c 100644
--- a/src/hugr/views/sibling.rs
+++ b/src/hugr/views/sibling.rs
@@ -246,7 +246,13 @@ pub struct SiblingMut<'g, Root = Node> {
 impl<'g, Root: NodeHandle> SiblingMut<'g, Root> {
     /// Create a new SiblingMut from a base.
     /// Equivalent to [HierarchyView::try_new] but takes a *mutable* reference.
-    pub fn try_new(hugr: &'g mut impl HugrMut, root: Node) -> Result<Self, HugrError> {
+    pub fn try_new<Base: HugrMut>(hugr: &'g mut Base, root: Node) -> Result<Self, HugrError> {
+        if root == hugr.root() && !Base::RootHandle::TAG.is_superset(Root::TAG) {
+            return Err(HugrError::InvalidTag {
+                required: Base::RootHandle::TAG,
+                actual: Root::TAG,
+            });
+        }
         check_tag::<Root>(hugr, root)?;
         Ok(Self {
             hugr: hugr.hugr_mut(),
@@ -480,15 +486,7 @@ mod test {
             })
         );
 
-        let mut nested_sib_mut =
-            SiblingMut::<DataflowParentID>::try_new(&mut sib_mut, root).unwrap();
-        nested_sib_mut
-            .replace_op(root, case_nodetype.clone())
-            .unwrap();
-        // The following line panics:
-        // assert_eq!(sib_mut.root_type(), &case_nodetype);
-        // But it'd have been much better to have stopped things getting to this point,
-        // is if we don't call root_type we can just carry on:
-        assert!(!DfgID::TAG.is_superset(case_nodetype.tag()));
+        let nested_sib_mut = SiblingMut::<DataflowParentID>::try_new(&mut sib_mut, root);
+        assert!(nested_sib_mut.is_err());
     }
 }

From c0bb4c07cd9f34954d1f835c32c93ffd47cd7d6b Mon Sep 17 00:00:00 2001
From: Alan Lawrence <alan.lawrence@cambridgequantum.com>
Date: Mon, 9 Oct 2023 12:12:46 +0100
Subject: [PATCH 3/3] Remove obsolete comment

---
 src/hugr/views/sibling.rs | 1 -
 1 file changed, 1 deletion(-)

diff --git a/src/hugr/views/sibling.rs b/src/hugr/views/sibling.rs
index f36089c2c..9237046b8 100644
--- a/src/hugr/views/sibling.rs
+++ b/src/hugr/views/sibling.rs
@@ -470,7 +470,6 @@ mod test {
     }
 
     #[rstest]
-    //#[should_panic] // See commented out line in test
     fn sibling_mut_covariance(mut simple_dfg_hugr: Hugr) {
         let root = simple_dfg_hugr.root();
         let case_nodetype = NodeType::open_extensions(crate::ops::Case {