Address code review comments

Author: AmrDeveloper

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

@@ -61,7 +61,7 @@ def CIR_AnyIntOrGlobalViewAttr
   string cppType = "::mlir::TypedAttr";
 }
 
-def CIR_AnyGlobalViewOrCatchAllOrUnwindAttr
+def CIR_TryHandlerAttr
     : AnyAttrOf<[CIR_AnyGlobalViewAttr, CIR_AnyCatchAllAttr, CIR_AnyUnwindAttr],
                 "catch all or unwind or global view attribute"> {
   string cppType = "::mlir::TypedAttr";
@@ -79,8 +79,7 @@ def CIR_IntOrGlobalViewArrayAttr : TypedArrayAttrBase<CIR_AnyIntOrGlobalViewAttr
   string cppType = "::mlir::ArrayAttr";
 }
 
-def CIR_GlobalViewOrCatchAllOrUnwindArrayAttr
-    : TypedArrayAttrBase<CIR_AnyGlobalViewOrCatchAllOrUnwindAttr,
+def CIR_TryHandlerArrayAttr : TypedArrayAttrBase<CIR_TryHandlerAttr,
                          "catch all or unwind or global view array attribute">;
 
 #endif // CLANG_CIR_DIALECT_IR_CIRATTRCONSTRAINTS_TD

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

@@ -4366,19 +4366,19 @@ def CIR_TryOp : CIR_Op<"try",[
   let arguments = (ins
     UnitAttr:$synthetic,
     UnitAttr:$cleanup,
-    CIR_GlobalViewOrCatchAllOrUnwindArrayAttr:$catch_types
+    CIR_TryHandlerArrayAttr:$handler_types
   );
 
   let regions = (region
     AnyRegion:$try_region,
-    VariadicRegion<MinSizedRegion<1>>:$handlers
+    VariadicRegion<MinSizedRegion<1>>:$handler_regions
   );
 
   let assemblyFormat = [{
     (`synthetic` $synthetic^)?
     (`cleanup` $cleanup^)?
     $try_region
-    custom<CatchRegions>($handlers, $catch_types)
+    custom<TryHandlerRegions>($handler_regions, $handler_types)
     attr-dict
   }];
 
@@ -4387,10 +4387,11 @@ def CIR_TryOp : CIR_Op<"try",[
       "llvm::function_ref<void(mlir::OpBuilder &, "
         "mlir::Location)>":$tryBuilder,
       "llvm::function_ref<void(mlir::OpBuilder &, mlir::Location, "
-        "mlir::OperationState &)>":$catchBuilder),
+        "mlir::OperationState &)>":$handlersBuilder),
     [{
       assert(tryBuilder && "expected builder callback for 'cir.try' body");
-      assert(catchBuilder && "expected builder callback for 'catch' body");
+      assert(handlersBuilder
+        && "expected builder callback for 'handlers' body");
 
       OpBuilder::InsertionGuard guard($_builder);
 
@@ -4400,7 +4401,7 @@ def CIR_TryOp : CIR_Op<"try",[
       // Create try body region and set insertion point
       $_builder.createBlock(tryBodyRegion);
       tryBuilder($_builder, $_state.location);
-      catchBuilder($_builder, $_state.location, $_state);
+      handlersBuilder($_builder, $_state.location, $_state);
     }]>
   ];
 

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

@@ -2931,47 +2931,49 @@ void cir::TryOp::getSuccessorRegions(
 
   // TODO(CIR): If we know a target function never throws a specific type, we
   // can remove the catch handler.
-  for (mlir::Region &handler : this->getHandlers())
-    regions.push_back(mlir::RegionSuccessor(&handler));
+  for (mlir::Region &handlerRegion : this->getHandlerRegions())
+    regions.push_back(mlir::RegionSuccessor(&handlerRegion));
 }
 
