Remove verify from OperationBuilder and SameOperandsAndResultType verification implementation

dialects/arith/src/ops/unary.rs

@@ -28,7 +28,7 @@ macro_rules! infer_return_ty_for_unary_op {
 /// Increment
 #[operation (
         dialect = ArithDialect,
-        traits(UnaryOp, SameOperandsAndResultType),
+        traits(UnaryOp, SameTypeOperands, SameOperandsAndResultType),
         implements(InferTypeOpInterface, MemoryEffectOpInterface)
     )]
 pub struct Incr {
@@ -44,7 +44,7 @@ has_no_effects!(Incr);
 /// Negation
 #[operation (
         dialect = ArithDialect,
-        traits(UnaryOp, SameOperandsAndResultType),
+        traits(UnaryOp, SameTypeOperands, SameOperandsAndResultType),
         implements(InferTypeOpInterface, MemoryEffectOpInterface)
     )]
 pub struct Neg {
@@ -60,7 +60,7 @@ has_no_effects!(Neg);
 /// Modular inverse
 #[operation (
         dialect = ArithDialect,
-        traits(UnaryOp, SameOperandsAndResultType),
+        traits(UnaryOp, SameTypeOperands, SameOperandsAndResultType),
         implements(InferTypeOpInterface, MemoryEffectOpInterface)
     )]
 pub struct Inv {
@@ -76,7 +76,7 @@ has_no_effects!(Inv);
 /// log2(operand)
 #[operation (
         dialect = ArithDialect,
-        traits(UnaryOp, SameOperandsAndResultType),
+        traits(UnaryOp, SameTypeOperands, SameOperandsAndResultType),
         implements(InferTypeOpInterface, MemoryEffectOpInterface)
     )]
 pub struct Ilog2 {
@@ -92,7 +92,7 @@ has_no_effects!(Ilog2);
 /// pow2(operand)
 #[operation (
         dialect = ArithDialect,
-        traits(UnaryOp, SameOperandsAndResultType),
+        traits(UnaryOp, SameTypeOperands, SameOperandsAndResultType),
         implements(InferTypeOpInterface, MemoryEffectOpInterface)
     )]
 pub struct Pow2 {
@@ -108,7 +108,7 @@ has_no_effects!(Pow2);
 /// Logical NOT
 #[operation (
         dialect = ArithDialect,
-        traits(UnaryOp, SameOperandsAndResultType),
+        traits(UnaryOp, SameTypeOperands, SameOperandsAndResultType),
         implements(InferTypeOpInterface, MemoryEffectOpInterface)
 
     )]
@@ -125,7 +125,7 @@ has_no_effects!(Not);
 /// Bitwise NOT
 #[operation (
         dialect = ArithDialect,
