[clang-tools-extra] [clang-tidy] Speed up/rewrite `llvm-use-new-mlir-op-builder` (PR #196596)
Victor Chernyakin via cfe-commits
cfe-commits at lists.llvm.org
Fri May 8 10:52:32 PDT 2026
https://github.com/localspook created https://github.com/llvm/llvm-project/pull/196596
None
>From 509f27d61887e6df161838b3fa5eb94f2de7c749 Mon Sep 17 00:00:00 2001
From: Victor Chernyakin <chernyakin.victor.j at outlook.com>
Date: Fri, 8 May 2026 10:50:43 -0700
Subject: [PATCH] [clang-tidy] Speed up/rewrite `llvm-use-new-mlir-op-builder`
---
.../llvm/UseNewMLIROpBuilderCheck.cpp | 169 ++++++------------
.../llvm/UseNewMLIROpBuilderCheck.h | 8 +-
.../checkers/llvm/use-new-mlir-op-builder.cpp | 6 +-
3 files changed, 59 insertions(+), 124 deletions(-)
diff --git a/clang-tools-extra/clang-tidy/llvm/UseNewMLIROpBuilderCheck.cpp b/clang-tools-extra/clang-tidy/llvm/UseNewMLIROpBuilderCheck.cpp
index ca3e778e573c1..66371f7b2a58f 100644
--- a/clang-tools-extra/clang-tidy/llvm/UseNewMLIROpBuilderCheck.cpp
+++ b/clang-tools-extra/clang-tidy/llvm/UseNewMLIROpBuilderCheck.cpp
@@ -7,137 +7,70 @@
//===----------------------------------------------------------------------===//
#include "UseNewMLIROpBuilderCheck.h"
-#include "../utils/LexerUtils.h"
#include "clang/ASTMatchers/ASTMatchers.h"
-#include "clang/Basic/LLVM.h"
-#include "clang/Tooling/Transformer/RangeSelector.h"
-#include "clang/Tooling/Transformer/RewriteRule.h"
-#include "clang/Tooling/Transformer/Stencil.h"
-#include "llvm/Support/Error.h"
-#include "llvm/Support/FormatVariadic.h"
+#include "clang/Lex/Lexer.h"
-namespace clang::tidy::llvm_check {
-
-using namespace ::clang::ast_matchers;
-using namespace ::clang::transformer;
+using namespace clang::ast_matchers;
-static 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)](const MatchFinder::MatchResult &Result)
- -> Expected<SmallVector<transformer::Edit, 1>> {
- Expected<CharSourceRange> CallRange = Call(Result);
- if (!CallRange)
- return CallRange.takeError();
- SourceManager &SM = *Result.SourceManager;
- const LangOptions &LangOpts = Result.Context->getLangOpts();
- SourceLocation Begin = CallRange->getBegin();
+namespace clang::tidy::llvm_check {
- // This will result in just a warning and no edit.
- const bool InMacro = CallRange->getBegin().isMacroID();
- if (InMacro) {
- while (SM.isMacroArgExpansion(Begin))
- Begin = SM.getImmediateExpansionRange(Begin).getBegin();
- Edit WarnOnly;
- WarnOnly.Kind = EditKind::Range;
- WarnOnly.Range = CharSourceRange::getCharRange(Begin, Begin);
- return SmallVector<Edit, 1>({WarnOnly});
- }
+void UseNewMlirOpBuilderCheck::registerMatchers(MatchFinder *Finder) {
+ // Match a create call on an OpBuilder.
+ const auto BuilderType =
+ cxxRecordDecl(isSameOrDerivedFrom("::mlir::OpBuilder"));
+ Finder->addMatcher(
+ cxxMemberCallExpr(
+ callee(cxxMethodDecl(hasTemplateArgument(0, templateArgument()),
+ hasName("create"))),
+ on(expr(anyOf(hasType(BuilderType), hasType(pointsTo(BuilderType))))
+ .bind("builder")))
+ .bind("call"),
+ this);
+}
- // This will try to extract the template argument as written so that the
- // rewritten code looks closest to original.
- auto NextToken = [&](std::optional<Token> CurrentToken) {
- if (!CurrentToken)
- return CurrentToken;
- if (CurrentToken->is(tok::eof))
- return std::optional<Token>();
- return utils::lexer::findNextTokenSkippingComments(
- CurrentToken->getLocation(), SM, LangOpts);
- };
- std::optional<Token> LessToken =
- utils::lexer::findNextTokenSkippingComments(Begin, SM, LangOpts);
- while (LessToken && LessToken->getKind() != tok::less)
- LessToken = NextToken(LessToken);
- if (!LessToken) {
- return llvm::make_error<llvm::StringError>(llvm::errc::invalid_argument,
- "missing '<' token");
- }
+void UseNewMlirOpBuilderCheck::check(const MatchFinder::MatchResult &Result) {
+ const auto *Call = Result.Nodes.getNodeAs<CXXMemberCallExpr>("call");
+ const auto *Builder = Result.Nodes.getNodeAs<Expr>("builder");
- std::optional<Token> EndToken = NextToken(LessToken);
- std::optional<Token> GreaterToken = NextToken(EndToken);
- for (; GreaterToken && GreaterToken->getKind() != tok::greater;
- GreaterToken = NextToken(GreaterToken)) {
- EndToken = GreaterToken;
- }
- if (!EndToken) {
- return llvm::make_error<llvm::StringError>(llvm::errc::invalid_argument,
- "missing '>' token");
- }
+ const DiagnosticBuilder Diag =
+ diag(Result.SourceManager->getExpansionLoc(Call->getBeginLoc()),
+ "use 'OpType::create(builder, ...)' instead of "
+ "'builder.create<OpType>(...)'");
- std::optional<Token> ArgStart = NextToken(GreaterToken);
- if (!ArgStart || ArgStart->getKind() != 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");
- }
- const bool HasArgs = Arg->getKind() != tok::r_paren;
+ if (Call->getBeginLoc().isMacroID())
+ return;
- Expected<CharSourceRange> BuilderRange = Builder(Result);
- if (!BuilderRange)
- return BuilderRange.takeError();
+ // Only attempt the rewrite if given an lvalue builder.
+ if (isa<CXXTemporaryObjectExpr>(Builder))
+ return;
- // Helper for concatting below.
- auto GetText = [&](const CharSourceRange &Range) {
- return Lexer::getSourceText(Range, SM, LangOpts);
- };
+ const auto *Callee = dyn_cast<MemberExpr>(Call->getCallee());
+ if (!Callee)
+ return;
- Edit Replace;
- Replace.Kind = EditKind::Range;
- 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();
- }
- const StringRef OpType = GetText(CharSourceRange::getTokenRange(
- LessToken->getEndLoc(), EndToken->getLastLoc()));
- Replace.Replacement = llvm::formatv("{}::create({}{}", OpType, BuilderText,
- HasArgs ? ", " : "");
+ Diag << FixItHint::CreateRemoval(
+ {Call->getBeginLoc(), Callee->getLAngleLoc()})
+ << FixItHint::CreateReplacement(Callee->getRAngleLoc(), "::create");
- return SmallVector<Edit, 1>({Replace});
- };
-}
+ std::string BuilderArg;
+ if (Builder->isImplicitCXXThis()) {
+ BuilderArg = "*this";
+ } else {
+ if (Callee->isArrow())
+ BuilderArg += '*';
-static RewriteRuleWith<std::string> useNewMlirOpBuilderCheckRule() {
- const Stencil Message = cat("use 'OpType::create(builder, ...)' instead of "
- "'builder.create<OpType>(...)'");
- // Match a create call on an OpBuilder.
- auto BuilderType = cxxRecordDecl(isSameOrDerivedFrom("::mlir::OpBuilder"));
- const ast_matchers::internal::Matcher<Stmt> Base =
- cxxMemberCallExpr(
- on(expr(anyOf(hasType(BuilderType), hasType(pointsTo(BuilderType))))
- .bind("builder")),
- callee(cxxMethodDecl(hasTemplateArgument(0, templateArgument()),
- hasName("create"))))
- .bind("call");
- return applyFirst(
- // Attempt rewrite given an lvalue builder, else just warn.
- {makeRule(cxxMemberCallExpr(unless(on(cxxTemporaryObjectExpr())), Base),
- rewrite(node("call"), node("builder")), Message),
- makeRule(Base, noopEdit(node("call")), Message)});
-}
+ BuilderArg += Lexer::getSourceText(
+ CharSourceRange::getTokenRange(Builder->getSourceRange()),
+ *Result.SourceManager, getLangOpts());
+ }
-UseNewMlirOpBuilderCheck::UseNewMlirOpBuilderCheck(StringRef Name,
- ClangTidyContext *Context)
- : TransformerClangTidyCheck(useNewMlirOpBuilderCheckRule(), Name, Context) {
+ if (Call->getNumArgs() == 0) {
+ Diag << FixItHint::CreateInsertion(Call->getRParenLoc(), BuilderArg);
+ } else {
+ BuilderArg += ", ";
+ Diag << FixItHint::CreateInsertion(Call->getArg(0)->getBeginLoc(),
+ BuilderArg);
+ }
}
} // namespace clang::tidy::llvm_check
diff --git a/clang-tools-extra/clang-tidy/llvm/UseNewMLIROpBuilderCheck.h b/clang-tools-extra/clang-tidy/llvm/UseNewMLIROpBuilderCheck.h
index 28c7477a18f7f..c24a6f8cdf5c8 100644
--- a/clang-tools-extra/clang-tidy/llvm/UseNewMLIROpBuilderCheck.h
+++ b/clang-tools-extra/clang-tidy/llvm/UseNewMLIROpBuilderCheck.h
@@ -9,15 +9,17 @@
#ifndef LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_LLVM_USENEWMLIROPBUILDERCHECK_H
#define LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_LLVM_USENEWMLIROPBUILDERCHECK_H
-#include "../utils/TransformerClangTidyCheck.h"
+#include "../ClangTidyCheck.h"
namespace clang::tidy::llvm_check {
/// Checks for uses of MLIR's old/to be deprecated `OpBuilder::create<T>` form
/// and suggests using `T::create` instead.
-class UseNewMlirOpBuilderCheck : public utils::TransformerClangTidyCheck {
+class UseNewMlirOpBuilderCheck : public ClangTidyCheck {
public:
- UseNewMlirOpBuilderCheck(StringRef Name, ClangTidyContext *Context);
+ using ClangTidyCheck::ClangTidyCheck;
+ void registerMatchers(ast_matchers::MatchFinder *Finder) override;
+ void check(const ast_matchers::MatchFinder::MatchResult &Result) override;
bool isLanguageVersionSupported(const LangOptions &LangOpts) const override {
return LangOpts.CPlusPlus;
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 c4a1d8d66cdeb..9b20ccf98d1e9 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
@@ -78,8 +78,8 @@ void f() {
builder.create<NamedOp>(builder.getUnknownLoc(), "baz");
// CHECK-MESSAGES: :[[@LINE+4]]:3: warning: use 'OpType::create(builder, ...)' instead of 'builder.create<OpType>(...)' [llvm-use-new-mlir-op-builder]
- // CHECK-FIXES: NamedOp::create(builder,
- // CHECK-FIXES: builder.getUnknownLoc(),
+ // CHECK-FIXES: NamedOp::create (
+ // CHECK-FIXES: builder, builder.getUnknownLoc(),
// CHECK-FIXES: "caz");
builder.
create<NamedOp> (
@@ -93,7 +93,7 @@ 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]
More information about the cfe-commits
mailing list