[clang-tools-extra] 3feb6f9 - [clang-tidy] Add MLIR check for old op builder usage. (#149148)
via cfe-commits
cfe-commits at lists.llvm.org
Thu Jul 24 06:48:08 PDT 2025
Author: Jacques Pienaar
Date: 2025-07-24T15:48:05+02:00
New Revision: 3feb6f971577701713034d3404b6737fe6462d43
URL: https://github.com/llvm/llvm-project/commit/3feb6f971577701713034d3404b6737fe6462d43
DIFF: https://github.com/llvm/llvm-project/commit/3feb6f971577701713034d3404b6737fe6462d43.diff
LOG: [clang-tidy] Add MLIR check for old op builder usage. (#149148)
Upstream is moving towards new create method invocation, add check to flag old
usage that will be deprecated.
---------
Co-authored-by: Baranov Victor <bar.victor.2002 at gmail.com>
Co-authored-by: EugeneZelenko <eugene.zelenko at gmail.com>
Added:
clang-tools-extra/clang-tidy/llvm/UseNewMLIROpBuilderCheck.cpp
clang-tools-extra/clang-tidy/llvm/UseNewMLIROpBuilderCheck.h
clang-tools-extra/docs/clang-tidy/checks/llvm/use-new-mlir-op-builder.rst
clang-tools-extra/test/clang-tidy/checkers/llvm/use-new-mlir-op-builder.cpp
Modified:
clang-tools-extra/clang-tidy/llvm/CMakeLists.txt
clang-tools-extra/clang-tidy/llvm/LLVMTidyModule.cpp
clang-tools-extra/docs/ReleaseNotes.rst
clang-tools-extra/docs/clang-tidy/checks/list.rst
Removed:
################################################################################
diff --git a/clang-tools-extra/clang-tidy/llvm/CMakeLists.txt b/clang-tools-extra/clang-tidy/llvm/CMakeLists.txt
index 3232f6e2cafe5..4f1da43d3f1b7 100644
--- a/clang-tools-extra/clang-tidy/llvm/CMakeLists.txt
+++ b/clang-tools-extra/clang-tidy/llvm/CMakeLists.txt
@@ -11,11 +11,13 @@ add_clang_library(clangTidyLLVMModule STATIC
PreferRegisterOverUnsignedCheck.cpp
PreferStaticOverAnonymousNamespaceCheck.cpp
TwineLocalCheck.cpp
+ UseNewMLIROpBuilderCheck.cpp
LINK_LIBS
clangTidy
clangTidyReadabilityModule
clangTidyUtils
+ clangTransformer
DEPENDS
omp_gen
diff --git a/clang-tools-extra/clang-tidy/llvm/LLVMTidyModule.cpp b/clang-tools-extra/clang-tidy/llvm/LLVMTidyModule.cpp
index 075453046f0a1..c7c61fd1649cc 100644
--- a/clang-tools-extra/clang-tidy/llvm/LLVMTidyModule.cpp
+++ b/clang-tools-extra/clang-tidy/llvm/LLVMTidyModule.cpp
@@ -18,6 +18,7 @@
#include "PreferRegisterOverUnsignedCheck.h"
#include "PreferStaticOverAnonymousNamespaceCheck.h"
#include "TwineLocalCheck.h"
+#include "UseNewMLIROpBuilderCheck.h"
namespace clang::tidy {
namespace llvm_check {
@@ -40,6 +41,8 @@ class LLVMModule : public ClangTidyModule {
CheckFactories.registerCheck<readability::QualifiedAutoCheck>(
"llvm-qualified-auto");
CheckFactories.registerCheck<TwineLocalCheck>("llvm-twine-local");
+ CheckFactories.registerCheck<UseNewMlirOpBuilderCheck>(
+ "llvm-use-new-mlir-op-builder");
}
ClangTidyOptions getModuleOptions() override {
diff --git a/clang-tools-extra/clang-tidy/llvm/UseNewMLIROpBuilderCheck.cpp b/clang-tools-extra/clang-tidy/llvm/UseNewMLIROpBuilderCheck.cpp
new file mode 100644
index 0000000000000..0b28ea2508977
--- /dev/null
+++ b/clang-tools-extra/clang-tidy/llvm/UseNewMLIROpBuilderCheck.cpp
@@ -0,0 +1,133 @@
+//===--- UseNewMLIROpBuilderCheck.cpp - clang-tidy ------------------------===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===----------------------------------------------------------------------===//
+
+#include "UseNewMLIROpBuilderCheck.h"
+#include "clang/ASTMatchers/ASTMatchers.h"
+#include "clang/Basic/LLVM.h"
+#include "clang/Lex/Lexer.h"
+#include "clang/Tooling/Transformer/RangeSelector.h"
+#include "clang/Tooling/Transformer/RewriteRule.h"
+#include "clang/Tooling/Transformer/SourceCode.h"
+#include "clang/Tooling/Transformer/Stencil.h"
+#include "llvm/Support/Error.h"
+#include "llvm/Support/FormatVariadic.h"
+
+namespace clang::tidy::llvm_check {
+namespace {
+
+using namespace ::clang::ast_matchers;
+using namespace ::clang::transformer;
+
+EditGenerator rewrite(RangeSelector Call, RangeSelector Builder,
+ RangeSelector CallArgs) {
+ // 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)
+ -> 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();
+
+ // This will result in just a warning and no edit.
+ 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});
+ }
+
+ // 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->getEndLoc() >= CallRange->getEnd())
+ return std::optional<Token>();
+ return clang::Lexer::findNextToken(CurrentToken->getLocation(), SM,
+ LangOpts);
+ };
+ std::optional<Token> LessToken =
+ clang::Lexer::findNextToken(Begin, SM, LangOpts);
+ while (LessToken && LessToken->getKind() != clang::tok::less) {
+ LessToken = NextToken(LessToken);
+ }
+ if (!LessToken) {
+ 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;
+ GreaterToken = NextToken(GreaterToken)) {
+ EndToken = GreaterToken;
+ }
+ if (!EndToken) {
+ return llvm::make_error<llvm::StringError>(llvm::errc::invalid_argument,
+ "missing '>' token");
+ }
+
+ 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) {
+ return clang::Lexer::getSourceText(Range, SM, LangOpts);
+ };
+
+ 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.Replacement =
+ llvm::formatv("{}::create({}{})",
+ GetText(CharSourceRange::getTokenRange(
+ LessToken->getEndLoc(), EndToken->getLastLoc())),
+ GetText(*BuilderRange), CallArgsStr);
+
+ return SmallVector<Edit, 1>({Replace});
+ };
+}
+
+RewriteRuleWith<std::string> useNewMlirOpBuilderCheckRule() {
+ return makeRule(
+ 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>(...)'"));
+}
+} // namespace
+
+UseNewMlirOpBuilderCheck::UseNewMlirOpBuilderCheck(StringRef Name,
+ ClangTidyContext *Context)
+ : TransformerClangTidyCheck(useNewMlirOpBuilderCheckRule(), Name, Context) {
+}
+
+} // 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
new file mode 100644
index 0000000000000..813a23c564782
--- /dev/null
+++ b/clang-tools-extra/clang-tidy/llvm/UseNewMLIROpBuilderCheck.h
@@ -0,0 +1,29 @@
+//===--- UseNewMLIROpBuilderCheck.h - clang-tidy ----------------*- C++ -*-===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===----------------------------------------------------------------------===//
+
+#ifndef LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_LLVM_USENEWMLIROPBUILDERCHECK_H
+#define LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_LLVM_USENEWMLIROPBUILDERCHECK_H
+
+#include "../utils/TransformerClangTidyCheck.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 {
+public:
+ UseNewMlirOpBuilderCheck(StringRef Name, ClangTidyContext *Context);
+
+ bool isLanguageVersionSupported(const LangOptions &LangOpts) const override {
+ return getLangOpts().CPlusPlus;
+ }
+};
+
+} // namespace clang::tidy::llvm_check
+
+#endif // LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_LLVM_USENEWMLIROPBUILDERCHECK_H
diff --git a/clang-tools-extra/docs/ReleaseNotes.rst b/clang-tools-extra/docs/ReleaseNotes.rst
index edab04730cb25..bde4ddec50ff3 100644
--- a/clang-tools-extra/docs/ReleaseNotes.rst
+++ b/clang-tools-extra/docs/ReleaseNotes.rst
@@ -96,6 +96,12 @@ Improvements to clang-tidy
New checks
^^^^^^^^^^
+- New :doc:`llvm-mlir-op-builder
+ <clang-tidy/checks/llvm/use-new-mlir-op-builder>` check.
+
+ Checks for uses of MLIR's old/to be deprecated ``OpBuilder::create<T>`` form
+ and suggests using ``T::create`` instead.
+
New check aliases
^^^^^^^^^^^^^^^^^
diff --git a/clang-tools-extra/docs/clang-tidy/checks/list.rst b/clang-tools-extra/docs/clang-tidy/checks/list.rst
index 0cffbd323caa2..20a43274f9788 100644
--- a/clang-tools-extra/docs/clang-tidy/checks/list.rst
+++ b/clang-tools-extra/docs/clang-tidy/checks/list.rst
@@ -247,6 +247,7 @@ Clang-Tidy Checks
:doc:`linuxkernel-must-check-errs <linuxkernel/must-check-errs>`,
:doc:`llvm-header-guard <llvm/header-guard>`,
:doc:`llvm-include-order <llvm/include-order>`, "Yes"
+ :doc:`llvm-use-new-mlir-op-builder <llvm/use-new-mlir-op-builder>`, "Yes"
:doc:`llvm-namespace-comment <llvm/namespace-comment>`,
:doc:`llvm-prefer-isa-or-dyn-cast-in-conditionals <llvm/prefer-isa-or-dyn-cast-in-conditionals>`, "Yes"
:doc:`llvm-prefer-register-over-unsigned <llvm/prefer-register-over-unsigned>`, "Yes"
diff --git a/clang-tools-extra/docs/clang-tidy/checks/llvm/use-new-mlir-op-builder.rst b/clang-tools-extra/docs/clang-tidy/checks/llvm/use-new-mlir-op-builder.rst
new file mode 100644
index 0000000000000..bb7427d4a4fbc
--- /dev/null
+++ b/clang-tools-extra/docs/clang-tidy/checks/llvm/use-new-mlir-op-builder.rst
@@ -0,0 +1,21 @@
+.. title:: clang-tidy - llvm-mlir-op-builder
+
+llvm-mlir-op-builder
+====================
+
+Checks for uses of MLIR's old/to be deprecated ``OpBuilder::create<T>`` form
+and suggests using ``T::create`` instead.
+
+Example
+-------
+
+.. code-block:: c++
+
+ builder.create<FooOp>(builder.getUnknownLoc(), "baz");
+
+
+Transforms to:
+
+.. code-block:: c++
+
+ FooOp::create(builder, builder.getUnknownLoc(), "baz");
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
new file mode 100644
index 0000000000000..57e026c10bf53
--- /dev/null
+++ b/clang-tools-extra/test/clang-tidy/checkers/llvm/use-new-mlir-op-builder.cpp
@@ -0,0 +1,72 @@
+// RUN: %check_clang_tidy --match-partial-fixes %s llvm-use-new-mlir-op-builder %t
+
+namespace mlir {
+class Location {};
+class OpBuilder {
+public:
+ template <typename OpTy, typename... Args>
+ OpTy create(Location location, Args &&...args) {
+ return OpTy(args...);
+ }
+ Location getUnknownLoc() { return Location(); }
+};
+class ImplicitLocOpBuilder : public OpBuilder {
+public:
+ template <typename OpTy, typename... Args>
+ OpTy create(Args &&...args) {
+ return OpTy(args...);
+ }
+};
+struct ModuleOp {
+ ModuleOp() {}
+ static ModuleOp create(OpBuilder &builder, Location location) {
+ return ModuleOp();
+ }
+};
+struct NamedOp {
+ NamedOp(const char* name) {}
+ static NamedOp create(OpBuilder &builder, Location location, const char* name) {
+ return NamedOp(name);
+ }
+};
+} // namespace mlir
+
+#define ASSIGN(A, B, C, D) C A = B.create<C>(B.getUnknownLoc(), D)
+
+template <typename T>
+void g(mlir::OpBuilder &b) {
+ // CHECK-MESSAGES: :[[@LINE+2]]:3: warning: use 'OpType::create(builder, ...)' instead of 'builder.create<OpType>(...)' [llvm-use-new-mlir-op-builder]
+ // CHECK-FIXES: T::create(b, b.getUnknownLoc(), "gaz")
+ b.create<T>(b.getUnknownLoc(), "gaz");
+}
+
+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]
+ // CHECK-FIXES: mlir:: ModuleOp::create(builder, builder.getUnknownLoc())
+ builder.create<mlir:: ModuleOp>(builder.getUnknownLoc());
+
+ using mlir::NamedOp;
+ // 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");
+
+ // 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: "caz")
+ builder.
+ create<NamedOp>(
+ builder.getUnknownLoc(),
+ "caz");
+
+ // CHECK-MESSAGES: :[[@LINE+1]]:3: warning: use 'OpType::create(builder, ...)' instead of 'builder.create<OpType>(...)' [llvm-use-new-mlir-op-builder]
+ ASSIGN(op, builder, NamedOp, "daz");
+
+ g<NamedOp>(builder);
+
+ 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)
+ ib.create<mlir::ModuleOp>( );
+}
More information about the cfe-commits
mailing list