address feedback, rename -> op{,def}_id

Author: doug-q

hugr-core/src/export.rs

@@ -447,7 +447,7 @@ impl<'a> Context<'a> {
             }
 
             OpType::OpaqueOp(op) => {
-                let node = self.make_named_global_ref(op.extension(), op.op_name());
+                let node = self.make_named_global_ref(op.extension(), op.unqualified_id());
                 let params = self
                     .bump
                     .alloc_slice_fill_iter(op.args().iter().map(|arg| self.export_type_arg(arg)));

hugr-core/src/extension.rs

@@ -466,8 +466,8 @@ trait CustomConcrete {
 impl CustomConcrete for OpaqueOp {
     type Identifier = OpName;
 
-    fn def_name(&self) -> &OpName {
-        self.op_name()
+    fn def_name(&self) -> &Self::Identifier {
+        self.unqualified_id()
     }
 
     fn type_args(&self) -> &[TypeArg] {

hugr-core/src/extension/prelude.rs

@@ -597,8 +597,8 @@ impl ConstFold for TupleOpDef {
 }
 
 impl MakeOpDef for TupleOpDef {
-    fn opdef_name(&self) -> OpName {
-        <&Self as Into<&'static str>>::into(self).into()
+    fn opdef_id(&self) -> OpName {
+        <&'static str>::from(self).into()
     }
 
     fn init_signature(&self, _extension_ref: &Weak<Extension>) -> SignatureFunc {
@@ -655,8 +655,8 @@ impl MakeTuple {
 }
 
 impl MakeExtensionOp for MakeTuple {
-    fn name(&self) -> OpName {
-        TupleOpDef::MakeTuple.opdef_name()
+    fn op_id(&self) -> OpName {
+        TupleOpDef::MakeTuple.opdef_id()
     }
 
     fn from_extension_op(ext_op: &crate::ops::ExtensionOp) -> Result<Self, OpLoadError>
@@ -665,9 +665,7 @@ impl MakeExtensionOp for MakeTuple {
     {
         let def = TupleOpDef::from_def(ext_op.def())?;
         if def != TupleOpDef::MakeTuple {
-            return Err(OpLoadError::NotMember(
-                ext_op.unqualified_name().to_string(),
-            ))?;
+            return Err(OpLoadError::NotMember(ext_op.unqualified_id().to_string()))?;
         }
         let [TypeArg::Sequence { elems }] = ext_op.args() else {
             return Err(SignatureError::InvalidTypeArgs)?;
@@ -717,8 +715,8 @@ impl UnpackTuple {
 }
 
 impl MakeExtensionOp for UnpackTuple {
-    fn name(&self) -> OpName {
-        TupleOpDef::UnpackTuple.opdef_name()
+    fn op_id(&self) -> OpName {
+        TupleOpDef::UnpackTuple.opdef_id()
     }
 
     fn from_extension_op(ext_op: &crate::ops::ExtensionOp) -> Result<Self, OpLoadError>
@@ -727,9 +725,7 @@ impl MakeExtensionOp for UnpackTuple {
     {
         let def = TupleOpDef::from_def(ext_op.def())?;
         if def != TupleOpDef::UnpackTuple {
-            return Err(OpLoadError::NotMember(
-                ext_op.unqualified_name().to_string(),
-            ))?;
+            return Err(OpLoadError::NotMember(ext_op.unqualified_id().to_string()))?;
         }
         let [TypeArg::Sequence { elems }] = ext_op.args() else {
             return Err(SignatureError::InvalidTypeArgs)?;
@@ -776,7 +772,7 @@ impl std::str::FromStr for NoopDef {
     type Err = ();
 
     fn from_str(s: &str) -> Result<Self, Self::Err> {
-        if s == NoopDef.name() {
+        if s == NoopDef.op_id() {
             Ok(Self)
         } else {
             Err(())
@@ -785,7 +781,7 @@ impl std::str::FromStr for NoopDef {
 }
 
 impl MakeOpDef for NoopDef {
-    fn opdef_name(&self) -> OpName {
+    fn opdef_id(&self) -> OpName {
         NOOP_OP_ID
     }
 
@@ -845,8 +841,8 @@ impl Default for Noop {
 }
 
 impl MakeExtensionOp for Noop {
-    fn name(&self) -> OpName {
-        NoopDef.name()
+    fn op_id(&self) -> OpName {
+        NoopDef.op_id()
     }
 
     fn from_extension_op(ext_op: &crate::ops::ExtensionOp) -> Result<Self, OpLoadError>
@@ -886,7 +882,7 @@ impl std::str::FromStr for BarrierDef {
     type Err = ();
 
     fn from_str(s: &str) -> Result<Self, Self::Err> {
-        if s == BarrierDef.name() {
+        if s == BarrierDef.op_id() {
             Ok(Self)
         } else {
             Err(())
@@ -895,7 +891,7 @@ impl std::str::FromStr for BarrierDef {
 }
 
 impl MakeOpDef for BarrierDef {
-    fn opdef_name(&self) -> OpName {
+    fn opdef_id(&self) -> OpName {
         BARRIER_OP_ID
     }
 
@@ -945,13 +941,13 @@ impl Barrier {
 
 impl NamedOp for Barrier {
     fn name(&self) -> OpName {
-        BarrierDef.name()
+        BarrierDef.op_id()
     }
 }
 
 impl MakeExtensionOp for Barrier {
-    fn name(&self) -> OpName {
-        BarrierDef.name()
+    fn op_id(&self) -> OpName {
+        BarrierDef.op_id()
     }
 
     fn from_extension_op(ext_op: &crate::ops::ExtensionOp) -> Result<Self, OpLoadError>

hugr-core/src/extension/prelude/generic.rs

@@ -37,7 +37,7 @@ impl FromStr for LoadNatDef {
     type Err = ();
 
     fn from_str(s: &str) -> Result<Self, Self::Err> {
-        if s == Self.name() {
+        if s == Self.op_id() {
             Ok(Self)
         } else {
             Err(())
@@ -65,7 +65,7 @@ impl ConstFold for LoadNatDef {
 }
 
 impl MakeOpDef for LoadNatDef {
-    fn opdef_name(&self) -> OpName {
+    fn opdef_id(&self) -> OpName {
         LOAD_NAT_OP_ID.clone()
     }
 
@@ -118,8 +118,8 @@ impl LoadNat {
 }
 
 impl MakeExtensionOp for LoadNat {
-    fn name(&self) -> OpName {
-        LoadNatDef.opdef_name()
+    fn op_id(&self) -> OpName {
+        LoadNatDef.opdef_id()
     }
 
     fn from_extension_op(ext_op: &ExtensionOp) -> Result<Self, OpLoadError>

hugr-core/src/extension/resolution/ops.rs

@@ -73,7 +73,7 @@ pub(crate) fn resolve_op_extensions<'e>(
     // Fail if the Extension is not in the registry, or if the Extension was
     // found but did not have the expected operation.
     let extension = extension.expect("OpaqueOp should have an extension");
-    let Some(def) = extension.get_op(opaque.op_name()) else {
+    let Some(def) = extension.get_op(opaque.unqualified_id()) else {
         return Err(OpaqueOpError::OpNotFoundInExtension {
             node,
             op: opaque.name().clone(),

hugr-core/src/extension/simple_op.rs

@@ -30,9 +30,18 @@ pub enum OpLoadError {
 ///
 /// Particularly useful with C-style enums that implement [strum::IntoEnumIterator],
 /// as then all definitions can be added to an extension at once.
+///
+/// [MakeExtensionOp] has a blanket impl for types that impl [MakeOpDef].
 pub trait MakeOpDef {
-    /// TODO docs
-    fn opdef_name(&self) -> OpName;
+    /// The [OpDef::name] which will be used when `Self`  is added to an [Extension]
+    /// or when `Self` is loaded from an [OpDef].
+    ///
+    /// This identifer must be unique within the extension with which the
+    /// [OpDef] is registered. An [ExtensionOp] instantiating this [OpDef] will
+    /// report `self.opdef_id()` as its [ExtensionOp::unqualified_id].
+    ///
+    /// [MakeExtensionOp::op_id] must match this function.
+    fn opdef_id(&self) -> OpName;
 
     /// Try to load one of the operations of this set from an [OpDef].
     fn from_def(op_def: &OpDef) -> Result<Self, OpLoadError>
@@ -57,9 +66,9 @@ pub trait MakeOpDef {
         self.init_signature(&self.extension_ref())
     }
 
-    /// Description of the operation. By default, the same as `self.name()`.
+    /// Description of the operation. By default, the same as `self.opdef_id()`.
     fn description(&self) -> String {
-        self.opdef_name().to_string()
+        self.opdef_id().to_string()
     }
 
     /// Edit the opdef before finalising. By default does nothing.
@@ -76,7 +85,7 @@ pub trait MakeOpDef {
         extension_ref: &Weak<Extension>,
     ) -> Result<(), ExtensionBuildError> {
         let def = extension.add_op(
-            self.opdef_name(),
+            self.opdef_id(),
             self.description(),
             self.init_signature(extension_ref),
             extension_ref,
@@ -140,8 +149,12 @@ pub trait HasDef: MakeExtensionOp {
 /// Traits implemented by types which can be loaded from [`ExtensionOp`]s,
 /// i.e. concrete instances of [`OpDef`]s, with defined type arguments.
 pub trait MakeExtensionOp {
-    /// The name of the operation
-    fn name(&self) -> OpName;
+    /// The [OpDef::name] of [ExtensionOp]s from which `Self` can be loaded.
+    ///
+    /// This identifer must be unique within the extension with which the
+    /// [OpDef] is registered. An [ExtensionOp] instantiating this [OpDef] will
+    /// report `self.opdef_id()` as its [ExtensionOp::unqualified_id].
+    fn op_id(&self) -> OpName;
 
     /// Try to load one of the operations of this set from an [OpDef].
     fn from_extension_op(ext_op: &ExtensionOp) -> Result<Self, OpLoadError>
@@ -180,8 +193,8 @@ pub trait MakeExtensionOp {
 
 /// Blanket implementation for non-polymorphic operations - [OpDef]s with no type parameters.
 impl<T: MakeOpDef> MakeExtensionOp for T {
-    fn name(&self) -> OpName {
-        <Self as MakeOpDef>::opdef_name(self)
+    fn op_id(&self) -> OpName {
+        self.opdef_id()
     }
 
     #[inline]
@@ -239,7 +252,7 @@ impl<T: MakeExtensionOp> RegisteredOp<T> {
     /// Generate an [OpType].
     pub fn to_extension_op(&self) -> Option<ExtensionOp> {
         ExtensionOp::new(
-            self.extension.upgrade()?.get_op(&self.name())?.clone(),
+            self.extension.upgrade()?.get_op(&self.op_id())?.clone(),
             self.type_args(),
         )
         .ok()
@@ -248,7 +261,7 @@ impl<T: MakeExtensionOp> RegisteredOp<T> {
     delegate! {
         to self.op {
             /// Name of the operation - derived from strum serialization.
-            pub fn name(&self) -> OpName;
+            pub fn op_id(&self) -> OpName;
             /// Any type args which define this operation. Default is no type arguments.
             pub fn type_args(&self) -> Vec<TypeArg>;
         }
@@ -306,8 +319,8 @@ mod test {
     }
 
     impl MakeOpDef for DummyEnum {
-        fn opdef_name(&self) -> OpName {
-            <&Self as Into<&'static str>>::into(self).into()
+        fn opdef_id(&self) -> OpName {
+            <&'static str>::from(self).into()
         }
 
         fn init_signature(&self, _extension_ref: &Weak<Extension>) -> SignatureFunc {
@@ -366,7 +379,7 @@ mod test {
         let o = DummyEnum::Dumb;
 
         assert_eq!(
-            DummyEnum::from_def(EXT.get_op(&o.opdef_name()).unwrap()).unwrap(),
+            DummyEnum::from_def(EXT.get_op(&o.opdef_id()).unwrap()).unwrap(),
             o
         );
 

hugr-core/src/hugr/validate.rs

@@ -102,7 +102,7 @@ impl<'a> ValidationContext<'a> {
         if let OpType::OpaqueOp(opaque) = op_type {
             Err(OpaqueOpError::UnresolvedOp(
                 node,
-                opaque.op_name().clone(),
+                opaque.unqualified_id().clone(),
                 opaque.extension().clone(),
             ))?;
         }
@@ -536,7 +536,7 @@ impl<'a> ValidationContext<'a> {
             OpType::OpaqueOp(opaque) => {
                 Err(OpaqueOpError::UnresolvedOp(
                     node,
-                    opaque.op_name().clone(),
+                    opaque.qualified_id(),
                     opaque.extension().clone(),
                 ))?;
             }

hugr-core/src/ops/custom.rs

@@ -137,15 +137,15 @@ impl ExtensionOp {
         self.def.extension_id()
     }
 
-    /// Returns the unqualified name of the operation. e.g. 'iadd'
+    /// Returns the unqualified id of the operation. e.g. 'iadd'
     ///
-    pub fn unqualified_name(&self) -> &OpNameRef {
+    pub fn unqualified_id(&self) -> &OpNameRef {
         self.def.name()
     }
 
-    /// Returns the unqualified name of the operation. e.g. 'arithmetic.iadd'
-    pub fn qualified_name(&self) -> OpName {
-        self.name()
+    /// Returns the qualified id of the operation. e.g. 'arithmetic.iadd'
+    pub fn qualified_id(&self) -> OpName {
+        qualify_name(self.extension_id(), self.unqualified_id())
     }
 }
 
@@ -177,7 +177,7 @@ impl Eq for ExtensionOp {}
 impl NamedOp for ExtensionOp {
     /// The name of the operation.
     fn name(&self) -> OpName {
-        qualify_name(self.def.extension_id(), self.def.name())
+        self.qualified_id()
     }
 }
 
@@ -260,18 +260,22 @@ impl OpaqueOp {
 }
 
 impl NamedOp for OpaqueOp {
-    /// The name of the operation.
     fn name(&self) -> OpName {
-        format!("OpaqueOp:{}", qualify_name(&self.extension, &self.name)).into()
+        format!("OpaqueOp:{}", self.qualified_id()).into()
     }
 }
 
 impl OpaqueOp {
     /// Unique name of the operation.
-    pub fn op_name(&self) -> &OpName {
+    pub fn unqualified_id(&self) -> &OpName {
         &self.name
     }
 
+    /// Unique name of the operation.
+    pub fn qualified_id(&self) -> OpName {
+        qualify_name(self.extension(), self.unqualified_id())
+    }
+
     /// Type arguments.
     pub fn args(&self) -> &[TypeArg] {
         &self.args

hugr-core/src/std_extensions/arithmetic/conversions.rs

@@ -44,8 +44,8 @@ pub enum ConvertOpDef {
 }
 
 impl MakeOpDef for ConvertOpDef {
-    fn opdef_name(&self) -> OpName {
-        <&Self as Into<&'static str>>::into(self).into()
+    fn opdef_id(&self) -> OpName {
+        <&'static str>::from(self).into()
     }
 
     fn from_def(op_def: &OpDef) -> Result<Self, OpLoadError> {
@@ -150,8 +150,8 @@ impl ConvertOpType {
 }
 
 impl MakeExtensionOp for ConvertOpType {
-    fn name(&self) -> OpName {
-        self.def.opdef_name()
+    fn op_id(&self) -> OpName {
+        self.def.opdef_id()
     }
 
     fn from_extension_op(ext_op: &ExtensionOp) -> Result<Self, OpLoadError> {

hugr-core/src/std_extensions/arithmetic/float_ops.rs

@@ -49,7 +49,7 @@ pub enum FloatOps {
 }
 
 impl MakeOpDef for FloatOps {
-    fn opdef_name(&self) -> OpName {
+    fn opdef_id(&self) -> OpName {
         <&Self as Into<&'static str>>::into(self).into()
     }