Skip to content

Commit

Permalink
feat!: infallible HugrMut methods (#869)
Browse files Browse the repository at this point in the history
While all `HugrView` methods are infallible (panicking when a node is
not found), most `HugrMut` methods returned a `Result`, even though they
only failed on the same case.

This PR makes most `HugrMut` methods infallible (except for
`replace_op`). This simplify the API and lets us remove a lot of
`unwrap`s all around the codebase.

We can further cascade the `unwrap` removal to the builder (as
`create_with_io` and others can now be made infallible), but I'll do
that on another PR as this one is noisy enough.

The first commit contains the relevant changes. The 2nd one just removes
all the `unwrap`s.

BREAKING CHANGE: Made most `HugrMut` methods infallible.
  • Loading branch information
aborgna-q authored Mar 8, 2024
1 parent 497f43c commit 5b450ff
Show file tree
Hide file tree
Showing 23 changed files with 629 additions and 645 deletions.
11 changes: 3 additions & 8 deletions quantinuum-hugr/src/builder.rs
Original file line number Diff line number Diff line change
Expand Up @@ -89,7 +89,7 @@
use thiserror::Error;

use crate::extension::SignatureError;
use crate::hugr::{HugrError, ValidationError};
use crate::hugr::ValidationError;
use crate::ops::handle::{BasicBlockID, CfgID, ConditionalID, DfgID, FuncID, TailLoopID};
use crate::types::ConstTypeError;
use crate::types::Type;
Expand Down Expand Up @@ -136,9 +136,6 @@ pub enum BuildError {
/// [Const]: crate::ops::constant::Const
#[error("Constant failed typechecking: {0}")]
BadConstant(#[from] ConstTypeError),
/// HUGR construction error.
#[error("Error when mutating HUGR: {0}.")]
ConstructError(#[from] HugrError),
/// CFG can only have one entry.
#[error("CFG entry node already built for CFG node: {0:?}.")]
EntryBuiltError(Node),
Expand Down Expand Up @@ -235,15 +232,13 @@ pub(crate) mod test {
ops::Input {
types: signature.input,
},
)
.unwrap();
);
hugr.add_node_with_parent(
hugr.root(),
ops::Output {
types: signature.output,
},
)
.unwrap();
);
hugr
}
}
20 changes: 10 additions & 10 deletions quantinuum-hugr/src/builder/build_traits.rs
Original file line number Diff line number Diff line change
Expand Up @@ -47,20 +47,20 @@ pub trait Container {
/// Add an [`OpType`] as the final child of the container.
fn add_child_op(&mut self, op: impl Into<OpType>) -> Result<Node, BuildError> {
let parent = self.container_node();
Ok(self.hugr_mut().add_node_with_parent(parent, op)?)
Ok(self.hugr_mut().add_node_with_parent(parent, op))
}
/// Add a [`NodeType`] as the final child of the container.
fn add_child_node(&mut self, node: NodeType) -> Result<Node, BuildError> {
let parent = self.container_node();
Ok(self.hugr_mut().add_node_with_parent(parent, node)?)
Ok(self.hugr_mut().add_node_with_parent(parent, node))
}

/// Adds a non-dataflow edge between two nodes. The kind is given by the operation's [`other_inputs`] or [`other_outputs`]
///
/// [`other_inputs`]: crate::ops::OpTrait::other_input
/// [`other_outputs`]: crate::ops::OpTrait::other_output
fn add_other_wire(&mut self, src: Node, dst: Node) -> Result<Wire, BuildError> {
let (src_port, _) = self.hugr_mut().add_other_edge(src, dst)?;
let (src_port, _) = self.hugr_mut().add_other_edge(src, dst);
Ok(Wire::new(src, src_port))
}

Expand Down Expand Up @@ -102,20 +102,20 @@ pub trait Container {
/// Insert a HUGR as a child of the container.
fn add_hugr(&mut self, child: Hugr) -> Result<InsertionResult, BuildError> {
let parent = self.container_node();
Ok(self.hugr_mut().insert_hugr(parent, child)?)
Ok(self.hugr_mut().insert_hugr(parent, child))
}

/// Insert a copy of a HUGR as a child of the container.
fn add_hugr_view(&mut self, child: &impl HugrView) -> Result<InsertionResult, BuildError> {
let parent = self.container_node();
Ok(self.hugr_mut().insert_from_view(parent, child)?)
Ok(self.hugr_mut().insert_from_view(parent, child))
}

/// Add metadata to the container node.
fn set_metadata(&mut self, key: impl AsRef<str>, meta: impl Into<NodeMetadata>) {
let parent = self.container_node();
// Implementor's container_node() should be a valid node
self.hugr_mut().set_metadata(parent, key, meta).unwrap();
self.hugr_mut().set_metadata(parent, key, meta);
}

/// Add metadata to a child node.
Expand All @@ -127,7 +127,7 @@ pub trait Container {
key: impl AsRef<str>,
meta: impl Into<NodeMetadata>,
) -> Result<(), BuildError> {
self.hugr_mut().set_metadata(child, key, meta)?;
self.hugr_mut().set_metadata(child, key, meta);
Ok(())
}
}
Expand Down Expand Up @@ -604,7 +604,7 @@ pub trait Dataflow: Container {
let src_port = self.hugr_mut().num_outputs(function.node()) - 1;

self.hugr_mut()
.connect(function.node(), src_port, op_id.node(), const_in_port)?;
.connect(function.node(), src_port, op_id.node(), const_in_port);
Ok(op_id)
}

Expand Down Expand Up @@ -695,7 +695,7 @@ fn wire_up<T: Dataflow + ?Sized>(
&& !OpTag::BasicBlock.is_superset(base.get_optype(src_sibling).tag())
{
// Add a state order constraint unless one of the nodes is a CFG BasicBlock
base.add_other_edge(src, src_sibling)?;
base.add_other_edge(src, src_sibling);
}
} else if !typ.copyable() & base.linked_ports(src, src_port).next().is_some() {
// Don't copy linear edges.
Expand All @@ -705,7 +705,7 @@ fn wire_up<T: Dataflow + ?Sized>(

data_builder
.hugr_mut()
.connect(src, src_port, dst, dst_port)?;
.connect(src, src_port, dst, dst_port);
Ok(local_source
&& matches!(
data_builder
Expand Down
7 changes: 4 additions & 3 deletions quantinuum-hugr/src/builder/cfg.rs
Original file line number Diff line number Diff line change
Expand Up @@ -190,7 +190,7 @@ impl<B: AsMut<Hugr> + AsRef<Hugr>> CFGBuilder<B> {
let exit_node = base
.as_mut()
// Make the extensions a parameter
.add_node_with_parent(cfg_node, exit_block_type)?;
.add_node_with_parent(cfg_node, exit_block_type);
Ok(Self {
base,
cfg_node,
Expand Down Expand Up @@ -246,7 +246,7 @@ impl<B: AsMut<Hugr> + AsRef<Hugr>> CFGBuilder<B> {
} else {
// TODO: Make extensions a parameter
self.hugr_mut().add_node_with_parent(parent, op)
}?;
};

BlockBuilder::create(self.hugr_mut(), block_n)
}
Expand Down Expand Up @@ -323,7 +323,8 @@ impl<B: AsMut<Hugr> + AsRef<Hugr>> CFGBuilder<B> {
) -> Result<(), BuildError> {
let from = predecessor.node();
let to = successor.node();
Ok(self.hugr_mut().connect(from, branch, to, 0)?)
self.hugr_mut().connect(from, branch, to, 0);
Ok(())
}
}

Expand Down
2 changes: 1 addition & 1 deletion quantinuum-hugr/src/builder/conditional.rs
Original file line number Diff line number Diff line change
Expand Up @@ -127,7 +127,7 @@ impl<B: AsMut<Hugr> + AsRef<Hugr>> ConditionalBuilder<B> {
let case_node =
// add case before any existing subsequent cases
if let Some(&sibling_node) = self.case_nodes[case + 1..].iter().flatten().next() {
self.hugr_mut().add_node_before(sibling_node, case_op)?
self.hugr_mut().add_node_before(sibling_node, case_op)
} else {
self.add_child_op(case_op)?
};
Expand Down
4 changes: 2 additions & 2 deletions quantinuum-hugr/src/builder/dataflow.rs
Original file line number Diff line number Diff line change
Expand Up @@ -50,14 +50,14 @@ impl<T: AsMut<Hugr> + AsRef<Hugr>> DFGBuilder<T> {
types: signature.output().clone(),
};
base.as_mut()
.add_node_with_parent(parent, NodeType::new(input, input_extensions.clone()))?;
.add_node_with_parent(parent, NodeType::new(input, input_extensions.clone()));
base.as_mut().add_node_with_parent(
parent,
NodeType::new(
output,
input_extensions.map(|inp| inp.union(signature.extension_reqs)),
),
)?;
);

Ok(Self {
base,
Expand Down
10 changes: 6 additions & 4 deletions quantinuum-hugr/src/builder/module.rs
Original file line number Diff line number Diff line change
Expand Up @@ -85,10 +85,12 @@ impl<T: AsMut<Hugr> + AsRef<Hugr>> ModuleBuilder<T> {
})?
.clone();
let body = signature.body().clone();
self.hugr_mut().replace_op(
f_node,
NodeType::new_pure(ops::FuncDefn { name, signature }),
)?;
self.hugr_mut()
.replace_op(
f_node,
NodeType::new_pure(ops::FuncDefn { name, signature }),
)
.expect("Replacing a FuncDecl node with a FuncDefn should always be valid");

let db = DFGBuilder::create_with_io(self.hugr_mut(), f_node, body, None)?;
Ok(FunctionBuilder::from_dfg_builder(db))
Expand Down
Loading

0 comments on commit 5b450ff

Please sign in to comment.