-static void printCatchRegions(mlir::OpAsmPrinter &printer, cir::TryOp op,
-                              mlir::MutableArrayRef<mlir::Region> regions,
-                              mlir::ArrayAttr catchersAttr) {
-  if (!catchersAttr)
+static void
+printTryHandlerRegions(mlir::OpAsmPrinter &printer, cir::TryOp op,
+                       mlir::MutableArrayRef<mlir::Region> handlerRegions,
+                       mlir::ArrayAttr handlerTypes) {
+  if (!handlerTypes)
     return;
 
-  for (const auto [catcherIdx, catcherAttr] : llvm::enumerate(catchersAttr)) {
-    if (catcherIdx)
+  for (const auto [typeIdx, typeAttr] : llvm::enumerate(handlerTypes)) {
+    if (typeIdx)
       printer << " ";
 
-    if (mlir::isa<cir::CatchAllAttr>(catcherAttr)) {
+    if (mlir::isa<cir::CatchAllAttr>(typeAttr)) {
       printer << "catch all ";
-    } else if (mlir::isa<cir::UnwindAttr>(catcherAttr)) {
+    } else if (mlir::isa<cir::UnwindAttr>(typeAttr)) {
       printer << "unwind ";
     } else {
       printer << "catch [type ";
-      printer.printAttribute(catcherAttr);
+      printer.printAttribute(typeAttr);
       printer << "] ";
     }
 
-    printer.printRegion(regions[catcherIdx], /*printEntryBLockArgs=*/false,
+    printer.printRegion(handlerRegions[typeIdx],
+                        /*printEntryBLockArgs=*/false,
                         /*printBlockTerminators=*/true);
   }
 }
 
-static mlir::ParseResult
-parseCatchRegions(mlir::OpAsmParser &parser,
-                  llvm::SmallVectorImpl<std::unique_ptr<mlir::Region>> &regions,
-                  mlir::ArrayAttr &catchersAttr) {
+static mlir::ParseResult parseTryHandlerRegions(
+    mlir::OpAsmParser &parser,
+    llvm::SmallVectorImpl<std::unique_ptr<mlir::Region>> &handlerRegions,
+    mlir::ArrayAttr &handlerTypes) {
 
   auto parseCheckedCatcherRegion = [&]() -> mlir::ParseResult {
-    regions.emplace_back(new mlir::Region);
+    handlerRegions.emplace_back(new mlir::Region);
 
-    mlir::Region &currRegion = *regions.back();
+    mlir::Region &currRegion = *handlerRegions.back();
     mlir::SMLoc regionLoc = parser.getCurrentLocation();
     if (parser.parseRegion(currRegion)) {
-      regions.clear();
+      handlerRegions.clear();
       return failure();
     }
 
@@ -3032,7 +3034,7 @@ parseCatchRegions(mlir::OpAsmParser &parser,
       return mlir::failure();
   }
 
-  catchersAttr = parser.getBuilder().getArrayAttr(catcherAttrs);
+  handlerTypes = parser.getBuilder().getArrayAttr(catcherAttrs);
   return mlir::success();
 }
 

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

@@ -22,7 +22,7 @@ module {
 
 cir.func dso_local @invalid_catch_rtti_type() {
   cir.scope {
-    // expected-error @below {{'cir.try' op attribute 'catch_types' failed to satisfy constraint: catch all or unwind or global view array attribute}}
+    // expected-error @below {{'cir.try' op attribute 'handler_types' failed to satisfy constraint: catch all or unwind or global view array attribute}}
     cir.try {
       cir.yield
     } catch [type #cir.undef] {
@@ -40,7 +40,7 @@ module {
 
 cir.func dso_local @invalid_catch_empty_block() {
   cir.scope {
-    // expected-error @below {{'cir.try' op region #1 ('catch_regions') failed to verify constraint: region with at least 1 blocks}}
+    // expected-error @below {{'cir.try' op region #1 ('handler_regions') failed to verify constraint: region with at least 1 blocks}}
     cir.try {
       cir.yield
     } catch all {