From 5c18ab024a4e0a0c0e72908825db3c1c6f0fd591 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Agust=C3=ADn=20Borgna?= Date: Fri, 6 Dec 2024 08:48:49 +0000 Subject: [PATCH 1/2] perf!: Return `Cow` where possible --- hugr-core/src/builder/cfg.rs | 2 +- hugr-core/src/builder/conditional.rs | 2 +- hugr-core/src/builder/dataflow.rs | 2 +- hugr-core/src/extension/prelude.rs | 4 +- hugr-core/src/extension/resolution/ops.rs | 4 +- hugr-core/src/hugr/rewrite/outline_cfg.rs | 2 +- hugr-core/src/hugr/rewrite/replace.rs | 4 +- hugr-core/src/hugr/validate.rs | 4 +- hugr-core/src/hugr/validate/test.rs | 2 +- hugr-core/src/hugr/views.rs | 6 ++- hugr-core/src/hugr/views/descendants.rs | 4 +- hugr-core/src/hugr/views/sibling.rs | 12 +++-- hugr-core/src/hugr/views/sibling_subgraph.rs | 2 +- hugr-core/src/ops.rs | 8 ++-- hugr-core/src/ops/constant.rs | 10 ++-- hugr-core/src/ops/controlflow.rs | 44 ++++++++++++------ hugr-core/src/ops/custom.rs | 18 ++++---- hugr-core/src/ops/dataflow.rs | 46 +++++++++++-------- hugr-core/src/ops/module.rs | 6 ++- hugr-core/src/ops/sum.rs | 9 ++-- hugr-core/src/ops/validate.rs | 4 +- hugr-core/src/package.rs | 5 +- .../src/std_extensions/arithmetic/int_ops.rs | 20 ++++---- hugr-llvm/src/extension/prelude/array.rs | 7 ++- .../src/utils/inline_constant_functions.rs | 1 + hugr-passes/src/dataflow/test.rs | 4 +- hugr-passes/src/merge_bbs.rs | 5 +- 27 files changed, 145 insertions(+), 92 deletions(-) diff --git a/hugr-core/src/builder/cfg.rs b/hugr-core/src/builder/cfg.rs index b25b6ef33..acf7e3af9 100644 --- a/hugr-core/src/builder/cfg.rs +++ b/hugr-core/src/builder/cfg.rs @@ -391,7 +391,7 @@ impl + AsRef> BlockBuilder { .get_optype(block_n) .as_dataflow_block() .unwrap(); - let signature = block_op.inner_signature(); + let signature = block_op.inner_signature().into_owned(); let db = DFGBuilder::create_with_io(base, block_n, signature)?; Ok(BlockBuilder::from_dfg_builder(db)) } diff --git a/hugr-core/src/builder/conditional.rs b/hugr-core/src/builder/conditional.rs index 4aec60c2a..0f4f6963d 100644 --- a/hugr-core/src/builder/conditional.rs +++ b/hugr-core/src/builder/conditional.rs @@ -107,7 +107,7 @@ impl + AsRef> ConditionalBuilder { .clone() .try_into() .expect("Parent node does not have Conditional optype."); - let extension_delta = cond.signature().extension_reqs; + let extension_delta = cond.signature().extension_reqs.clone(); let inputs = cond .case_input_row(case) .ok_or(ConditionalBuildError::NotCase { conditional, case })?; diff --git a/hugr-core/src/builder/dataflow.rs b/hugr-core/src/builder/dataflow.rs index ce38650c0..e4720a676 100644 --- a/hugr-core/src/builder/dataflow.rs +++ b/hugr-core/src/builder/dataflow.rs @@ -248,7 +248,7 @@ impl FunctionBuilder { .get_optype(parent) .as_func_defn() .expect("FunctionBuilder node must be a FuncDefn"); - let signature = old_optype.inner_signature(); + let signature = old_optype.inner_signature().into_owned(); let name = old_optype.name.clone(); self.hugr_mut() .replace_op( diff --git a/hugr-core/src/extension/prelude.rs b/hugr-core/src/extension/prelude.rs index 86cae4e1c..8807a3b35 100644 --- a/hugr-core/src/extension/prelude.rs +++ b/hugr-core/src/extension/prelude.rs @@ -1022,8 +1022,8 @@ mod test { let op = Lift::new(type_row![Type::UNIT], ExtensionSet::singleton(XA)); let optype: OpType = op.clone().into(); assert_eq!( - optype.dataflow_signature().unwrap(), - Signature::new_endo(type_row![Type::UNIT]) + optype.dataflow_signature().unwrap().as_ref(), + &Signature::new_endo(type_row![Type::UNIT]) .with_extension_delta(XA) .with_prelude() ); diff --git a/hugr-core/src/extension/resolution/ops.rs b/hugr-core/src/extension/resolution/ops.rs index 42e3954dd..8e9591688 100644 --- a/hugr-core/src/extension/resolution/ops.rs +++ b/hugr-core/src/extension/resolution/ops.rs @@ -98,8 +98,8 @@ pub(crate) fn resolve_op_extensions<'e>( node, extension: opaque.extension().clone(), op: def.name().clone(), - computed: ext_op.signature().clone(), - stored: opaque.signature().clone(), + computed: ext_op.signature().into_owned(), + stored: opaque.signature().into_owned(), } .into()); }; diff --git a/hugr-core/src/hugr/rewrite/outline_cfg.rs b/hugr-core/src/hugr/rewrite/outline_cfg.rs index b7b1da3bc..932d41ff4 100644 --- a/hugr-core/src/hugr/rewrite/outline_cfg.rs +++ b/hugr-core/src/hugr/rewrite/outline_cfg.rs @@ -71,7 +71,7 @@ impl OutlineCfg { } } } - extension_delta = extension_delta.union(o.signature().extension_reqs); + extension_delta = extension_delta.union(o.signature().extension_reqs.clone()); let external_succs = h.output_neighbours(n).filter(|s| !self.blocks.contains(s)); match external_succs.at_most_one() { Ok(None) => (), // No external successors diff --git a/hugr-core/src/hugr/rewrite/replace.rs b/hugr-core/src/hugr/rewrite/replace.rs index a201e6809..f652e2170 100644 --- a/hugr-core/src/hugr/rewrite/replace.rs +++ b/hugr-core/src/hugr/rewrite/replace.rs @@ -615,9 +615,9 @@ mod test { }, op_sig.input() ); - h.simple_entry_builder_exts(op_sig.output, 1, op_sig.extension_reqs.clone())? + h.simple_entry_builder_exts(op_sig.output.clone(), 1, op_sig.extension_reqs.clone())? } else { - h.simple_block_builder(op_sig, 1)? + h.simple_block_builder(op_sig.into_owned(), 1)? }; let op: OpType = op.into(); let op = bb.add_dataflow_op(op, bb.input_wires())?; diff --git a/hugr-core/src/hugr/validate.rs b/hugr-core/src/hugr/validate.rs index 663253e1a..e48e47ede 100644 --- a/hugr-core/src/hugr/validate.rs +++ b/hugr-core/src/hugr/validate.rs @@ -16,7 +16,7 @@ use crate::ops::custom::{ExtensionOp, OpaqueOpError}; use crate::ops::validate::{ChildrenEdgeData, ChildrenValidationError, EdgeValidationError}; use crate::ops::{FuncDefn, NamedOp, OpName, OpParent, OpTag, OpTrait, OpType, ValidateOp}; use crate::types::type_param::TypeParam; -use crate::types::{EdgeKind, Signature}; +use crate::types::EdgeKind; use crate::{Direction, Hugr, Node, Port}; use super::views::{HierarchyView, HugrView, SiblingGraph}; @@ -61,7 +61,7 @@ impl Hugr { return Err(ValidationError::ExtensionsNotInferred { node: parent }); } let parent_extensions = match parent_op.inner_function_type() { - Some(Signature { extension_reqs, .. }) => extension_reqs, + Some(s) => s.extension_reqs.clone(), None => match parent_op.tag() { OpTag::Cfg | OpTag::Conditional => parent_op.extension_delta(), // ModuleRoot holds but does not execute its children, so allow any extensions diff --git a/hugr-core/src/hugr/validate/test.rs b/hugr-core/src/hugr/validate/test.rs index 155449af5..dd4497aa7 100644 --- a/hugr-core/src/hugr/validate/test.rs +++ b/hugr-core/src/hugr/validate/test.rs @@ -784,7 +784,7 @@ fn test_polymorphic_call() -> Result<(), Box> { let exp_fun_ty = Signature::new(vec![utou(PRELUDE_ID), int_pair.clone()], int_pair) .with_extension_delta(EXT_ID) .with_prelude(); - assert_eq!(call_ty, exp_fun_ty); + assert_eq!(call_ty.as_ref(), &exp_fun_ty); Ok(()) } diff --git a/hugr-core/src/hugr/views.rs b/hugr-core/src/hugr/views.rs index 34396ec68..f5b3ec5de 100644 --- a/hugr-core/src/hugr/views.rs +++ b/hugr-core/src/hugr/views.rs @@ -11,6 +11,8 @@ pub mod sibling_subgraph; #[cfg(test)] mod tests; +use std::borrow::Cow; + pub use self::petgraph::PetgraphWrapper; use self::render::RenderConfig; pub use descendants::DescendantsGraph; @@ -311,7 +313,7 @@ pub trait HugrView: HugrInternals { /// /// In contrast to [`poly_func_type`][HugrView::poly_func_type], this /// method always return a concrete [`Signature`]. - fn inner_function_type(&self) -> Option { + fn inner_function_type(&self) -> Option> { self.root_type().inner_function_type() } @@ -408,7 +410,7 @@ pub trait HugrView: HugrInternals { /// Get the "signature" (incoming and outgoing types) of a node, non-Value /// kind ports will be missing. - fn signature(&self, node: Node) -> Option { + fn signature(&self, node: Node) -> Option> { self.get_optype(node).dataflow_signature() } diff --git a/hugr-core/src/hugr/views/descendants.rs b/hugr-core/src/hugr/views/descendants.rs index 92a09037d..7d3fd2b62 100644 --- a/hugr-core/src/hugr/views/descendants.rs +++ b/hugr-core/src/hugr/views/descendants.rs @@ -173,6 +173,8 @@ where #[cfg(test)] pub(super) mod test { + use std::borrow::Cow; + use rstest::rstest; use crate::extension::prelude::{qb_t, usize_t}; @@ -241,7 +243,7 @@ pub(super) mod test { let inner_region: DescendantsGraph = DescendantsGraph::try_new(&hugr, inner)?; assert_eq!( - inner_region.inner_function_type(), + inner_region.inner_function_type().map(Cow::into_owned), Some(Signature::new(vec![usize_t()], vec![usize_t()])) ); assert_eq!(inner_region.node_count(), 3); diff --git a/hugr-core/src/hugr/views/sibling.rs b/hugr-core/src/hugr/views/sibling.rs index 0f0794831..59c69f8bc 100644 --- a/hugr-core/src/hugr/views/sibling.rs +++ b/hugr-core/src/hugr/views/sibling.rs @@ -332,6 +332,8 @@ impl HugrMut for SiblingMut<'_, Root> {} #[cfg(test)] mod test { + use std::borrow::Cow; + use rstest::rstest; use crate::builder::test::simple_dfg_hugr; @@ -376,7 +378,7 @@ mod test { ); assert_eq!( - inner_region.inner_function_type(), + inner_region.inner_function_type().map(Cow::into_owned), Some(Signature::new(vec![usize_t()], vec![usize_t()])) ); assert_eq!(inner_region.node_count(), 3); @@ -487,7 +489,7 @@ mod test { fn flat_mut(mut simple_dfg_hugr: Hugr) { simple_dfg_hugr.validate().unwrap(); let root = simple_dfg_hugr.root(); - let signature = simple_dfg_hugr.inner_function_type().unwrap().clone(); + let signature = simple_dfg_hugr.inner_function_type().unwrap().into_owned(); let sib_mut = SiblingMut::::try_new(&mut simple_dfg_hugr, root); assert_eq!( @@ -517,7 +519,11 @@ mod test { fn sibling_mut_covariance(mut simple_dfg_hugr: Hugr) { let root = simple_dfg_hugr.root(); let case_nodetype = crate::ops::Case { - signature: simple_dfg_hugr.root_type().dataflow_signature().unwrap(), + signature: simple_dfg_hugr + .root_type() + .dataflow_signature() + .unwrap() + .into_owned(), }; let mut sib_mut = SiblingMut::::try_new(&mut simple_dfg_hugr, root).unwrap(); // As expected, we cannot replace the root with a Case diff --git a/hugr-core/src/hugr/views/sibling_subgraph.rs b/hugr-core/src/hugr/views/sibling_subgraph.rs index 82033db42..589e96968 100644 --- a/hugr-core/src/hugr/views/sibling_subgraph.rs +++ b/hugr-core/src/hugr/views/sibling_subgraph.rs @@ -388,7 +388,7 @@ impl SiblingSubgraph { { return Err(InvalidReplacement::InvalidSignature { expected: self.signature(hugr), - actual: dfg_optype.dataflow_signature(), + actual: dfg_optype.dataflow_signature().map(|s| s.into_owned()), }); } diff --git a/hugr-core/src/ops.rs b/hugr-core/src/ops.rs index 05f854589..e509917bf 100644 --- a/hugr-core/src/ops.rs +++ b/hugr-core/src/ops.rs @@ -12,6 +12,8 @@ pub mod validate; use crate::extension::resolution::{ collect_op_extension, collect_op_types_extensions, ExtensionCollectionError, }; +use std::borrow::Cow; + use crate::extension::simple_op::MakeExtensionOp; use crate::extension::{ExtensionId, ExtensionRegistry, ExtensionSet}; use crate::types::{EdgeKind, Signature}; @@ -377,7 +379,7 @@ pub trait OpTrait { /// The signature of the operation. /// /// Only dataflow operations have a signature, otherwise returns None. - fn dataflow_signature(&self) -> Option { + fn dataflow_signature(&self) -> Option> { None } @@ -440,13 +442,13 @@ pub trait OpParent { /// sibling graph. /// /// Non-container ops like `FuncDecl` return `None` even though they represent a function. - fn inner_function_type(&self) -> Option { + fn inner_function_type(&self) -> Option> { None } } impl OpParent for T { - fn inner_function_type(&self) -> Option { + fn inner_function_type(&self) -> Option> { Some(DataflowParent::inner_signature(self)) } } diff --git a/hugr-core/src/ops/constant.rs b/hugr-core/src/ops/constant.rs index e4a38e079..844ffb159 100644 --- a/hugr-core/src/ops/constant.rs +++ b/hugr-core/src/ops/constant.rs @@ -2,6 +2,7 @@ mod custom; +use std::borrow::Cow; use std::collections::hash_map::DefaultHasher; // Moves into std::hash in Rust 1.76. use std::hash::{Hash, Hasher}; @@ -348,12 +349,15 @@ pub enum ConstTypeError { } /// Hugrs (even functions) inside Consts must be monomorphic -fn mono_fn_type(h: &Hugr) -> Result { +fn mono_fn_type(h: &Hugr) -> Result, ConstTypeError> { let err = || ConstTypeError::NotMonomorphicFunction { hugr_root_type: h.root_type().clone(), }; if let Some(pf) = h.poly_func_type() { - return pf.try_into().map_err(|_| err()); + match pf.try_into() { + Ok(sig) => return Ok(Cow::Owned(sig)), + Err(_) => return Err(err()), + }; } h.inner_function_type().ok_or_else(err) @@ -367,7 +371,7 @@ impl Value { Self::Sum(Sum { sum_type, .. }) => sum_type.clone().into(), Self::Function { hugr } => { let func_type = mono_fn_type(hugr).unwrap_or_else(|e| panic!("{}", e)); - Type::new_function(func_type) + Type::new_function(func_type.into_owned()) } } } diff --git a/hugr-core/src/ops/controlflow.rs b/hugr-core/src/ops/controlflow.rs index 19b0f167a..94c817a53 100644 --- a/hugr-core/src/ops/controlflow.rs +++ b/hugr-core/src/ops/controlflow.rs @@ -1,5 +1,7 @@ //! Control flow operations. +use std::borrow::Cow; + use crate::extension::ExtensionSet; use crate::types::{EdgeKind, Signature, Type, TypeRow}; use crate::Direction; @@ -31,10 +33,13 @@ impl DataflowOpTrait for TailLoop { "A tail-controlled loop" } - fn signature(&self) -> Signature { + fn signature(&self) -> Cow<'_, Signature> { + // TODO: Store a cached signature let [inputs, outputs] = [&self.just_inputs, &self.just_outputs].map(|row| row.extend(self.rest.iter())); - Signature::new(inputs, outputs).with_extension_delta(self.extension_delta.clone()) + Cow::Owned( + Signature::new(inputs, outputs).with_extension_delta(self.extension_delta.clone()), + ) } } @@ -64,9 +69,12 @@ impl TailLoop { } impl DataflowParent for TailLoop { - fn inner_signature(&self) -> Signature { - Signature::new(self.body_input_row(), self.body_output_row()) - .with_extension_delta(self.extension_delta.clone()) + fn inner_signature(&self) -> Cow<'_, Signature> { + // TODO: Store a cached signature + Cow::Owned( + Signature::new(self.body_input_row(), self.body_output_row()) + .with_extension_delta(self.extension_delta.clone()), + ) } } @@ -92,13 +100,16 @@ impl DataflowOpTrait for Conditional { "HUGR conditional operation" } - fn signature(&self) -> Signature { + fn signature(&self) -> Cow<'_, Signature> { + // TODO: Store a cached signature let mut inputs = self.other_inputs.clone(); inputs .to_mut() .insert(0, Type::new_sum(self.sum_rows.clone())); - Signature::new(inputs, self.outputs.clone()) - .with_extension_delta(self.extension_delta.clone()) + Cow::Owned( + Signature::new(inputs, self.outputs.clone()) + .with_extension_delta(self.extension_delta.clone()), + ) } } @@ -126,8 +137,8 @@ impl DataflowOpTrait for CFG { "A dataflow node defined by a child CFG" } - fn signature(&self) -> Signature { - self.signature.clone() + fn signature(&self) -> Cow<'_, Signature> { + Cow::Borrowed(&self.signature) } } @@ -172,13 +183,16 @@ impl StaticTag for ExitBlock { } impl DataflowParent for DataflowBlock { - fn inner_signature(&self) -> Signature { + fn inner_signature(&self) -> Cow<'_, Signature> { + // TODO: Store a cached signature // The node outputs a Sum before the data outputs of the block node let sum_type = Type::new_sum(self.sum_rows.clone()); let mut node_outputs = vec![sum_type]; node_outputs.extend_from_slice(&self.other_outputs); - Signature::new(self.inputs.clone(), TypeRow::from(node_outputs)) - .with_extension_delta(self.extension_delta.clone()) + Cow::Owned( + Signature::new(self.inputs.clone(), TypeRow::from(node_outputs)) + .with_extension_delta(self.extension_delta.clone()), + ) } } @@ -280,8 +294,8 @@ impl StaticTag for Case { } impl DataflowParent for Case { - fn inner_signature(&self) -> Signature { - self.signature.clone() + fn inner_signature(&self) -> Cow<'_, Signature> { + Cow::Borrowed(&self.signature) } } diff --git a/hugr-core/src/ops/custom.rs b/hugr-core/src/ops/custom.rs index b0312cd9d..91b93e332 100644 --- a/hugr-core/src/ops/custom.rs +++ b/hugr-core/src/ops/custom.rs @@ -1,6 +1,7 @@ //! Extensible operations. use itertools::Itertools; +use std::borrow::Cow; use std::sync::Arc; use thiserror::Error; #[cfg(test)] @@ -68,7 +69,7 @@ impl ExtensionOp { Ok(sig) => sig, Err(SignatureError::MissingComputeFunc) => { // TODO raise warning: https://github.com/CQCL/hugr/issues/1432 - opaque.signature() + opaque.signature().into_owned() } Err(e) => return Err(e), }; @@ -162,8 +163,8 @@ impl DataflowOpTrait for ExtensionOp { self.def().description() } - fn signature(&self) -> Signature { - self.signature.clone() + fn signature(&self) -> Cow<'_, Signature> { + Cow::Borrowed(&self.signature) } } @@ -204,6 +205,7 @@ impl OpaqueOp { args: impl Into>, signature: Signature, ) -> Self { + let signature = signature.with_extension_delta(extension.clone()); Self { extension, name: name.into(), @@ -254,10 +256,8 @@ impl DataflowOpTrait for OpaqueOp { &self.description } - fn signature(&self) -> Signature { - self.signature - .clone() - .with_extension_delta(self.extension().clone()) + fn signature(&self) -> Cow<'_, Signature> { + Cow::Borrowed(&self.signature) } } @@ -345,8 +345,8 @@ mod test { assert_eq!(DataflowOpTrait::description(&op), "desc"); assert_eq!(op.args(), &[TypeArg::Type { ty: usize_t() }]); assert_eq!( - op.signature(), - sig.with_extension_delta(op.extension().clone()) + op.signature().as_ref(), + &sig.with_extension_delta(op.extension().clone()) ); } diff --git a/hugr-core/src/ops/dataflow.rs b/hugr-core/src/ops/dataflow.rs index ed9a9e118..be406eb1a 100644 --- a/hugr-core/src/ops/dataflow.rs +++ b/hugr-core/src/ops/dataflow.rs @@ -1,5 +1,7 @@ //! Dataflow operations. +use std::borrow::Cow; + use super::{impl_op_name, OpTag, OpTrait}; use crate::extension::{ExtensionRegistry, ExtensionSet, SignatureError}; @@ -19,7 +21,7 @@ pub trait DataflowOpTrait { fn description(&self) -> &str; /// The signature of the operation. - fn signature(&self) -> Signature; + fn signature(&self) -> Cow<'_, Signature>; /// The edge kind for the non-dataflow or constant inputs of the operation, /// not described by the signature. @@ -105,8 +107,9 @@ impl DataflowOpTrait for Input { None } - fn signature(&self) -> Signature { - Signature::new(TypeRow::new(), self.types.clone()) + fn signature(&self) -> Cow<'_, Signature> { + // TODO: Store a cached signature + Cow::Owned(Signature::new(TypeRow::new(), self.types.clone())) } } impl DataflowOpTrait for Output { @@ -118,8 +121,9 @@ impl DataflowOpTrait for Output { // Note: We know what the input extensions should be, so we *could* give an // instantiated Signature instead - fn signature(&self) -> Signature { - Signature::new(self.types.clone(), TypeRow::new()) + fn signature(&self) -> Cow<'_, Signature> { + // TODO: Store a cached signature + Cow::Owned(Signature::new(self.types.clone(), TypeRow::new())) } fn other_output(&self) -> Option { @@ -134,7 +138,7 @@ impl OpTrait for T { fn tag(&self) -> OpTag { T::TAG } - fn dataflow_signature(&self) -> Option { + fn dataflow_signature(&self) -> Option> { Some(DataflowOpTrait::signature(self)) } fn extension_delta(&self) -> ExtensionSet { @@ -180,8 +184,8 @@ impl DataflowOpTrait for Call { "Call a function directly" } - fn signature(&self) -> Signature { - self.instantiation.clone() + fn signature(&self) -> Cow<'_, Signature> { + Cow::Borrowed(&self.instantiation) } fn static_input(&self) -> Option { @@ -198,7 +202,7 @@ impl Call { type_args: impl Into>, exts: &ExtensionRegistry, ) -> Result { - let type_args = type_args.into(); + let type_args: Vec<_> = type_args.into(); let instantiation = func_sig.instantiate(&type_args, exts)?; Ok(Self { func_sig, @@ -272,12 +276,13 @@ impl DataflowOpTrait for CallIndirect { "Call a function indirectly" } - fn signature(&self) -> Signature { + fn signature(&self) -> Cow<'_, Signature> { + // TODO: Store a cached signature let mut s = self.signature.clone(); s.input .to_mut() .insert(0, Type::new_function(self.signature.clone())); - s + Cow::Owned(s) } } @@ -296,8 +301,9 @@ impl DataflowOpTrait for LoadConstant { "Load a static constant in to the local dataflow graph" } - fn signature(&self) -> Signature { - Signature::new(TypeRow::new(), vec![self.datatype.clone()]) + fn signature(&self) -> Cow<'_, Signature> { + // TODO: Store a cached signature + Cow::Owned(Signature::new(TypeRow::new(), vec![self.datatype.clone()])) } fn static_input(&self) -> Option { @@ -351,8 +357,8 @@ impl DataflowOpTrait for LoadFunction { "Load a static function in to the local dataflow graph" } - fn signature(&self) -> Signature { - self.signature.clone() + fn signature(&self) -> Cow<'_, Signature> { + Cow::Borrowed(&self.signature) } fn static_input(&self) -> Option { @@ -369,7 +375,7 @@ impl LoadFunction { type_args: impl Into>, exts: &ExtensionRegistry, ) -> Result { - let type_args = type_args.into(); + let type_args: Vec<_> = type_args.into(); let instantiation = func_sig.instantiate(&type_args, exts)?; let signature = Signature::new(TypeRow::new(), vec![Type::new_function(instantiation)]); Ok(Self { @@ -418,7 +424,7 @@ impl LoadFunction { /// Operations that is the parent of a dataflow graph. pub trait DataflowParent { /// Signature of the inner dataflow graph. - fn inner_signature(&self) -> Signature; + fn inner_signature(&self) -> Cow<'_, Signature>; } /// A simply nested dataflow graph. @@ -432,8 +438,8 @@ pub struct DFG { impl_op_name!(DFG); impl DataflowParent for DFG { - fn inner_signature(&self) -> Signature { - self.signature.clone() + fn inner_signature(&self) -> Cow<'_, Signature> { + Cow::Borrowed(&self.signature) } } @@ -444,7 +450,7 @@ impl DataflowOpTrait for DFG { "A simply nested dataflow graph" } - fn signature(&self) -> Signature { + fn signature(&self) -> Cow<'_, Signature> { self.inner_signature() } } diff --git a/hugr-core/src/ops/module.rs b/hugr-core/src/ops/module.rs index c4c2a1e62..f321cfa9e 100644 --- a/hugr-core/src/ops/module.rs +++ b/hugr-core/src/ops/module.rs @@ -1,5 +1,7 @@ //! Module-level operations +use std::borrow::Cow; + use smol_str::SmolStr; #[cfg(test)] use { @@ -64,8 +66,8 @@ impl StaticTag for FuncDefn { } impl DataflowParent for FuncDefn { - fn inner_signature(&self) -> Signature { - self.signature.body().clone() + fn inner_signature(&self) -> Cow<'_, Signature> { + Cow::Borrowed(self.signature.body()) } } diff --git a/hugr-core/src/ops/sum.rs b/hugr-core/src/ops/sum.rs index aa7119104..b07179d00 100644 --- a/hugr-core/src/ops/sum.rs +++ b/hugr-core/src/ops/sum.rs @@ -1,5 +1,7 @@ //! Definition of dataflow operations with no children. +use std::borrow::Cow; + use super::dataflow::DataflowOpTrait; use super::{impl_op_name, OpTag}; use crate::types::{EdgeKind, Signature, Type, TypeRow}; @@ -35,14 +37,15 @@ impl DataflowOpTrait for Tag { } /// The signature of the operation. - fn signature(&self) -> Signature { - Signature::new( + fn signature(&self) -> Cow<'_, Signature> { + // TODO: Store a cached signature + Cow::Owned(Signature::new( self.variants .get(self.tag) .expect("Not a valid tag") .clone(), vec![Type::new_sum(self.variants.clone())], - ) + )) } fn other_input(&self) -> Option { diff --git a/hugr-core/src/ops/validate.rs b/hugr-core/src/ops/validate.rs index e12e29445..ab8a4c761 100644 --- a/hugr-core/src/ops/validate.rs +++ b/hugr-core/src/ops/validate.rs @@ -291,7 +291,7 @@ fn validate_io_nodes<'a>( if &first_sig.output != expected_input { return Err(ChildrenValidationError::IOSignatureMismatch { child: first, - actual: first_sig.output, + actual: first_sig.into_owned().output, expected: expected_input.clone(), node_desc: "Input", container_desc, @@ -302,7 +302,7 @@ fn validate_io_nodes<'a>( if &second_sig.input != expected_output { return Err(ChildrenValidationError::IOSignatureMismatch { child: second, - actual: second_sig.input, + actual: second_sig.into_owned().input, expected: expected_output.clone(), node_desc: "Output", container_desc, diff --git a/hugr-core/src/package.rs b/hugr-core/src/package.rs index 7461203f4..40c4e3ba0 100644 --- a/hugr-core/src/package.rs +++ b/hugr-core/src/package.rs @@ -244,7 +244,7 @@ fn to_module_hugr(mut hugr: Hugr) -> Result { root, FuncDefn { name: "main".to_string(), - signature: signature.into(), + signature: signature.into_owned().into(), }, ) .expect("Hugr accepts any root node"); @@ -259,7 +259,8 @@ fn to_module_hugr(mut hugr: Hugr) -> Result { if OpTag::DataflowChild.is_superset(tag) && !root_op.is_input() && !root_op.is_output() { let signature = root_op .dataflow_signature() - .unwrap_or_else(|| panic!("Dataflow child {} without signature", root_op.name())); + .unwrap_or_else(|| panic!("Dataflow child {} without signature", root_op.name())) + .into_owned(); let mut new_hugr = ModuleBuilder::new(); let mut func = new_hugr.define_function("main", signature).unwrap(); let dataflow_node = func.add_hugr_with_wires(hugr, func.input_wires()).unwrap(); diff --git a/hugr-core/src/std_extensions/arithmetic/int_ops.rs b/hugr-core/src/std_extensions/arithmetic/int_ops.rs index 6f4b248d1..2bb269076 100644 --- a/hugr-core/src/std_extensions/arithmetic/int_ops.rs +++ b/hugr-core/src/std_extensions/arithmetic/int_ops.rs @@ -384,24 +384,27 @@ mod test { .with_two_log_widths(3, 4) .to_extension_op() .unwrap() - .signature(), - Signature::new(int_type(3), int_type(4)).with_extension_delta(EXTENSION_ID) + .signature() + .as_ref(), + &Signature::new(int_type(3), int_type(4)).with_extension_delta(EXTENSION_ID) ); assert_eq!( IntOpDef::iwiden_s .with_two_log_widths(3, 3) .to_extension_op() .unwrap() - .signature(), - Signature::new_endo(int_type(3)).with_extension_delta(EXTENSION_ID) + .signature() + .as_ref(), + &Signature::new_endo(int_type(3)).with_extension_delta(EXTENSION_ID) ); assert_eq!( IntOpDef::inarrow_s .with_two_log_widths(3, 3) .to_extension_op() .unwrap() - .signature(), - Signature::new(int_type(3), sum_ty_with_err(int_type(3))) + .signature() + .as_ref(), + &Signature::new(int_type(3), sum_ty_with_err(int_type(3))) .with_extension_delta(EXTENSION_ID) ); assert!( @@ -417,8 +420,9 @@ mod test { .with_two_log_widths(2, 1) .to_extension_op() .unwrap() - .signature(), - Signature::new(int_type(2), sum_ty_with_err(int_type(1))) + .signature() + .as_ref(), + &Signature::new(int_type(2), sum_ty_with_err(int_type(1))) .with_extension_delta(EXTENSION_ID) ); diff --git a/hugr-llvm/src/extension/prelude/array.rs b/hugr-llvm/src/extension/prelude/array.rs index 4333dde72..7c6cbf750 100644 --- a/hugr-llvm/src/extension/prelude/array.rs +++ b/hugr-llvm/src/extension/prelude/array.rs @@ -116,7 +116,12 @@ pub fn emit_array_op<'c, H: HugrView>( ) -> Result<()> { let builder = ctx.builder(); let ts = ctx.typing_session(); - let sig = op.clone().to_extension_op().unwrap().signature(); + let sig = op + .clone() + .to_extension_op() + .unwrap() + .signature() + .into_owned(); let ArrayOp { def, ref elem_ty, diff --git a/hugr-llvm/src/utils/inline_constant_functions.rs b/hugr-llvm/src/utils/inline_constant_functions.rs index f946da615..99cb6527a 100644 --- a/hugr-llvm/src/utils/inline_constant_functions.rs +++ b/hugr-llvm/src/utils/inline_constant_functions.rs @@ -66,6 +66,7 @@ fn inline_constant_functions_impl( "Constant function hugr has no inner_func_type: {}", konst_n.index() ))? + .into_owned() .into(); let func_defn = FuncDefn { name: const_fn_name(konst_n), diff --git a/hugr-passes/src/dataflow/test.rs b/hugr-passes/src/dataflow/test.rs index 9e745a698..52e24b137 100644 --- a/hugr-passes/src/dataflow/test.rs +++ b/hugr-passes/src/dataflow/test.rs @@ -165,8 +165,8 @@ fn test_tail_loop_two_iters() { ) .unwrap(); assert_eq!( - tlb.loop_signature().unwrap().signature(), - Signature::new_endo(vec![bool_t(); 2]) + tlb.loop_signature().unwrap().signature().as_ref(), + &Signature::new_endo(vec![bool_t(); 2]) ); let [in_w1, in_w2] = tlb.input_wires_arr(); let tail_loop = tlb.finish_with_outputs(in_w1, [in_w2, in_w1]).unwrap(); diff --git a/hugr-passes/src/merge_bbs.rs b/hugr-passes/src/merge_bbs.rs index 9bc2f7f1d..9f24daefa 100644 --- a/hugr-passes/src/merge_bbs.rs +++ b/hugr-passes/src/merge_bbs.rs @@ -83,7 +83,7 @@ fn mk_rep( let dfg1 = replacement.add_node_with_parent( merged, DFG { - signature: pred_ty.inner_signature().clone(), + signature: pred_ty.inner_signature().into_owned(), }, ); for (i, _) in pred_ty.inputs.iter().enumerate() { @@ -93,7 +93,7 @@ fn mk_rep( let dfg2 = replacement.add_node_with_parent( merged, DFG { - signature: succ_sig.clone(), + signature: succ_sig.as_ref().clone(), }, ); for (i, _) in succ_sig.output.iter().enumerate() { @@ -323,6 +323,7 @@ mod test { let [res_t] = tst_op .dataflow_signature() .unwrap() + .into_owned() .output .into_owned() .try_into() From a9732c6a7a259b103bffab3a50815c19d301cf02 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Agust=C3=ADn=20Borgna?= Date: Tue, 10 Dec 2024 15:18:01 +0000 Subject: [PATCH 2/2] temp fix waiting for 1758 --- hugr-core/src/ops/custom.rs | 9 ++++++++- 1 file changed, 8 insertions(+), 1 deletion(-) diff --git a/hugr-core/src/ops/custom.rs b/hugr-core/src/ops/custom.rs index 91b93e332..eed0a761f 100644 --- a/hugr-core/src/ops/custom.rs +++ b/hugr-core/src/ops/custom.rs @@ -257,7 +257,14 @@ impl DataflowOpTrait for OpaqueOp { } fn signature(&self) -> Cow<'_, Signature> { - Cow::Borrowed(&self.signature) + // TODO: Return a borrowed cow once + // https://github.com/CQCL/hugr/issues/1758 + // gets fixed + Cow::Owned( + self.signature + .clone() + .with_extension_delta(self.extension.clone()), + ) } }