Skip to content

Commit

Permalink
Implement HugrView::get_{parent,{node,op}type,metadata} once only, wi…
Browse files Browse the repository at this point in the history
…th 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<Hugr> 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
  • Loading branch information
acl-cqc committed Sep 12, 2023
1 parent 1fea126 commit 8dc94f8
Show file tree
Hide file tree
Showing 5 changed files with 46 additions and 74 deletions.
8 changes: 7 additions & 1 deletion src/hugr.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -64,6 +64,12 @@ pub struct NodeType {
input_extensions: Option<ExtensionSet>,
}

/// 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<OpType>, input_extensions: impl Into<Option<ExtensionSet>>) -> Self {
Expand Down
56 changes: 31 additions & 25 deletions src/hugr/views.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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};
Expand Down Expand Up @@ -105,16 +105,42 @@ pub trait HugrView: sealed::HugrInternals {
}

/// Returns the parent of a node.
fn get_parent(&self, node: Node) -> Option<Node>;
#[inline]
fn get_parent(&self, node: Node) -> Option<Node> {
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;
Expand Down Expand Up @@ -311,21 +337,6 @@ where
self.as_ref().graph.contains_node(node.index)
}

#[inline]
fn get_parent(&self, node: Node) -> Option<Node> {
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()
Expand Down Expand Up @@ -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 {
Expand Down
27 changes: 2 additions & 25 deletions src/hugr/views/descendants.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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>;

Expand Down Expand Up @@ -94,29 +94,6 @@ where
self.graph.contains_node(node.index)
}

#[inline]
fn get_parent(&self, node: Node) -> Option<Node> {
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()
Expand Down
24 changes: 2 additions & 22 deletions src/hugr/views/sibling.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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>;

Expand Down Expand Up @@ -94,26 +94,6 @@ where
self.graph.contains_node(node.index)
}

#[inline]
fn get_parent(&self, node: Node) -> Option<Node> {
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
Expand Down
5 changes: 4 additions & 1 deletion src/ops.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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
}
}

Expand Down

0 comments on commit 8dc94f8

Please sign in to comment.