From 8dc94f8df7c88689c9a437e5870b5fa0cd9abbd4 Mon Sep 17 00:00:00 2001 From: Alan Lawrence Date: Tue, 12 Sep 2023 12:56:15 +0100 Subject: [PATCH] Implement HugrView::get_{parent,{node,op}type,metadata} once only, with valid_node checks ==> SiblingGraph::new's "get_optype" will implicitly check the root is in the base view. This does mean some unnecessary index checking for &Hugr's. We could continue to reimplement those methods for AsRef if we want to avoid this inefficiency. Alternatively we could make UnmanagedDenseMap return Options rather than unwrapping. Some other ugly alternative/hacks for returning default values - DEFAULT_{NODE,OP}TYPE --- src/hugr.rs | 8 ++++- src/hugr/views.rs | 56 +++++++++++++++++++---------------- src/hugr/views/descendants.rs | 27 ++--------------- src/hugr/views/sibling.rs | 24 ++------------- src/ops.rs | 5 +++- 5 files changed, 46 insertions(+), 74 deletions(-) diff --git a/src/hugr.rs b/src/hugr.rs index f697c63b73..a3ab615423 100644 --- a/src/hugr.rs +++ b/src/hugr.rs @@ -29,7 +29,7 @@ pub use self::views::HugrView; use crate::extension::{ infer_extensions, ExtensionRegistry, ExtensionSet, ExtensionSolution, InferExtensionError, }; -use crate::ops::{OpTag, OpTrait, OpType}; +use crate::ops::{OpTag, OpTrait, OpType, DEFAULT_OPTYPE}; use crate::types::{FunctionType, Signature}; use delegate::delegate; @@ -64,6 +64,12 @@ pub struct NodeType { input_extensions: Option, } +/// The default NodeType, with open extensions +pub const DEFAULT_NODETYPE: NodeType = NodeType { + op: DEFAULT_OPTYPE, + input_extensions: None, // Default for any Option +}; + impl NodeType { /// Create a new optype with some ExtensionSet pub fn new(op: impl Into, input_extensions: impl Into>) -> Self { diff --git a/src/hugr/views.rs b/src/hugr/views.rs index 4c533fa4fa..841b972400 100644 --- a/src/hugr/views.rs +++ b/src/hugr/views.rs @@ -18,7 +18,7 @@ use itertools::{Itertools, MapInto}; use portgraph::dot::{DotFormat, EdgeStyle, NodeStyle, PortStyle}; use portgraph::{multiportgraph, LinkView, MultiPortGraph, PortView}; -use super::{Hugr, HugrError, NodeMetadata, NodeType}; +use super::{Hugr, HugrError, NodeMetadata, NodeType, DEFAULT_NODETYPE}; use crate::ops::handle::NodeHandle; use crate::ops::{FuncDecl, FuncDefn, OpName, OpTag, OpType, DFG}; use crate::types::{EdgeKind, FunctionType}; @@ -105,16 +105,42 @@ pub trait HugrView: sealed::HugrInternals { } /// Returns the parent of a node. - fn get_parent(&self, node: Node) -> Option; + #[inline] + fn get_parent(&self, node: Node) -> Option { + self.valid_non_root(node).ok()?; + self.base_hugr() + .hierarchy + .parent(node.index) + .map(Into::into) + } /// Returns the operation type of a node. - fn get_optype(&self, node: Node) -> &OpType; + #[inline] + fn get_optype(&self, node: Node) -> &OpType { + &self.get_nodetype(node).op + } /// Returns the type of a node. - fn get_nodetype(&self, node: Node) -> &NodeType; + #[inline] + fn get_nodetype(&self, node: Node) -> &NodeType { + match self.contains_node(node) { + true => self.base_hugr().op_types.get(node.index), + false => &DEFAULT_NODETYPE, + } + } /// Returns the metadata associated with a node. - fn get_metadata(&self, node: Node) -> &NodeMetadata; + #[inline] + fn get_metadata(&self, node: Node) -> &NodeMetadata { + // The other way to do it - exploit the UnmanagedDenseMap's get() returning &default + let md = &self.base_hugr().metadata; + + let idx = match self.contains_node(node) { + true => node.index, + false => portgraph::NodeIndex::new(md.capacity() + 1), + }; + md.get(idx) + } /// Returns the number of nodes in the hugr. fn node_count(&self) -> usize; @@ -311,21 +337,6 @@ where self.as_ref().graph.contains_node(node.index) } - #[inline] - fn get_parent(&self, node: Node) -> Option { - self.as_ref().hierarchy.parent(node.index).map(Into::into) - } - - #[inline] - fn get_optype(&self, node: Node) -> &OpType { - &self.as_ref().op_types.get(node.index).op - } - - #[inline] - fn get_nodetype(&self, node: Node) -> &NodeType { - self.as_ref().op_types.get(node.index) - } - #[inline] fn node_count(&self) -> usize { self.as_ref().graph.node_count() @@ -410,11 +421,6 @@ where None } } - - #[inline] - fn get_metadata(&self, node: Node) -> &NodeMetadata { - self.as_ref().metadata.get(node.index) - } } pub(crate) mod sealed { diff --git a/src/hugr/views/descendants.rs b/src/hugr/views/descendants.rs index 73f7791a95..4355ddb53c 100644 --- a/src/hugr/views/descendants.rs +++ b/src/hugr/views/descendants.rs @@ -7,9 +7,9 @@ use portgraph::{LinkView, MultiPortGraph, PortIndex, PortView}; use crate::ops::handle::NodeHandle; use crate::ops::OpTrait; -use crate::{hugr::NodeType, hugr::OpType, Direction, Hugr, Node, Port}; +use crate::{Direction, Hugr, Node, Port}; -use super::{sealed::HugrInternals, HierarchyView, HugrView, NodeMetadata}; +use super::{sealed::HugrInternals, HierarchyView, HugrView}; type RegionGraph<'g> = portgraph::view::Region<'g, &'g MultiPortGraph>; @@ -94,29 +94,6 @@ where self.graph.contains_node(node.index) } - #[inline] - fn get_parent(&self, node: Node) -> Option { - self.hugr - .get_parent(node) - .filter(|&parent| self.graph.contains_node(parent.index)) - .map(Into::into) - } - - #[inline] - fn get_optype(&self, node: Node) -> &OpType { - self.hugr.get_optype(node) - } - - #[inline] - fn get_nodetype(&self, node: Node) -> &NodeType { - self.hugr.get_nodetype(node) - } - - #[inline] - fn get_metadata(&self, node: Node) -> &NodeMetadata { - self.hugr.get_metadata(node) - } - #[inline] fn node_count(&self) -> usize { self.graph.node_count() diff --git a/src/hugr/views/sibling.rs b/src/hugr/views/sibling.rs index d2caf509d6..16640e367d 100644 --- a/src/hugr/views/sibling.rs +++ b/src/hugr/views/sibling.rs @@ -8,9 +8,9 @@ use portgraph::{LinkView, MultiPortGraph, PortIndex, PortView}; use crate::ops::handle::NodeHandle; use crate::ops::OpTrait; -use crate::{hugr::NodeType, hugr::OpType, Direction, Hugr, Node, Port}; +use crate::{Direction, Hugr, Node, Port}; -use super::{sealed::HugrInternals, HierarchyView, HugrView, NodeMetadata}; +use super::{sealed::HugrInternals, HierarchyView, HugrView}; type FlatRegionGraph<'g> = portgraph::view::FlatRegion<'g, &'g MultiPortGraph>; @@ -94,26 +94,6 @@ where self.graph.contains_node(node.index) } - #[inline] - fn get_parent(&self, node: Node) -> Option { - self.hugr.get_parent(node).filter(|&n| n == self.root) - } - - #[inline] - fn get_optype(&self, node: Node) -> &OpType { - self.hugr.get_optype(node) - } - - #[inline] - fn get_nodetype(&self, node: Node) -> &NodeType { - self.hugr.get_nodetype(node) - } - - #[inline] - fn get_metadata(&self, node: Node) -> &NodeMetadata { - self.hugr.get_metadata(node) - } - #[inline] fn node_count(&self) -> usize { self.base_hugr().hierarchy.child_count(self.root.index) + 1 diff --git a/src/ops.rs b/src/ops.rs index cbd928fe73..0334becd3d 100644 --- a/src/ops.rs +++ b/src/ops.rs @@ -52,9 +52,12 @@ pub enum OpType { Case, } +/// The default OpType (as returned by [Default::default]) +pub const DEFAULT_OPTYPE: OpType = OpType::Module(Module); + impl Default for OpType { fn default() -> Self { - Module.into() + DEFAULT_OPTYPE } }