Address code review comments

Author: AmrDeveloper

clang/include/clang/CIR/Dialect/IR/CIROps.td

@@ -4346,11 +4346,27 @@ def CIR_TryOp : CIR_Op<"try",[
   }];
 
   // Everything already covered elsewhere.
-  let builders = [OpBuilder<(ins
+  let builders = [
+    OpBuilder<(ins
       "llvm::function_ref<void(mlir::OpBuilder &, "
         "mlir::Location)>":$tryBuilder,
       "llvm::function_ref<void(mlir::OpBuilder &, mlir::Location, "
-        "mlir::OperationState &)>":$catchBuilder)>];
+        "mlir::OperationState &)>":$catchBuilder),
+    [{
+      assert(tryBuilder && "expected builder callback for 'cir.try' body");
+      assert(catchBuilder && "expected builder callback for 'catch' body");
+
+      OpBuilder::InsertionGuard guard($_builder);
+
+      // Try body region
+      Region *tryBodyRegion = $_state.addRegion();
+
+      // Create try body region and set insertion point
+      $_builder.createBlock(tryBodyRegion);
+      tryBuilder($_builder, $_state.location);
+      catchBuilder($_builder, $_state.location, $_state);
+    }]>
+  ];
 
   let hasLLVMLowering = false;
 }

clang/lib/CIR/Dialect/IR/CIRDialect.cpp

@@ -2882,24 +2882,6 @@ LogicalResult cir::TypeInfoAttr::verify(
 // TryOp
 //===----------------------------------------------------------------------===//
 
-void cir::TryOp::build(
-    OpBuilder &builder, OperationState &result,
-    function_ref<void(OpBuilder &, Location)> tryBuilder,
-    function_ref<void(OpBuilder &, Location, OperationState &)> catchBuilder) {
-  assert(tryBuilder && "expected builder callback for 'cir.try' body");
-  assert(catchBuilder && "expected builder callback for 'catch' body");
-
-  OpBuilder::InsertionGuard guard(builder);
-
-  // Try body region
-  Region *tryBodyRegion = result.addRegion();
-
-  // Create try body region and set insertion point
-  builder.createBlock(tryBodyRegion);
-  tryBuilder(builder, result.location);
-  catchBuilder(builder, result.location, result);
-}
-
 void cir::TryOp::getSuccessorRegions(
     mlir::RegionBranchPoint point, SmallVectorImpl<RegionSuccessor> &regions) {
   // If any index all the underlying regions branch back to the parent

clang/test/CIR/IR/invalid-try-catch.cir

@@ -0,0 +1,75 @@
+// RUN: cir-opt %s -verify-diagnostics -split-input-file
+
+module {
+
+cir.func dso_local @invalid_catch_without_all_or_type() {
+  cir.scope {
+    cir.try {
+      cir.yield
+     // expected-error @below {{'cir.try' expected attribute, 'all', or 'type' keyword}}
+    } catch [invalid_keyword {
+      cir.yield
+    }]
+  }
+  cir.return
+}
+
+}
+
+// -----
+
+module {
+
+cir.func dso_local @invalid_catch_rtti_type() {
+  cir.scope {
+    cir.try {
+      cir.yield
+     // expected-error @below {{expected attribute value}}
+     // expected-error @below {{'cir.try' expected valid RTTI info attribute}}
+    } catch [type invalid_type {
+      cir.yield
+    }]
+  }
+  cir.return
+}
+
+}
+
+// -----
+
+module {
+
+cir.func dso_local @invalid_catch_empty_block() {
+  cir.scope {
+    cir.try {
+      cir.yield
+    } catch [type #cir.all {
+      // expected-error @below {{'cir.try' catch region shall not be empty}}
+    }]
+  }
+  cir.return
+}
+
+}
+
+// -----
+
+!s32i = !cir.int<s, 32>
+
+module {
+
+cir.func dso_local @invalid_catch_not_terminated() {
+  %a = cir.alloca !s32i, !cir.ptr<!s32i>, ["a", init]
+  cir.scope {
+    cir.try {
+      cir.yield
+    } 
+    // expected-error @below {{'cir.try' blocks are expected to be explicitly terminated}}
+    catch [type #cir.all {
+       %tmp_a = cir.load %a : !cir.ptr<!s32i>, !s32i
+    }]
+  }
+  cir.return
+}
+
+}