[clang-tidy][mlir] Expand to cover pointer of builder #159423

jpienaar · 2025-09-17T18:56:08Z

Previously this only checked for OpBuilder usage, but it could also be invoked via pointer. Also change how call range is calculated to avoid false overlaps which limits rewriting builder calls inside arguments of builder calls.

llvmbot · 2025-09-17T18:56:40Z

@llvm/pr-subscribers-clang-tidy

Author: Jacques Pienaar (jpienaar)

Changes

Previously this only checked for OpBuilder usage, but it could also be invoked via pointer. Also change how call range is calculated to avoid false overlaps which limits rewriting builder calls inside arguments of builder calls.

Full diff: https://github.com/llvm/llvm-project/pull/159423.diff

2 Files Affected:

(modified) clang-tools-extra/clang-tidy/llvm/UseNewMLIROpBuilderCheck.cpp (+39-32)
(modified) clang-tools-extra/test/clang-tidy/checkers/llvm/use-new-mlir-op-builder.cpp (+36-2)

diff --git a/clang-tools-extra/clang-tidy/llvm/UseNewMLIROpBuilderCheck.cpp b/clang-tools-extra/clang-tidy/llvm/UseNewMLIROpBuilderCheck.cpp
index 0d81b9a9e38ca..8705c10f5c646 100644
--- a/clang-tools-extra/clang-tidy/llvm/UseNewMLIROpBuilderCheck.cpp
+++ b/clang-tools-extra/clang-tidy/llvm/UseNewMLIROpBuilderCheck.cpp
@@ -23,13 +23,11 @@ namespace {
 using namespace ::clang::ast_matchers;
 using namespace ::clang::transformer;
 
-EditGenerator rewrite(RangeSelector Call, RangeSelector Builder,
-                      RangeSelector CallArgs) {
+EditGenerator rewrite(RangeSelector Call, RangeSelector Builder) {
   // This is using an EditGenerator rather than ASTEdit as we want to warn even
   // if in macro.
-  return [Call = std::move(Call), Builder = std::move(Builder),
-          CallArgs =
-              std::move(CallArgs)](const MatchFinder::MatchResult &Result)
+  return [Call = std::move(Call),
+          Builder = std::move(Builder)](const MatchFinder::MatchResult &Result)
              -> Expected<SmallVector<transformer::Edit, 1>> {
     Expected<CharSourceRange> CallRange = Call(Result);
     if (!CallRange)
@@ -54,7 +52,7 @@ EditGenerator rewrite(RangeSelector Call, RangeSelector Builder,
     auto NextToken = [&](std::optional<Token> CurrentToken) {
       if (!CurrentToken)
         return CurrentToken;
-      if (CurrentToken->getEndLoc() >= CallRange->getEnd())
+      if (CurrentToken->is(clang::tok::eof))
         return std::optional<Token>();
       return clang::Lexer::findNextToken(CurrentToken->getLocation(), SM,
                                          LangOpts);
@@ -68,9 +66,10 @@ EditGenerator rewrite(RangeSelector Call, RangeSelector Builder,
       return llvm::make_error<llvm::StringError>(llvm::errc::invalid_argument,
                                                  "missing '<' token");
     }
+
     std::optional<Token> EndToken = NextToken(LessToken);
-    for (std::optional<Token> GreaterToken = NextToken(EndToken);
-         GreaterToken && GreaterToken->getKind() != clang::tok::greater;
+    std::optional<Token> GreaterToken = NextToken(EndToken);
+    for (; GreaterToken && GreaterToken->getKind() != clang::tok::greater;
          GreaterToken = NextToken(GreaterToken)) {
       EndToken = GreaterToken;
     }
@@ -79,12 +78,21 @@ EditGenerator rewrite(RangeSelector Call, RangeSelector Builder,
                                                  "missing '>' token");
     }
 
+    std::optional<Token> ArgStart = NextToken(GreaterToken);
+    if (!ArgStart || ArgStart->getKind() != clang::tok::l_paren) {
+      return llvm::make_error<llvm::StringError>(llvm::errc::invalid_argument,
+                                                 "missing '(' token");
+    }
+    std::optional<Token> Arg = NextToken(ArgStart);
+    if (!Arg) {
+      return llvm::make_error<llvm::StringError>(llvm::errc::invalid_argument,
+                                                 "unexpected end of file");
+    }
+    bool hasArgs = Arg->getKind() != clang::tok::r_paren;
+
     Expected<CharSourceRange> BuilderRange = Builder(Result);
     if (!BuilderRange)
       return BuilderRange.takeError();
-    Expected<CharSourceRange> CallArgsRange = CallArgs(Result);
-    if (!CallArgsRange)
-      return CallArgsRange.takeError();
 
     // Helper for concatting below.
     auto GetText = [&](const CharSourceRange &Range) {
@@ -93,18 +101,19 @@ EditGenerator rewrite(RangeSelector Call, RangeSelector Builder,
 
     Edit Replace;
     Replace.Kind = EditKind::Range;
-    Replace.Range = *CallRange;
-    std::string CallArgsStr;
-    // Only emit args if there are any.
-    if (auto CallArgsText = GetText(*CallArgsRange).ltrim();
-        !CallArgsText.rtrim().empty()) {
-      CallArgsStr = llvm::formatv(", {}", CallArgsText);
+    Replace.Range.setBegin(CallRange->getBegin());
+    Replace.Range.setEnd(ArgStart->getEndLoc());
+    const Expr *BuilderExpr = Result.Nodes.getNodeAs<Expr>("builder");
+    std::string BuilderText = GetText(*BuilderRange).str();
+    if (BuilderExpr->getType()->isPointerType()) {
+      BuilderText = BuilderExpr->isImplicitCXXThis()
+                        ? "*this"
+                        : llvm::formatv("*{}", BuilderText).str();
     }
-    Replace.Replacement =
-        llvm::formatv("{}::create({}{})",
-                      GetText(CharSourceRange::getTokenRange(
-                          LessToken->getEndLoc(), EndToken->getLastLoc())),
-                      GetText(*BuilderRange), CallArgsStr);
+    StringRef OpType = GetText(CharSourceRange::getTokenRange(
+        LessToken->getEndLoc(), EndToken->getLastLoc()));
+    Replace.Replacement = llvm::formatv("{}::create({}{}", OpType, BuilderText,
+                                        hasArgs ? ", " : "");
 
     return SmallVector<Edit, 1>({Replace});
   };
@@ -114,19 +123,17 @@ RewriteRuleWith<std::string> useNewMlirOpBuilderCheckRule() {
   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");
+  auto BuilderType = cxxRecordDecl(isSameOrDerivedFrom("::mlir::OpBuilder"));
+  ast_matchers::internal::Matcher<Stmt> base = cxxMemberCallExpr(
+      on(expr(anyOf(hasType(BuilderType), hasType(pointsTo(BuilderType))))
+             .bind("builder")),
+      callee(expr().bind("call")),
+      callee(cxxMethodDecl(hasTemplateArgument(0, templateArgument()))),
+      callee(cxxMethodDecl(hasName("create"))));
   return applyFirst(
       //  Attempt rewrite given an lvalue builder, else just warn.
       {makeRule(cxxMemberCallExpr(unless(on(cxxTemporaryObjectExpr())), base),
-                rewrite(node("call"), node("builder"), callArgs("call")),
-                message),
+                rewrite(node("call"), node("builder")), message),
        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 0971a1611e3cb..1fbbf0a76c42a 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
@@ -2,6 +2,7 @@
 
 namespace mlir {
 class Location {};
+class Value {};
 class OpBuilder {
 public:
   template <typename OpTy, typename... Args>
@@ -28,6 +29,13 @@ struct NamedOp {
   static NamedOp create(OpBuilder &builder, Location location, const char* name) {
     return NamedOp(name);
   }
+  Value getResult() { return Value(); }
+};
+struct OperandOp {
+  OperandOp(Value val) {}
+  static OperandOp create(OpBuilder &builder, Location location, Value val) {
+    return OperandOp(val);
+  }
 };
 } // namespace mlir
 
@@ -40,6 +48,15 @@ void g(mlir::OpBuilder &b) {
   b.create<T>(b.getUnknownLoc(), "gaz");
 }
 
+class CustomBuilder : public mlir::ImplicitLocOpBuilder {
+public:
+  mlir::NamedOp f(const char *name) {
+    // CHECK-MESSAGES: :[[@LINE+2]]:12: warning: use 'OpType::create(builder, ...)'
+    // CHECK-FIXES: NamedOp::create(*this, name);
+    return create<mlir::NamedOp>(name);
+  }
+};
+
 void f() {
   mlir::OpBuilder builder;
   // CHECK-MESSAGES: :[[@LINE+2]]:3: warning: use 'OpType::create(builder, ...)' instead of 'builder.create<OpType>(...)' [llvm-use-new-mlir-op-builder]
@@ -47,6 +64,8 @@ void f() {
   builder.create<mlir::  ModuleOp>(builder.getUnknownLoc());
 
   using mlir::NamedOp;
+  using mlir::OperandOp;
+
   // CHECK-MESSAGES: :[[@LINE+2]]:3: warning: use 'OpType::create(builder, ...)' instead of 'builder.create<OpType>(...)' [llvm-use-new-mlir-op-builder]
   // CHECK-FIXES: NamedOp::create(builder, builder.getUnknownLoc(), "baz")
   builder.create<NamedOp>(builder.getUnknownLoc(), "baz");
@@ -56,7 +75,7 @@ void f() {
   // CHECK-FIXES:   builder.getUnknownLoc(),
   // CHECK-FIXES:   "caz")
   builder.
-   create<NamedOp>(
+   create<NamedOp>  (
      builder.getUnknownLoc(),
      "caz");
 
@@ -67,10 +86,25 @@ void f() {
 
   mlir::ImplicitLocOpBuilder ib;
   // 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)
+  // CHECK-FIXES: mlir::ModuleOp::create(ib   )
   ib.create<mlir::ModuleOp>(   );
 
   // 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());
+
+  auto *p = &builder;
+  // CHECK-MESSAGES: :[[@LINE+2]]:3: warning: use 'OpType::create(builder, ...)'
+  // CHECK-FIXES: NamedOp::create(*p, builder.getUnknownLoc(), "eaz")
+  p->create<NamedOp>(builder.getUnknownLoc(), "eaz");
+
+  CustomBuilder cb;
+  cb.f("faz");
+
+  // CHECK-MESSAGES: :[[@LINE+4]]:3: warning: use 'OpType::create(builder, ...)' instead of 'builder.create<OpType>(...)' [llvm-use-new-mlir-op-builder]
+  // CHECK-FIXES: OperandOp::create(builder, builder.getUnknownLoc(),
+  // CHECK-MESSAGES: :[[@LINE+3]]:5: warning: use 'OpType::create(builder, ...)' instead of 'builder.create<OpType>(...)' [llvm-use-new-mlir-op-builder]
+  // CHECK-FIXES: NamedOp::create(builder,
+  builder.create<OperandOp>(builder.getUnknownLoc(),
+    builder.create<NamedOp>(builder.getUnknownLoc(), "gaz").getResult());
 }

llvmbot · 2025-09-17T18:56:41Z

@llvm/pr-subscribers-clang-tools-extra

Author: Jacques Pienaar (jpienaar)

Changes

Previously this only checked for OpBuilder usage, but it could also be invoked via pointer. Also change how call range is calculated to avoid false overlaps which limits rewriting builder calls inside arguments of builder calls.

Full diff: https://github.com/llvm/llvm-project/pull/159423.diff

2 Files Affected:

(modified) clang-tools-extra/clang-tidy/llvm/UseNewMLIROpBuilderCheck.cpp (+39-32)
(modified) clang-tools-extra/test/clang-tidy/checkers/llvm/use-new-mlir-op-builder.cpp (+36-2)

diff --git a/clang-tools-extra/clang-tidy/llvm/UseNewMLIROpBuilderCheck.cpp b/clang-tools-extra/clang-tidy/llvm/UseNewMLIROpBuilderCheck.cpp
index 0d81b9a9e38ca..8705c10f5c646 100644
--- a/clang-tools-extra/clang-tidy/llvm/UseNewMLIROpBuilderCheck.cpp
+++ b/clang-tools-extra/clang-tidy/llvm/UseNewMLIROpBuilderCheck.cpp
@@ -23,13 +23,11 @@ namespace {
 using namespace ::clang::ast_matchers;
 using namespace ::clang::transformer;
 
-EditGenerator rewrite(RangeSelector Call, RangeSelector Builder,
-                      RangeSelector CallArgs) {
+EditGenerator rewrite(RangeSelector Call, RangeSelector Builder) {
   // This is using an EditGenerator rather than ASTEdit as we want to warn even
   // if in macro.
-  return [Call = std::move(Call), Builder = std::move(Builder),
-          CallArgs =
-              std::move(CallArgs)](const MatchFinder::MatchResult &Result)
+  return [Call = std::move(Call),
+          Builder = std::move(Builder)](const MatchFinder::MatchResult &Result)
              -> Expected<SmallVector<transformer::Edit, 1>> {
     Expected<CharSourceRange> CallRange = Call(Result);
     if (!CallRange)
@@ -54,7 +52,7 @@ EditGenerator rewrite(RangeSelector Call, RangeSelector Builder,
     auto NextToken = [&](std::optional<Token> CurrentToken) {
       if (!CurrentToken)
         return CurrentToken;
-      if (CurrentToken->getEndLoc() >= CallRange->getEnd())
+      if (CurrentToken->is(clang::tok::eof))
         return std::optional<Token>();
       return clang::Lexer::findNextToken(CurrentToken->getLocation(), SM,
                                          LangOpts);
@@ -68,9 +66,10 @@ EditGenerator rewrite(RangeSelector Call, RangeSelector Builder,
       return llvm::make_error<llvm::StringError>(llvm::errc::invalid_argument,
                                                  "missing '<' token");
     }
+
     std::optional<Token> EndToken = NextToken(LessToken);
-    for (std::optional<Token> GreaterToken = NextToken(EndToken);
-         GreaterToken && GreaterToken->getKind() != clang::tok::greater;
+    std::optional<Token> GreaterToken = NextToken(EndToken);
+    for (; GreaterToken && GreaterToken->getKind() != clang::tok::greater;
          GreaterToken = NextToken(GreaterToken)) {
       EndToken = GreaterToken;
     }
@@ -79,12 +78,21 @@ EditGenerator rewrite(RangeSelector Call, RangeSelector Builder,
                                                  "missing '>' token");
     }
 
+    std::optional<Token> ArgStart = NextToken(GreaterToken);
+    if (!ArgStart || ArgStart->getKind() != clang::tok::l_paren) {
+      return llvm::make_error<llvm::StringError>(llvm::errc::invalid_argument,
+                                                 "missing '(' token");
+    }
+    std::optional<Token> Arg = NextToken(ArgStart);
+    if (!Arg) {
+      return llvm::make_error<llvm::StringError>(llvm::errc::invalid_argument,
+                                                 "unexpected end of file");
+    }
+    bool hasArgs = Arg->getKind() != clang::tok::r_paren;
+
     Expected<CharSourceRange> BuilderRange = Builder(Result);
     if (!BuilderRange)
       return BuilderRange.takeError();
-    Expected<CharSourceRange> CallArgsRange = CallArgs(Result);
-    if (!CallArgsRange)
-      return CallArgsRange.takeError();
 
     // Helper for concatting below.
     auto GetText = [&](const CharSourceRange &Range) {
@@ -93,18 +101,19 @@ EditGenerator rewrite(RangeSelector Call, RangeSelector Builder,
 
     Edit Replace;
     Replace.Kind = EditKind::Range;
-    Replace.Range = *CallRange;
-    std::string CallArgsStr;
-    // Only emit args if there are any.
-    if (auto CallArgsText = GetText(*CallArgsRange).ltrim();
-        !CallArgsText.rtrim().empty()) {
-      CallArgsStr = llvm::formatv(", {}", CallArgsText);
+    Replace.Range.setBegin(CallRange->getBegin());
+    Replace.Range.setEnd(ArgStart->getEndLoc());
+    const Expr *BuilderExpr = Result.Nodes.getNodeAs<Expr>("builder");
+    std::string BuilderText = GetText(*BuilderRange).str();
+    if (BuilderExpr->getType()->isPointerType()) {
+      BuilderText = BuilderExpr->isImplicitCXXThis()
+                        ? "*this"
+                        : llvm::formatv("*{}", BuilderText).str();
     }
-    Replace.Replacement =
-        llvm::formatv("{}::create({}{})",
-                      GetText(CharSourceRange::getTokenRange(
-                          LessToken->getEndLoc(), EndToken->getLastLoc())),
-                      GetText(*BuilderRange), CallArgsStr);
+    StringRef OpType = GetText(CharSourceRange::getTokenRange(
+        LessToken->getEndLoc(), EndToken->getLastLoc()));
+    Replace.Replacement = llvm::formatv("{}::create({}{}", OpType, BuilderText,
+                                        hasArgs ? ", " : "");
 
     return SmallVector<Edit, 1>({Replace});
   };
@@ -114,19 +123,17 @@ RewriteRuleWith<std::string> useNewMlirOpBuilderCheckRule() {
   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");
+  auto BuilderType = cxxRecordDecl(isSameOrDerivedFrom("::mlir::OpBuilder"));
+  ast_matchers::internal::Matcher<Stmt> base = cxxMemberCallExpr(
+      on(expr(anyOf(hasType(BuilderType), hasType(pointsTo(BuilderType))))
+             .bind("builder")),
+      callee(expr().bind("call")),
+      callee(cxxMethodDecl(hasTemplateArgument(0, templateArgument()))),
+      callee(cxxMethodDecl(hasName("create"))));
   return applyFirst(
       //  Attempt rewrite given an lvalue builder, else just warn.
       {makeRule(cxxMemberCallExpr(unless(on(cxxTemporaryObjectExpr())), base),
-                rewrite(node("call"), node("builder"), callArgs("call")),
-                message),
+                rewrite(node("call"), node("builder")), message),
        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 0971a1611e3cb..1fbbf0a76c42a 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
@@ -2,6 +2,7 @@
 
 namespace mlir {
 class Location {};
+class Value {};
 class OpBuilder {
 public:
   template <typename OpTy, typename... Args>
@@ -28,6 +29,13 @@ struct NamedOp {
   static NamedOp create(OpBuilder &builder, Location location, const char* name) {
     return NamedOp(name);
   }
+  Value getResult() { return Value(); }
+};
+struct OperandOp {
+  OperandOp(Value val) {}
+  static OperandOp create(OpBuilder &builder, Location location, Value val) {
+    return OperandOp(val);
+  }
 };
 } // namespace mlir
 
@@ -40,6 +48,15 @@ void g(mlir::OpBuilder &b) {
   b.create<T>(b.getUnknownLoc(), "gaz");
 }
 
+class CustomBuilder : public mlir::ImplicitLocOpBuilder {
+public:
+  mlir::NamedOp f(const char *name) {
+    // CHECK-MESSAGES: :[[@LINE+2]]:12: warning: use 'OpType::create(builder, ...)'
+    // CHECK-FIXES: NamedOp::create(*this, name);
+    return create<mlir::NamedOp>(name);
+  }
+};
+
 void f() {
   mlir::OpBuilder builder;
   // CHECK-MESSAGES: :[[@LINE+2]]:3: warning: use 'OpType::create(builder, ...)' instead of 'builder.create<OpType>(...)' [llvm-use-new-mlir-op-builder]
@@ -47,6 +64,8 @@ void f() {
   builder.create<mlir::  ModuleOp>(builder.getUnknownLoc());
 
   using mlir::NamedOp;
+  using mlir::OperandOp;
+
   // CHECK-MESSAGES: :[[@LINE+2]]:3: warning: use 'OpType::create(builder, ...)' instead of 'builder.create<OpType>(...)' [llvm-use-new-mlir-op-builder]
   // CHECK-FIXES: NamedOp::create(builder, builder.getUnknownLoc(), "baz")
   builder.create<NamedOp>(builder.getUnknownLoc(), "baz");
@@ -56,7 +75,7 @@ void f() {
   // CHECK-FIXES:   builder.getUnknownLoc(),
   // CHECK-FIXES:   "caz")
   builder.
-   create<NamedOp>(
+   create<NamedOp>  (
      builder.getUnknownLoc(),
      "caz");
 
@@ -67,10 +86,25 @@ void f() {
 
   mlir::ImplicitLocOpBuilder ib;
   // 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)
+  // CHECK-FIXES: mlir::ModuleOp::create(ib   )
   ib.create<mlir::ModuleOp>(   );
 
   // 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());
+
+  auto *p = &builder;
+  // CHECK-MESSAGES: :[[@LINE+2]]:3: warning: use 'OpType::create(builder, ...)'
+  // CHECK-FIXES: NamedOp::create(*p, builder.getUnknownLoc(), "eaz")
+  p->create<NamedOp>(builder.getUnknownLoc(), "eaz");
+
+  CustomBuilder cb;
+  cb.f("faz");
+
+  // CHECK-MESSAGES: :[[@LINE+4]]:3: warning: use 'OpType::create(builder, ...)' instead of 'builder.create<OpType>(...)' [llvm-use-new-mlir-op-builder]
+  // CHECK-FIXES: OperandOp::create(builder, builder.getUnknownLoc(),
+  // CHECK-MESSAGES: :[[@LINE+3]]:5: warning: use 'OpType::create(builder, ...)' instead of 'builder.create<OpType>(...)' [llvm-use-new-mlir-op-builder]
+  // CHECK-FIXES: NamedOp::create(builder,
+  builder.create<OperandOp>(builder.getUnknownLoc(),
+    builder.create<NamedOp>(builder.getUnknownLoc(), "gaz").getResult());
 }

clang-tools-extra/clang-tidy/llvm/UseNewMLIROpBuilderCheck.cpp

Co-authored-by: EugeneZelenko <[email protected]>

clang-tools-extra/clang-tidy/llvm/UseNewMLIROpBuilderCheck.cpp

clang-tools-extra/test/clang-tidy/checkers/llvm/use-new-mlir-op-builder.cpp

…p-builder.cpp Co-authored-by: Victor Chernyakin <[email protected]>

github-actions · 2025-09-23T11:54:43Z

✅ With the latest revision this PR passed the C/C++ code linter.

jpienaar · 2025-10-24T14:11:41Z

I don't know clang-tool processes well, I'm assuming you want a review from one of the regular contributors. But let me know if I should add regular LLVM project reviewer first.

vbvictor

Apart from some comments in tests, LGTM

vbvictor · 2025-11-01T22:43:56Z

clang-tools-extra/test/clang-tidy/checkers/llvm/use-new-mlir-op-builder.cpp

+    // CHECK-MESSAGES: :[[@LINE+2]]:12: warning: use 'OpType::create(builder, ...)'
+    // CHECK-FIXES: return mlir::NamedOp::create(*this, name);
+    return create<mlir::NamedOp>(name);


Can we add test with

using mlir; create<NamedOp>(name);

vbvictor · 2025-11-01T22:45:51Z

clang-tools-extra/test/clang-tidy/checkers/llvm/use-new-mlir-op-builder.cpp

+  // CHECK-FIXES: OperandOp::create(builder, builder.getUnknownLoc(),
+  // CHECK-MESSAGES: :[[@LINE+3]]:5: warning: use 'OpType::create(builder, ...)' instead of 'builder.create<OpType>(...)' [llvm-use-new-mlir-op-builder]
+  // CHECK-FIXES: NamedOp::create(builder, builder.getUnknownLoc(), "gaz").getResult());


Instead of 2 separate check-fixes, can we use check-fixes-next to match expression as a whole? Or we intentionally keep separated

Good question, I had kept them separate as its two different reports (so a report and its fixed together rather than separate per type). No strong feelings about this, I can make it match whole.

Webedit

Forgot to rename check ...

Previously this only checked for OpBuilder usage, but it could also be invoked via pointer. Also change how call range is calculated to avoid false overlaps which limits rewriting builder calls inside arguments of builder calls. --------- Co-authored-by: EugeneZelenko <[email protected]> Co-authored-by: Victor Chernyakin <[email protected]>

[clang-tidy][mlir] Expand to cover pointer of builder

f23c42b

Previously this only checked for OpBuilder usage, but it could also be invoked via pointer. Also change how call range is calculated to avoid false overlaps which limits rewriting builder calls inside arguments of builder calls.

llvmbot added clang-tools-extra clang-tidy labels Sep 17, 2025

Merge branch 'main' into buildertidy

3fa56c1

jpienaar requested review from vbvictor and ymand September 17, 2025 19:02

EugeneZelenko reviewed Sep 18, 2025

View reviewed changes

clang-tools-extra/clang-tidy/llvm/UseNewMLIROpBuilderCheck.cpp Outdated Show resolved Hide resolved

clang-tools-extra/clang-tidy/llvm/UseNewMLIROpBuilderCheck.cpp Outdated Show resolved Hide resolved

EugeneZelenko requested review from HerrCai0907 and localspook September 18, 2025 14:21

Apply suggestions from code review

b2fb3e8

Co-authored-by: EugeneZelenko <[email protected]>

localspook reviewed Sep 19, 2025

View reviewed changes

clang-tools-extra/clang-tidy/llvm/UseNewMLIROpBuilderCheck.cpp Outdated Show resolved Hide resolved

5chmidti reviewed Sep 19, 2025

View reviewed changes

clang-tools-extra/clang-tidy/llvm/UseNewMLIROpBuilderCheck.cpp Outdated Show resolved Hide resolved

clang-tools-extra/test/clang-tidy/checkers/llvm/use-new-mlir-op-builder.cpp Outdated Show resolved Hide resolved

Update clang-tools-extra/test/clang-tidy/checkers/llvm/use-new-mlir-o…

540e4af

…p-builder.cpp Co-authored-by: Victor Chernyakin <[email protected]>

jpienaar added 4 commits September 23, 2025 14:03

Merge two cxxMethodDecl checks

87fc9c0

Fix clang-tidy flags

0ce25d1

Change bind to allow one callee descend.

01d0e6f

Fix typo

baf6916

jpienaar requested a review from localspook September 30, 2025 05:45

jpienaar requested a review from EugeneZelenko October 8, 2025 06:07

jpienaar and others added 5 commits October 16, 2025 16:26

Merge branch 'main' into buildertidy

803856d

Address change in check matching

195d5e0

Merge branch 'main' into buildertidy

3945776

Fix bad merge via web diff

d3f671c

Switch out of anonymous to static functions

bfffb49

vbvictor approved these changes Nov 1, 2025

View reviewed changes

Update test to address comments

b8788c4

Webedit

jpienaar and others added 2 commits November 2, 2025 19:14

Fix typo

951f4c9

Forgot to rename check ...

Merge branch 'main' into buildertidy

c162ecc

jpienaar enabled auto-merge (squash) November 2, 2025 17:15

jpienaar merged commit 44be079 into llvm:main Nov 2, 2025
10 of 11 checks passed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

[clang-tidy][mlir] Expand to cover pointer of builder #159423

[clang-tidy][mlir] Expand to cover pointer of builder #159423

Uh oh!

jpienaar commented Sep 17, 2025

Uh oh!

llvmbot commented Sep 17, 2025

Uh oh!

llvmbot commented Sep 17, 2025

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

github-actions bot commented Sep 23, 2025 •

edited

Loading

Uh oh!

jpienaar commented Oct 24, 2025

Uh oh!

vbvictor left a comment

Uh oh!

vbvictor Nov 1, 2025

Uh oh!

vbvictor Nov 1, 2025

Uh oh!

jpienaar Nov 2, 2025

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

6 participants

[clang-tidy][mlir] Expand to cover pointer of builder #159423

[clang-tidy][mlir] Expand to cover pointer of builder #159423

Uh oh!

Conversation

jpienaar commented Sep 17, 2025

Uh oh!

llvmbot commented Sep 17, 2025

Uh oh!

llvmbot commented Sep 17, 2025

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

github-actions bot commented Sep 23, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

jpienaar commented Oct 24, 2025

Uh oh!

vbvictor left a comment

Choose a reason for hiding this comment

Uh oh!

vbvictor Nov 1, 2025

Choose a reason for hiding this comment

Uh oh!

vbvictor Nov 1, 2025

Choose a reason for hiding this comment

Uh oh!

jpienaar Nov 2, 2025

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

6 participants

github-actions bot commented Sep 23, 2025 •

edited

Loading