[clang] 49dbeca - [CIR] Fix missing terminators in regions with cleanup (#187604)

[via cfe-commits]

Author: Andy Kaylor
Date: 2026-03-31T10:54:56-07:00
New Revision: 49dbeca38d8faeef42d7cb03db649fbfb4c195c4

URL: https://github.com/llvm/llvm-project/commit/49dbeca38d8faeef42d7cb03db649fbfb4c195c4
DIFF: https://github.com/llvm/llvm-project/commit/49dbeca38d8faeef42d7cb03db649fbfb4c195c4.diff

LOG: [CIR] Fix missing terminators in regions with cleanup (#187604)

When a cleanup scope was created within an if operation's then or else
region and the source scope ended with a return, we would fail to
terminate the region following the cleanup scope, which surrounded the
return operation, resulting in an MLIR verification error. This change
forces the addition of terminators in the if's then and else regions.

Added: 
    clang/test/CIR/CodeGen/return-in-if-with-cleanups.cpp

Modified: 
    clang/lib/CIR/CodeGen/CIRGenExpr.cpp
    clang/lib/CIR/CodeGen/CIRGenFunction.h
    clang/lib/CIR/CodeGen/CIRGenStmt.cpp

Removed: 
    


################################################################################
diff  --git a/clang/lib/CIR/CodeGen/CIRGenExpr.cpp b/clang/lib/CIR/CodeGen/CIRGenExpr.cpp
index 7f3e476b006c1..65644bc1a3fd9 100644
--- a/clang/lib/CIR/CodeGen/CIRGenExpr.cpp
+++ b/clang/lib/CIR/CodeGen/CIRGenExpr.cpp
@@ -2278,9 +2278,15 @@ cir::IfOp CIRGenFunction::emitIfOnBoolExpr(
 
   // Emit the code with the fully general case.
   mlir::Value condV = emitOpOnBoolExpr(loc, cond);
-  return cir::IfOp::create(builder, loc, condV, elseLoc.has_value(),
-                           /*thenBuilder=*/thenBuilder,
-                           /*elseBuilder=*/elseBuilder);
+  cir::IfOp ifOp = cir::IfOp::create(builder, loc, condV, elseLoc.has_value(),
+                                     /*thenBuilder=*/thenBuilder,
+                                     /*elseBuilder=*/elseBuilder);
+  terminateStructuredRegionBody(ifOp.getThenRegion(), thenLoc);
+  assert((elseLoc.has_value() || ifOp.getElseRegion().empty()) &&
+         "else region created with no else location");
+  if (elseLoc.has_value())
+    terminateStructuredRegionBody(ifOp.getElseRegion(), *elseLoc);
+  return ifOp;
 }
 
 /// TODO(cir): see EmitBranchOnBoolExpr for extra ideas).

diff  --git a/clang/lib/CIR/CodeGen/CIRGenFunction.h b/clang/lib/CIR/CodeGen/CIRGenFunction.h
index d80c2d635a89f..0bd440a61db20 100644
--- a/clang/lib/CIR/CodeGen/CIRGenFunction.h
+++ b/clang/lib/CIR/CodeGen/CIRGenFunction.h
@@ -972,6 +972,8 @@ class CIRGenFunction : public CIRGenTypeCache {
                         ArrayRef<mlir::Value *> valuesToReload = {});
   void popCleanupBlock();
 
+  void terminateStructuredRegionBody(mlir::Region &r, mlir::Location loc);
+
   /// Deactivates the given cleanup block. The block cannot be reactivated. Pops
   /// it if it's the top of the stack.
   ///

diff  --git a/clang/lib/CIR/CodeGen/CIRGenStmt.cpp b/clang/lib/CIR/CodeGen/CIRGenStmt.cpp
index a33a0b8bdd50c..07d1d62053ea6 100644
--- a/clang/lib/CIR/CodeGen/CIRGenStmt.cpp
+++ b/clang/lib/CIR/CodeGen/CIRGenStmt.cpp
@@ -488,8 +488,8 @@ mlir::LogicalResult CIRGenFunction::emitLabelStmt(const clang::LabelStmt &s) {
 }
 
 // Add a terminating yield on a body region if no other terminators are used.
-static void terminateBody(CIRGenBuilderTy &builder, mlir::Region &r,
-                          mlir::Location loc) {
+void CIRGenFunction::terminateStructuredRegionBody(mlir::Region &r,
+                                                   mlir::Location loc) {
   if (r.empty())
     return;
 
@@ -979,7 +979,7 @@ CIRGenFunction::emitCXXForRangeStmt(const CXXForRangeStmt &s,
   if (res.failed())
     return res;
 
-  terminateBody(builder, forOp.getBody(), getLoc(s.getEndLoc()));
+  terminateStructuredRegionBody(forOp.getBody(), getLoc(s.getEndLoc()));
   return mlir::success();
 }
 
@@ -1051,7 +1051,7 @@ mlir::LogicalResult CIRGenFunction::emitForStmt(const ForStmt &s) {
   if (res.failed())
     return res;
 
-  terminateBody(builder, forOp.getBody(), getLoc(s.getEndLoc()));
+  terminateStructuredRegionBody(forOp.getBody(), getLoc(s.getEndLoc()));
   return mlir::success();
 }
 
@@ -1102,7 +1102,7 @@ mlir::LogicalResult CIRGenFunction::emitDoStmt(const DoStmt &s) {
   if (res.failed())
     return res;
 
-  terminateBody(builder, doWhileOp.getBody(), getLoc(s.getEndLoc()));
+  terminateStructuredRegionBody(doWhileOp.getBody(), getLoc(s.getEndLoc()));
   return mlir::success();
 }
 
@@ -1158,7 +1158,7 @@ mlir::LogicalResult CIRGenFunction::emitWhileStmt(const WhileStmt &s) {
   if (res.failed())
     return res;
 
-  terminateBody(builder, whileOp.getBody(), getLoc(s.getEndLoc()));
+  terminateStructuredRegionBody(whileOp.getBody(), getLoc(s.getEndLoc()));
   return mlir::success();
 }
 
@@ -1253,8 +1253,8 @@ mlir::LogicalResult CIRGenFunction::emitSwitchStmt(const clang::SwitchStmt &s) {
   llvm::SmallVector<CaseOp> cases;
   swop.collectCases(cases);
   for (auto caseOp : cases)
-    terminateBody(builder, caseOp.getCaseRegion(), caseOp.getLoc());
-  terminateBody(builder, swop.getBody(), swop.getLoc());
+    terminateStructuredRegionBody(caseOp.getCaseRegion(), caseOp.getLoc());
+  terminateStructuredRegionBody(swop.getBody(), swop.getLoc());
 
   swop.setAllEnumCasesCovered(s.isAllEnumCasesCovered());
 

diff  --git a/clang/test/CIR/CodeGen/return-in-if-with-cleanups.cpp b/clang/test/CIR/CodeGen/return-in-if-with-cleanups.cpp
new file mode 100644
index 0000000000000..c6a36797004c6
--- /dev/null
+++ b/clang/test/CIR/CodeGen/return-in-if-with-cleanups.cpp
@@ -0,0 +1,23 @@
+// RUN: %clang_cc1 -triple x86_64-unknown-linux-gnu -fclangir -emit-cir %s -o %t.cir
+// RUN: FileCheck --input-file=%t.cir %s
+
+// Out-of-line member + `if` + EH cleanups + early `return` used to emit a
+// `cir.if` "then" region whose block ended on `cir.cleanup.scope` with no
+// following `cir.yield`, tripping MLIR's "block with no terminator" verifier.
+
+struct S {
+  ~S();
+};
+
+int f(bool b) {
+  if (b) {
+    S temp;
+    return 1;
+  }
+  return 0;
+}
+
+// CHECK-LABEL: cir.func{{.*}} @_Z1fb
+// CHECK: cir.if
+// CHECK: cir.cleanup.scope
+// CHECK: cir.yield