[clang-tools-extra] [clang-tidy][mlir] Make rewrite more conservative. (PR #150757)

[Jacques Pienaar via cfe-commits]

https://github.com/jpienaar updated https://github.com/llvm/llvm-project/pull/150757

>From 8d1b6ac9820dc17b365546883d18bf852300048e Mon Sep 17 00:00:00 2001
From: Jacques Pienaar <jacques+gh at japienaar.info>
Date: Sat, 26 Jul 2025 12:24:14 +0000
Subject: [PATCH 1/3] [clang-tidy][mlir] Make rewrite more conservative.

Don't create a fix where object invoked on is a temporary object as
create method requires a reference.
---
 .../llvm/UseNewMLIROpBuilderCheck.cpp           | 17 ++++++++++++-----
 .../checkers/llvm/use-new-mlir-op-builder.cpp   |  3 +++
 2 files changed, 15 insertions(+), 5 deletions(-)

diff --git a/clang-tools-extra/clang-tidy/llvm/UseNewMLIROpBuilderCheck.cpp b/clang-tools-extra/clang-tidy/llvm/UseNewMLIROpBuilderCheck.cpp
index 0b28ea2508977..131c5c32eb780 100644
--- a/clang-tools-extra/clang-tidy/llvm/UseNewMLIROpBuilderCheck.cpp
+++ b/clang-tools-extra/clang-tidy/llvm/UseNewMLIROpBuilderCheck.cpp
@@ -111,17 +111,24 @@ EditGenerator rewrite(RangeSelector Call, RangeSelector Builder,
 }
 
 RewriteRuleWith<std::string> useNewMlirOpBuilderCheckRule() {
-  return makeRule(
+  Stencil message = cat("use 'OpType::create(builder, ...)' instead of "
+                        "'builder.create<OpType>(...)'");
+  // Match a create call on an OpBuilder.
+  ast_matchers::internal::Matcher<Stmt> base =
       cxxMemberCallExpr(
           on(expr(hasType(
                       cxxRecordDecl(isSameOrDerivedFrom("::mlir::OpBuilder"))))
                  .bind("builder")),
           callee(cxxMethodDecl(hasTemplateArgument(0, templateArgument()))),
           callee(cxxMethodDecl(hasName("create"))))
-          .bind("call"),
-      rewrite(node("call"), node("builder"), callArgs("call")),
-      cat("use 'OpType::create(builder, ...)' instead of "
-          "'builder.create<OpType>(...)'"));
+          .bind("call");
+  return applyFirst(
+      {// Attempt to rewrite with a concrete builder.
+       makeRule(cxxMemberCallExpr(unless(on(cxxTemporaryObjectExpr())), base),
+                rewrite(node("call"), node("builder"), callArgs("call")),
+                message),
+       // Warn on calls on temporary objects only.
+       makeRule(base, noopEdit(node("call")), message)});
 }
 } // namespace
 
diff --git a/clang-tools-extra/test/clang-tidy/checkers/llvm/use-new-mlir-op-builder.cpp b/clang-tools-extra/test/clang-tidy/checkers/llvm/use-new-mlir-op-builder.cpp
index 57e026c10bf53..3b8d5da968f7f 100644
--- a/clang-tools-extra/test/clang-tidy/checkers/llvm/use-new-mlir-op-builder.cpp
+++ b/clang-tools-extra/test/clang-tidy/checkers/llvm/use-new-mlir-op-builder.cpp
@@ -69,4 +69,7 @@ void f() {
   // CHECK-MESSAGES: :[[@LINE+2]]:3: warning: use 'OpType::create(builder, ...)' instead of 'builder.create<OpType>(...)' [llvm-use-new-mlir-op-builder]
   // CHECK-FIXES: mlir::ModuleOp::create(ib)
   ib.create<mlir::ModuleOp>(   );
+
+  // CHECK-MESSAGES: :[[@LINE+1]]:3: warning: use 'OpType::create(builder, ...)' instead of 'builder.create<OpType>(...)' [llvm-use-new-mlir-op-builder]
+  mlir::OpBuilder().create<mlir::ModuleOp>(builder.getUnknownLoc());
 }

>From 3789784d5829debff67198a3e6ad49af1a4180a1 Mon Sep 17 00:00:00 2001
From: Jacques Pienaar <jacques+gh at japienaar.info>
Date: Sat, 26 Jul 2025 13:31:27 +0000
Subject: [PATCH 2/3] Update comment

---
 .../clang-tidy/llvm/UseNewMLIROpBuilderCheck.cpp             | 5 ++---
 1 file changed, 2 insertions(+), 3 deletions(-)

diff --git a/clang-tools-extra/clang-tidy/llvm/UseNewMLIROpBuilderCheck.cpp b/clang-tools-extra/clang-tidy/llvm/UseNewMLIROpBuilderCheck.cpp
index 131c5c32eb780..4722199364cb5 100644
--- a/clang-tools-extra/clang-tidy/llvm/UseNewMLIROpBuilderCheck.cpp
+++ b/clang-tools-extra/clang-tidy/llvm/UseNewMLIROpBuilderCheck.cpp
@@ -123,11 +123,10 @@ RewriteRuleWith<std::string> useNewMlirOpBuilderCheckRule() {
           callee(cxxMethodDecl(hasName("create"))))
           .bind("call");
   return applyFirst(
-      {// Attempt to rewrite with a concrete builder.
-       makeRule(cxxMemberCallExpr(unless(on(cxxTemporaryObjectExpr())), base),
+      //  Attempt rewrite given an lvalue builder, else just warn.
+      {makeRule(cxxMemberCallExpr(unless(on(cxxTemporaryObjectExpr())), base),
                 rewrite(node("call"), node("builder"), callArgs("call")),
                 message),
-       // Warn on calls on temporary objects only.
        makeRule(base, noopEdit(node("call")), message)});
 }
 } // namespace

>From 02328f9338d536a5eddc88e4e60bc2a2c54d92a0 Mon Sep 17 00:00:00 2001
From: Jacques Pienaar <jacques+gh at japienaar.info>
Date: Mon, 28 Jul 2025 11:02:04 -0700
Subject: [PATCH 3/3] Update use-new-mlir-op-builder.cpp

---
 .../test/clang-tidy/checkers/llvm/use-new-mlir-op-builder.cpp  | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/clang-tools-extra/test/clang-tidy/checkers/llvm/use-new-mlir-op-builder.cpp b/clang-tools-extra/test/clang-tidy/checkers/llvm/use-new-mlir-op-builder.cpp
index 3b8d5da968f7f..0971a1611e3cb 100644
--- a/clang-tools-extra/test/clang-tidy/checkers/llvm/use-new-mlir-op-builder.cpp
+++ b/clang-tools-extra/test/clang-tidy/checkers/llvm/use-new-mlir-op-builder.cpp
@@ -70,6 +70,7 @@ void f() {
   // CHECK-FIXES: mlir::ModuleOp::create(ib)
   ib.create<mlir::ModuleOp>(   );
 
-  // CHECK-MESSAGES: :[[@LINE+1]]:3: warning: use 'OpType::create(builder, ...)' instead of 'builder.create<OpType>(...)' [llvm-use-new-mlir-op-builder]
+  // CHECK-MESSAGES: :[[@LINE+2]]:3: warning: use 'OpType::create(builder, ...)' instead of 'builder.create<OpType>(...)' [llvm-use-new-mlir-op-builder]
+  // CHECK-FIXES: mlir::OpBuilder().create<mlir::ModuleOp>(builder.getUnknownLoc());
   mlir::OpBuilder().create<mlir::ModuleOp>(builder.getUnknownLoc());
 }