Skip to content

Commit

Permalink
refactor!: Merge CustomOp and ExternalOp. (#923)
Browse files Browse the repository at this point in the history
Followup of #922 .

Drops `ExternalOp` and makes `CustomOp` an enum with two boxed variants
for `OpaqueOp` and `ExtensionOp`.
The schema remains the same.

---------

Co-authored-by: Seyon Sivarajah <seyon.sivarajah@quantinuum.com>
  • Loading branch information
aborgna-q and ss2165 committed Apr 12, 2024
1 parent 3598913 commit c4a5631
Show file tree
Hide file tree
Showing 12 changed files with 139 additions and 175 deletions.
2 changes: 1 addition & 1 deletion hugr/src/algorithm/const_fold.rs
Original file line number Diff line number Diff line change
Expand Up @@ -65,7 +65,7 @@ pub fn fold_leaf_op(op: &OpType, consts: &[(IncomingPort, Const)]) -> ConstFoldR
)
.unwrap()]),
OpType::CustomOp(op) => {
let ext_op = op.as_ref().as_extension_op()?;
let ext_op = op.as_extension_op()?;
ext_op.constant_fold(consts)
}
_ => None,
Expand Down
4 changes: 2 additions & 2 deletions hugr/src/builder/circuit.rs
Original file line number Diff line number Diff line change
Expand Up @@ -278,13 +278,13 @@ mod test {

#[test]
fn with_nonlinear_and_outputs() {
let my_custom_op = CustomOp::new(crate::ops::custom::ExternalOp::Opaque(OpaqueOp::new(
let my_custom_op = CustomOp::new_opaque(OpaqueOp::new(
"MissingRsrc".try_into().unwrap(),
"MyOp",
"unknown op".to_string(),
vec![],
FunctionType::new(vec![QB, NAT], vec![QB]),
)));
));
let build_res = build_main(
FunctionType::new(type_row![QB, QB, NAT], type_row![QB, QB, BOOL_T]).into(),
|mut f_build| {
Expand Down
13 changes: 6 additions & 7 deletions hugr/src/extension/infer/test.rs
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@ use crate::extension::ExtensionId;
use crate::extension::{prelude::PRELUDE_REGISTRY, ExtensionSet};
use crate::hugr::{Hugr, HugrMut, HugrView, NodeType};
use crate::macros::const_extension_ids;
use crate::ops::custom::{ExternalOp, OpaqueOp};
use crate::ops::custom::OpaqueOp;
use crate::ops::{self, dataflow::IOTrait};
use crate::ops::{CustomOp, Lift, OpType};
#[cfg(feature = "extension_inference")]
Expand Down Expand Up @@ -439,8 +439,7 @@ fn extension_adding_sequence() -> Result<(), Box<dyn Error>> {
}

fn make_opaque(extension: impl Into<ExtensionId>, signature: FunctionType) -> CustomOp {
let opaque = ops::custom::OpaqueOp::new(extension.into(), "", "".into(), vec![], signature);
ops::custom::ExternalOp::from(opaque).into()
ops::custom::OpaqueOp::new(extension.into(), "", "".into(), vec![], signature).into()
}

fn make_block(
Expand Down Expand Up @@ -930,21 +929,21 @@ fn plus_on_self() -> Result<(), Box<dyn std::error::Error>> {
// While https://github.com/CQCL/hugr/issues/388 is unsolved,
// most operations have empty extension_reqs (not including their own extension).
// Define some that do.
let binop = CustomOp::new(ExternalOp::Opaque(OpaqueOp::new(
let binop = CustomOp::new_opaque(OpaqueOp::new(
ext.clone(),
"2qb_op",
String::new(),
vec![],
ft,
)));
));
let unary_sig = FunctionType::new_endo(type_row![QB_T]).with_extension_delta(ext.clone());
let unop = CustomOp::new(ExternalOp::Opaque(OpaqueOp::new(
let unop = CustomOp::new_opaque(OpaqueOp::new(
ext,
"1qb_op",
String::new(),
vec![],
unary_sig,
)));
));
// Constrain q1,q2 as PLUS(ext1, inputs):
let [q1, q2] = dfg
.add_dataflow_op(binop.clone(), dfg.input_wires())?
Expand Down
5 changes: 2 additions & 3 deletions hugr/src/extension/op_def.rs
Original file line number Diff line number Diff line change
Expand Up @@ -476,7 +476,6 @@ mod test {
use crate::extension::prelude::USIZE_T;
use crate::extension::{ExtensionRegistry, ExtensionSet, PRELUDE};
use crate::extension::{SignatureError, EMPTY_REG, PRELUDE_REGISTRY};
use crate::ops::custom::ExternalOp;
use crate::ops::CustomOp;
use crate::std_extensions::collections::{EXTENSION, LIST_TYPENAME};
use crate::types::Type;
Expand Down Expand Up @@ -513,10 +512,10 @@ mod test {
Type::new_extension(list_def.instantiate(vec![TypeArg::Type { ty: USIZE_T }])?);
let mut dfg = DFGBuilder::new(FunctionType::new_endo(vec![list_usize]))?;
let rev = dfg.add_dataflow_op(
CustomOp::new(ExternalOp::Extension(
CustomOp::new_extension(
e.instantiate_extension_op(&OP_NAME, vec![TypeArg::Type { ty: USIZE_T }], &reg)
.unwrap(),
)),
),
dfg.input_wires(),
)?;
dfg.finish_hugr_with_outputs(rev.outputs(), &reg)?;
Expand Down
2 changes: 1 addition & 1 deletion hugr/src/extension/simple_op.rs
Original file line number Diff line number Diff line change
Expand Up @@ -94,7 +94,7 @@ pub trait MakeExtensionOp: OpName {
where
Self: Sized,
{
let ext: &ExtensionOp = op.as_custom_op()?.as_ref().as_extension_op()?;
let ext: &ExtensionOp = op.as_custom_op()?.as_extension_op()?;
Self::from_extension_op(ext).ok()
}

Expand Down
2 changes: 0 additions & 2 deletions hugr/src/hugr.rs
Original file line number Diff line number Diff line change
Expand Up @@ -203,8 +203,6 @@ impl Hugr {

/// Infer extension requirements and add new information to `op_types` field
/// (if the "extension_inference" feature is on; otherwise, do nothing)
///
/// See [`infer_extensions`] for details on the "closure" value
pub fn infer_extensions(&mut self) -> Result<(), InferExtensionError> {
#[cfg(feature = "extension_inference")]
{
Expand Down
8 changes: 4 additions & 4 deletions hugr/src/hugr/rewrite/replace.rs
Original file line number Diff line number Diff line change
Expand Up @@ -455,10 +455,10 @@ mod test {
use crate::hugr::hugrmut::sealed::HugrMutInternals;
use crate::hugr::rewrite::replace::WhichHugr;
use crate::hugr::{HugrMut, NodeType, Rewrite};
use crate::ops::custom::{ExternalOp, OpaqueOp};
use crate::ops::custom::{CustomOp, OpaqueOp};
use crate::ops::dataflow::DataflowOpTrait;
use crate::ops::handle::{BasicBlockID, ConstID, NodeHandle};
use crate::ops::{self, Case, CustomOp, DataflowBlock, OpTag, OpType, DFG};
use crate::ops::{self, Case, DataflowBlock, OpTag, OpType, DFG};
use crate::std_extensions::collections;
use crate::types::{FunctionType, Type, TypeArg, TypeRow};
use crate::{type_row, Direction, Hugr, HugrView, OutgoingPort};
Expand Down Expand Up @@ -650,13 +650,13 @@ mod test {
fn test_invalid() -> Result<(), Box<dyn std::error::Error>> {
let utou = FunctionType::new_endo(vec![USIZE_T]);
let mk_op = |s| {
CustomOp::new(ExternalOp::Opaque(OpaqueOp::new(
CustomOp::new_opaque(OpaqueOp::new(
ExtensionId::new("unknown_ext").unwrap(),
s,
String::new(),
vec![],
utou.clone(),
)))
))
};
let mut h = DFGBuilder::new(FunctionType::new(
type_row![USIZE_T, BOOL_T],
Expand Down
13 changes: 1 addition & 12 deletions hugr/src/hugr/serialize.rs
Original file line number Diff line number Diff line change
Expand Up @@ -347,18 +347,7 @@ pub mod test {
for node in new_hugr.nodes() {
let new_op = new_hugr.get_optype(node);
let old_op = h_canon.get_optype(node);
if let OpType::CustomOp(new_op) = new_op {
if let OpType::CustomOp(old_op) = old_op {
assert_eq!(
new_op.as_ref().clone().into_opaque(),
old_op.as_ref().clone().into_opaque()
);
} else {
panic!("Expected old_op to be a custom op");
}
} else {
assert_eq!(new_op, old_op);
}
assert_eq!(new_op, old_op);
}

// Check that the graphs are equivalent up to port renumbering.
Expand Down
27 changes: 13 additions & 14 deletions hugr/src/hugr/validate.rs
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@ use crate::extension::{
};

use crate::ops::custom::CustomOpError;
use crate::ops::custom::{resolve_opaque_op, ExternalOp};
use crate::ops::custom::{resolve_opaque_op, CustomOp};
use crate::ops::validate::{ChildrenEdgeData, ChildrenValidationError, EdgeValidationError};
use crate::ops::{FuncDefn, OpTag, OpTrait, OpType, ValidateOp};
use crate::types::type_param::TypeParam;
Expand Down Expand Up @@ -525,33 +525,32 @@ impl<'a, 'b> ValidationContext<'a, 'b> {
// The op_type must be defined only in terms of type variables defined outside the node
// TODO consider turning this match into a trait method?
match op_type {
OpType::CustomOp(b) => {
OpType::CustomOp(op) => {
// Try to resolve serialized names to actual OpDefs in Extensions.
let temp: ExternalOp;
let external = b.as_ref();
let resolved = match external {
ExternalOp::Opaque(op) => {
let temp: CustomOp;
let resolved = match op {
CustomOp::Opaque(opaque) => {
// If resolve_extension_ops has been called first, this would always return Ok(None)
match resolve_opaque_op(node, op, self.extension_registry)? {
match resolve_opaque_op(node, opaque, self.extension_registry)? {
Some(exten) => {
temp = ExternalOp::Extension(exten);
temp = CustomOp::new_extension(exten);
&temp
}
None => external,
None => op,
}
}
ExternalOp::Extension(_) => external,
CustomOp::Extension(_) => op,
};
// Check TypeArgs are valid, and if we can, fit the declared TypeParams
match resolved {
ExternalOp::Extension(exten) => exten
CustomOp::Extension(exten) => exten
.def()
.validate_args(exten.args(), self.extension_registry, var_decls)
.map_err(|cause| ValidationError::SignatureError { node, cause })?,
ExternalOp::Opaque(opaq) => {
CustomOp::Opaque(opaque) => {
// Best effort. Just check TypeArgs are valid in themselves, allowing any of them
// to contain type vars (we don't know how many are binary params, so accept if in doubt)
for arg in opaq.args() {
for arg in opaque.args() {
arg.validate(self.extension_registry, var_decls)
.map_err(|cause| ValidationError::SignatureError { node, cause })?;
}
Expand Down Expand Up @@ -699,7 +698,7 @@ pub enum ValidationError {
/// Error in a [CustomOp] serialized as an [Opaque]
///
/// [CustomOp]: crate::ops::CustomOp
/// [Opaque]: crate::ops::custom::ExternalOp::Opaque
/// [Opaque]: crate::ops::CustomOp::Opaque
#[error(transparent)]
CustomOpError(#[from] CustomOpError),
}
Expand Down
3 changes: 2 additions & 1 deletion hugr/src/ops.rs
Original file line number Diff line number Diff line change
Expand Up @@ -22,8 +22,9 @@ use enum_dispatch::enum_dispatch;

pub use constant::Const;
pub use controlflow::{BasicBlock, Case, Conditional, DataflowBlock, ExitBlock, TailLoop, CFG};
pub use custom::CustomOp;
pub use dataflow::{Call, CallIndirect, DataflowParent, Input, LoadConstant, Output, DFG};
pub use leaf::{CustomOp, Lift, MakeTuple, Noop, Tag, UnpackTuple};
pub use leaf::{Lift, MakeTuple, Noop, Tag, UnpackTuple};
pub use module::{AliasDecl, AliasDefn, FuncDecl, FuncDefn, Module};
pub use tag::OpTag;

Expand Down
Loading

0 comments on commit c4a5631

Please sign in to comment.