-        traits(UnaryOp, SameOperandsAndResultType),
+        traits(UnaryOp, SameTypeOperands, SameOperandsAndResultType),
         implements(InferTypeOpInterface, MemoryEffectOpInterface)
     )]
 pub struct Bnot {

dialects/hir/src/ops/primop.rs

@@ -4,7 +4,7 @@ use crate::HirDialect;
 
 #[operation(
     dialect = HirDialect,
-    traits(SameOperandsAndResultType),
+    traits(SameTypeOperands, SameOperandsAndResultType),
     implements(InferTypeOpInterface, MemoryEffectOpInterface)
 )]
 pub struct MemGrow {

dialects/hir/src/ops/spills.rs

@@ -5,7 +5,7 @@ use crate::HirDialect;
 
 #[operation(
     dialect = HirDialect,
-    traits(SameOperandsAndResultType),
+    traits(SameTypeOperands, SameOperandsAndResultType),
     implements(MemoryEffectOpInterface, SpillLike)
 )]
 pub struct Spill {
@@ -34,7 +34,7 @@ impl EffectOpInterface<MemoryEffect> for Spill {
 
 #[operation(
     dialect = HirDialect,
-    traits(SameOperandsAndResultType),
+    traits(SameTypeOperands, SameOperandsAndResultType),
     implements(InferTypeOpInterface, MemoryEffectOpInterface, ReloadLike)
 )]
 pub struct Reload {

hir/src/derive.rs

@@ -1,11 +1,16 @@
 pub use midenc_hir_macros::operation;
 
 /// This macro is used to generate the boilerplate for operation trait implementations.
+/// Super traits have to be declared as a comma separated list of traits, instead of the traditional
+/// "+" separated list of traits.
+/// Example:
+///
+/// pub trait SomeTrait: SuperTraitA, SuperTraitB {}
 #[macro_export]
 macro_rules! derive {
     (
         $(#[$outer:meta])*
-        $vis:vis trait $OpTrait:ident {
+        $vis:vis trait $OpTrait:ident $(:)? $( $ParentTrait:ident ),* $(,)? {
             $(
                 $OpTraitItem:item
             )*
@@ -21,7 +26,7 @@ macro_rules! derive {
     ) => {
         $crate::__derive_op_trait! {
             $(#[$outer])*
-            $vis trait $OpTrait {
+            $vis trait $OpTrait : $( $ParentTrait , )*   {
                 $(
                     $OpTraitItem:item
                 )*
@@ -65,7 +70,7 @@ macro_rules! derive {
 macro_rules! __derive_op_trait {
     (
         $(#[$outer:meta])*
-        $vis:vis trait $OpTrait:ident {
+        $vis:vis trait $OpTrait:ident $(:)? $( $ParentTrait:ident ),* $(,)? {
             $(
                 $OpTraitItem:item
             )*
@@ -78,7 +83,7 @@ macro_rules! __derive_op_trait {
         }
     ) => {
         $(#[$outer])*
-        $vis trait $OpTrait {
+        $vis trait $OpTrait : $( $ParentTrait + )* {
             $(
                 $OpTraitItem
             )*
@@ -87,12 +92,19 @@ macro_rules! __derive_op_trait {
         impl<T: $crate::Op + $OpTrait> $crate::Verify<dyn $OpTrait> for T {
             #[inline]
             fn verify(&self, context: &$crate::Context) -> Result<(), $crate::Report> {
+                $(
+                <$crate::Operation as $crate::Verify<dyn $ParentTrait>>::verify(self.as_operation(), context)?;
+                 )*
                 <$crate::Operation as $crate::Verify<dyn $OpTrait>>::verify(self.as_operation(), context)
             }
         }
 
         impl $crate::Verify<dyn $OpTrait> for $crate::Operation {
             fn should_verify(&self, _context: &$crate::Context) -> bool {
+                $(
+                    self.implements::<dyn $ParentTrait>()
+                    &&
+                )*
                 self.implements::<dyn $OpTrait>()
             }
 
@@ -136,7 +148,8 @@ mod tests {
 
     use crate::{
         attributes::Overflow,
-        dialects::test::{self, Add},
+        dialects::test::{self, Add, InvalidOpsWithReturn},
+        pass::{Nesting, PassManager},
         Builder, BuilderExt, Context, Op, Operation, Report, Spanned,
     };
 
@@ -191,25 +204,64 @@ mod tests {
         ));
     }
 
-    #[ignore = "until https://github.com/0xMiden/compiler/issues/378 is fixed"]
     #[test]
     #[should_panic = "expected 'u32', got 'i64'"]
     fn derived_op_verifier_test() {
         use crate::{SourceSpan, Type};
 
         let context = Rc::new(Context::default());
+
         let block = context.create_block_with_params([Type::U32, Type::I64]);
+
+        context.get_or_register_dialect::<test::TestDialect>();
+        context.registered_dialects();
+
         let (lhs, invalid_rhs) = {
             let block = block.borrow();
             let lhs = block.get_argument(0).upcast::<dyn crate::Value>();
             let rhs = block.get_argument(1).upcast::<dyn crate::Value>();
             (lhs, rhs)
         };
-        let mut builder = context.builder();
+
+        let mut builder = context.clone().builder();
         builder.set_insertion_point_to_end(block);
         // Try to create instance of AddOp with mismatched operand types
         let op_builder = builder.create::<Add, _>(SourceSpan::default());
         let op = op_builder(lhs, invalid_rhs, Overflow::Wrapping);
-        let _op = op.unwrap();
+        let op = op.unwrap();
+
+        // Construct a pass manager with the default pass pipeline
+        let mut pm = PassManager::on::<Add>(context.clone(), Nesting::Implicit);
+        // Run pass pipeline
+        pm.run(op.as_operation_ref()).unwrap();
+    }
+
+    /// Fails if [`InvalidOpsWithReturn`] is created successfully. [`InvalidOpsWithReturn`] is a
+    /// struct that has differing types in its result and arguments, despite implementing the
+    /// [`SameOperandsAndResultType`] trait.
+    #[test]
+    #[should_panic = "expected 'i32', got 'u64'"]
+    fn same_operands_and_result_type_verifier_test() {
+        use crate::{SourceSpan, Type};
+
+        let context = Rc::new(Context::default());
+        let block = context.create_block_with_params([Type::I32, Type::I32]);
+        let (lhs, rhs) = {
+            let block = block.borrow();
+            let lhs = block.get_argument(0).upcast::<dyn crate::Value>();
+            let rhs = block.get_argument(1).upcast::<dyn crate::Value>();
+            (lhs, rhs)
+        };
+        let mut builder = context.clone().builder();
+        builder.set_insertion_point_to_end(block);
+        // Try to create instance of AddOp with mismatched operand types
+        let op_builder = builder.create::<InvalidOpsWithReturn, _>(SourceSpan::default());
+        let op = op_builder(lhs, rhs);
+        let op = op.unwrap();
+
+        // Construct a pass manager with the default pass pipeline
+        let mut pm = PassManager::on::<InvalidOpsWithReturn>(context.clone(), Nesting::Implicit);
+        // Run pass pipeline
+        pm.run(op.as_operation_ref()).unwrap();
     }
 }

hir/src/dialects/test.rs

@@ -115,6 +115,7 @@ impl DialectRegistration for TestDialect {
 
     fn register_operations(info: &mut DialectInfo) {
         info.register_operation::<ops::Add>();
+        info.register_operation::<ops::InvalidOpsWithReturn>();
         info.register_operation::<ops::Mul>();
         info.register_operation::<ops::Shl>();
         info.register_operation::<ops::Ret>();

hir/src/dialects/test/ops/binary.rs

@@ -74,3 +74,25 @@ impl InferTypeOpInterface for Shl {
         Ok(())
     }
 }
+
+/// Invalid operation that breaks the SameOperandsAndResultType trait (used for testing).
+#[operation(
+    dialect = TestDialect,
+    traits(BinaryOp, SameTypeOperands, SameOperandsAndResultType),
+    implements(InferTypeOpInterface)
+)]
+pub struct InvalidOpsWithReturn {
+    #[operand]
+    lhs: AnyInteger,
+    #[operand]
+    rhs: AnyInteger,
+    #[result]
+    result: AnyUnsignedInteger,
+}
+
+impl InferTypeOpInterface for InvalidOpsWithReturn {
+    fn infer_return_types(&mut self, _context: &Context) -> Result<(), Report> {
+        self.result_mut().set_type(Type::U64);
+        Ok(())
+    }
+}

hir/src/ir/operation/builder.rs

@@ -256,12 +256,6 @@ where
             unsafe { UnsafeIntrusiveEntityRef::from_raw(op.container().cast()) }
         };
 
-        // Run op-specific verification
-        {
-            let op: super::EntityRef<T> = op.borrow();
-            op.verify(self.builder.context())?;
-        }
-
         // Insert op at current insertion point, if set
         if self.builder.insertion_point().is_valid() {
             self.builder.insert(self.op);