[clang-tools-extra] [clang-tidy] Add MLIR check for old op builder usage. (PR #149148)
Jacques Pienaar via cfe-commits
cfe-commits at lists.llvm.org
Wed Jul 16 11:57:45 PDT 2025
https://github.com/jpienaar updated https://github.com/llvm/llvm-project/pull/149148
>From f5d80596c0dc56086b585e9d59afa7472ff16321 Mon Sep 17 00:00:00 2001
From: Jacques Pienaar <jpienaar at google.com>
Date: Wed, 16 Jul 2025 17:37:53 +0000
Subject: [PATCH] [clang-tidy] Add MLIR check for old op builder usage.
Moving towards new create method invocation, add check to flag old usage.
---
clang-tools-extra/clang-tidy/CMakeLists.txt | 2 +
.../clang-tidy/ClangTidyForceLinker.h | 5 +
.../clang-tidy/mlir/CMakeLists.txt | 28 +++++
.../clang-tidy/mlir/MLIRTidyModule.cpp | 39 +++++++
.../clang-tidy/mlir/OpBuilderCheck.cpp | 102 ++++++++++++++++++
.../clang-tidy/mlir/OpBuilderCheck.h | 21 ++++
clang-tools-extra/docs/ReleaseNotes.rst | 5 +
.../docs/clang-tidy/checks/list.rst | 2 +
.../clang-tidy/checks/mlir/op-builder.rst | 22 ++++
.../clang-tidy/checkers/mlir/op_builder.cpp | 51 +++++++++
10 files changed, 277 insertions(+)
create mode 100644 clang-tools-extra/clang-tidy/mlir/CMakeLists.txt
create mode 100644 clang-tools-extra/clang-tidy/mlir/MLIRTidyModule.cpp
create mode 100644 clang-tools-extra/clang-tidy/mlir/OpBuilderCheck.cpp
create mode 100644 clang-tools-extra/clang-tidy/mlir/OpBuilderCheck.h
create mode 100644 clang-tools-extra/docs/clang-tidy/checks/mlir/op-builder.rst
create mode 100644 clang-tools-extra/test/clang-tidy/checkers/mlir/op_builder.cpp
diff --git a/clang-tools-extra/clang-tidy/CMakeLists.txt b/clang-tools-extra/clang-tidy/CMakeLists.txt
index 93117cf1d6373..b89003bf6c926 100644
--- a/clang-tools-extra/clang-tidy/CMakeLists.txt
+++ b/clang-tools-extra/clang-tidy/CMakeLists.txt
@@ -66,6 +66,7 @@ add_subdirectory(linuxkernel)
add_subdirectory(llvm)
add_subdirectory(llvmlibc)
add_subdirectory(misc)
+add_subdirectory(mlir)
add_subdirectory(modernize)
if(CLANG_TIDY_ENABLE_STATIC_ANALYZER)
add_subdirectory(mpi)
@@ -93,6 +94,7 @@ set(ALL_CLANG_TIDY_CHECKS
clangTidyLLVMModule
clangTidyLLVMLibcModule
clangTidyMiscModule
+ clangTidyMLIRModule
clangTidyModernizeModule
clangTidyObjCModule
clangTidyOpenMPModule
diff --git a/clang-tools-extra/clang-tidy/ClangTidyForceLinker.h b/clang-tools-extra/clang-tidy/ClangTidyForceLinker.h
index adde9136ff1dd..3cde93124c6e4 100644
--- a/clang-tools-extra/clang-tidy/ClangTidyForceLinker.h
+++ b/clang-tools-extra/clang-tidy/ClangTidyForceLinker.h
@@ -94,6 +94,11 @@ extern volatile int MiscModuleAnchorSource;
static int LLVM_ATTRIBUTE_UNUSED MiscModuleAnchorDestination =
MiscModuleAnchorSource;
+// This anchor is used to force the linker to link the MLIRModule.
+extern volatile int MLIRModuleAnchorSource;
+static int LLVM_ATTRIBUTE_UNUSED MLIRModuleAnchorDestination =
+ MLIRModuleAnchorSource;
+
// This anchor is used to force the linker to link the ModernizeModule.
extern volatile int ModernizeModuleAnchorSource;
static int LLVM_ATTRIBUTE_UNUSED ModernizeModuleAnchorDestination =
diff --git a/clang-tools-extra/clang-tidy/mlir/CMakeLists.txt b/clang-tools-extra/clang-tidy/mlir/CMakeLists.txt
new file mode 100644
index 0000000000000..7d0b2de1df24c
--- /dev/null
+++ b/clang-tools-extra/clang-tidy/mlir/CMakeLists.txt
@@ -0,0 +1,28 @@
+set(LLVM_LINK_COMPONENTS
+ FrontendOpenMP
+ Support
+ )
+
+add_clang_library(clangTidyMLIRModule STATIC
+ MLIRTidyModule.cpp
+ OpBuilderCheck.cpp
+
+ LINK_LIBS
+ clangTidy
+ clangTidyReadabilityModule
+ clangTidyUtils
+ clangTransformer
+
+ DEPENDS
+ omp_gen
+ ClangDriverOptions
+ )
+
+clang_target_link_libraries(clangTidyMLIRModule
+ PRIVATE
+ clangAST
+ clangASTMatchers
+ clangBasic
+ clangLex
+ clangTooling
+ )
diff --git a/clang-tools-extra/clang-tidy/mlir/MLIRTidyModule.cpp b/clang-tools-extra/clang-tidy/mlir/MLIRTidyModule.cpp
new file mode 100644
index 0000000000000..f542020a0afdd
--- /dev/null
+++ b/clang-tools-extra/clang-tidy/mlir/MLIRTidyModule.cpp
@@ -0,0 +1,39 @@
+//===--- MLIRTidyModule.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 "../ClangTidy.h"
+#include "../ClangTidyModule.h"
+#include "../ClangTidyModuleRegistry.h"
+#include "OpBuilderCheck.h"
+
+namespace clang::tidy {
+namespace mlir_check {
+
+class MLIRModule : public ClangTidyModule {
+public:
+ void addCheckFactories(ClangTidyCheckFactories &CheckFactories) override {
+ CheckFactories.registerCheck<OpBuilderCheck>("mlir-op-builder");
+ }
+
+ ClangTidyOptions getModuleOptions() override {
+ ClangTidyOptions Options;
+ return Options;
+ }
+};
+
+// Register the ModuleModule using this statically initialized variable.
+static ClangTidyModuleRegistry::Add<MLIRModule> X("mlir-module",
+ "Adds MLIR lint checks.");
+
+} // namespace mlir_check
+
+// This anchor is used to force the linker to link in the generated object file
+// and thus register the MLIRModule.
+volatile int MLIRModuleAnchorSource = 0; // NOLINT(misc-use-internal-linkage)
+
+} // namespace clang::tidy
diff --git a/clang-tools-extra/clang-tidy/mlir/OpBuilderCheck.cpp b/clang-tools-extra/clang-tidy/mlir/OpBuilderCheck.cpp
new file mode 100644
index 0000000000000..7521096d5b91d
--- /dev/null
+++ b/clang-tools-extra/clang-tidy/mlir/OpBuilderCheck.cpp
@@ -0,0 +1,102 @@
+//===--- OpBuilderCheck.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 "OpBuilderCheck.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"
+
+namespace clang::tidy::mlir_check {
+namespace {
+
+using namespace ::clang::ast_matchers; // NOLINT: Too many names.
+using namespace ::clang::transformer; // NOLINT: Too many names.
+
+class TypeAsWrittenStencil : public StencilInterface {
+public:
+ explicit TypeAsWrittenStencil(std::string S) : id(std::move(S)) {}
+ std::string toString() const override {
+ return (llvm::Twine("TypeAsWritten(\"") + id + "\")").str();
+ }
+
+ llvm::Error eval(const MatchFinder::MatchResult &match,
+ std::string *result) const override {
+ auto n = node(id)(match);
+ if (!n)
+ return n.takeError();
+ auto srcRange = n->getAsRange();
+ if (srcRange.isInvalid()) {
+ return llvm::make_error<llvm::StringError>(llvm::errc::invalid_argument,
+ "srcRange is invalid");
+ }
+ auto range = n->getTokenRange(srcRange);
+ auto nextToken = [&](std::optional<Token> token) {
+ if (!token)
+ return token;
+ return clang::Lexer::findNextToken(token->getLocation(),
+ *match.SourceManager,
+ match.Context->getLangOpts());
+ };
+ auto lessToken = clang::Lexer::findNextToken(
+ range.getBegin(), *match.SourceManager, match.Context->getLangOpts());
+ 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 (auto 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");
+ }
+ *result += clang::tooling::getText(
+ CharSourceRange::getTokenRange(lessToken->getEndLoc(),
+ endToken->getLastLoc()),
+ *match.Context);
+ return llvm::Error::success();
+ }
+ std::string id;
+};
+
+Stencil typeAsWritten(StringRef Id) {
+ // Using this instead of `describe` so that we get the exact same spelling.
+ return std::make_shared<TypeAsWrittenStencil>(std::string(Id));
+}
+
+RewriteRuleWith<std::string> OpBuilderCheckRule() {
+ return makeRule(
+ cxxMemberCallExpr(
+ on(expr(hasType(
+ cxxRecordDecl(isSameOrDerivedFrom("::mlir::OpBuilder"))))
+ .bind("builder")),
+ callee(cxxMethodDecl(hasTemplateArgument(0, templateArgument()))),
+ callee(cxxMethodDecl(hasName("create"))))
+ .bind("call"),
+ changeTo(cat(typeAsWritten("call"), "::create(", node("builder"), ", ",
+ callArgs("call"), ")")),
+ cat("Use OpType::create(builder, ...) instead of "
+ "builder.create<OpType>(...)"));
+}
+} // namespace
+
+OpBuilderCheck::OpBuilderCheck(StringRef Name, ClangTidyContext *Context)
+ : TransformerClangTidyCheck(OpBuilderCheckRule(), Name, Context) {}
+
+} // namespace clang::tidy::mlir_check
diff --git a/clang-tools-extra/clang-tidy/mlir/OpBuilderCheck.h b/clang-tools-extra/clang-tidy/mlir/OpBuilderCheck.h
new file mode 100644
index 0000000000000..792ac7b782add
--- /dev/null
+++ b/clang-tools-extra/clang-tidy/mlir/OpBuilderCheck.h
@@ -0,0 +1,21 @@
+#ifndef LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_MLIR_OPBUILDERCHECK_H
+#define LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_MLIR_OPBUILDERCHECK_H
+
+#include "../utils/TransformerClangTidyCheck.h"
+
+namespace clang::tidy::mlir_check {
+
+/// Checks for uses of `OpBuilder::create<T>` and suggests using `T::create`
+/// instead.
+class OpBuilderCheck : public utils::TransformerClangTidyCheck {
+public:
+ OpBuilderCheck(StringRef Name, ClangTidyContext *Context);
+
+ bool isLanguageVersionSupported(const LangOptions &LangOpts) const override {
+ return getLangOpts().CPlusPlus;
+ }
+};
+
+} // namespace clang::tidy::mlir_check
+
+#endif // LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_MLIR_OPBUILDERCHECK_H
diff --git a/clang-tools-extra/docs/ReleaseNotes.rst b/clang-tools-extra/docs/ReleaseNotes.rst
index 9eb3835fe8340..466b2a9a7e84c 100644
--- a/clang-tools-extra/docs/ReleaseNotes.rst
+++ b/clang-tools-extra/docs/ReleaseNotes.rst
@@ -162,6 +162,11 @@ New checks
Finds uses of ``std::lock_guard`` and suggests replacing them with C++17's
alternative ``std::scoped_lock``.
+- New :doc:`mlir-op-builder
+ <clang-tidy/checks/mlir/op-builder>` check.
+
+ Flags usage of old OpBuilder format.
+
- New :doc:`portability-avoid-pragma-once
<clang-tidy/checks/portability/avoid-pragma-once>` check.
diff --git a/clang-tools-extra/docs/clang-tidy/checks/list.rst b/clang-tools-extra/docs/clang-tidy/checks/list.rst
index 0cffbd323caa2..49cd008e7588c 100644
--- a/clang-tools-extra/docs/clang-tidy/checks/list.rst
+++ b/clang-tools-extra/docs/clang-tidy/checks/list.rst
@@ -24,6 +24,7 @@ Clang-Tidy Checks
llvm/*
llvmlibc/*
misc/*
+ mlir/*
modernize/*
mpi/*
objc/*
@@ -279,6 +280,7 @@ Clang-Tidy Checks
:doc:`misc-unused-using-decls <misc/unused-using-decls>`, "Yes"
:doc:`misc-use-anonymous-namespace <misc/use-anonymous-namespace>`,
:doc:`misc-use-internal-linkage <misc/use-internal-linkage>`, "Yes"
+ :doc:`mlir-op-builder <mlir/op-builder>`, "Yes"
:doc:`modernize-avoid-bind <modernize/avoid-bind>`, "Yes"
:doc:`modernize-avoid-c-arrays <modernize/avoid-c-arrays>`,
:doc:`modernize-concat-nested-namespaces <modernize/concat-nested-namespaces>`, "Yes"
diff --git a/clang-tools-extra/docs/clang-tidy/checks/mlir/op-builder.rst b/clang-tools-extra/docs/clang-tidy/checks/mlir/op-builder.rst
new file mode 100644
index 0000000000000..30bae06a36836
--- /dev/null
+++ b/clang-tools-extra/docs/clang-tidy/checks/mlir/op-builder.rst
@@ -0,0 +1,22 @@
+.. title:: clang-tidy - mlir-op-builder
+
+mlir-op-builder
+===============
+
+Flags usage of old form of invoking create on `OpBuilder` and suggesting new
+form.
+
+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/mlir/op_builder.cpp b/clang-tools-extra/test/clang-tidy/checkers/mlir/op_builder.cpp
new file mode 100644
index 0000000000000..bf60c665e86ad
--- /dev/null
+++ b/clang-tools-extra/test/clang-tidy/checkers/mlir/op_builder.cpp
@@ -0,0 +1,51 @@
+// RUN: %check_clang_tidy --match-partial-fixes %s 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
+
+void f() {
+ mlir::OpBuilder builder;
+ // CHECK-MESSAGES: :[[@LINE+2]]:3: warning: Use OpType::create(builder, ...) instead of builder.create<OpType>(...) [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>(...) [mlir-op-builder]
+ // CHECK-FIXES: NamedOp::create(builder, builder.getUnknownLoc(), "baz")
+ builder.create<NamedOp>(builder.getUnknownLoc(), "baz");
+
+ mlir::ImplicitLocOpBuilder ib;
+ // Note: extra space in the case where there is no other arguments. Could be
+ // improved, but also clang-format will do that just post.
+ // CHECK-MESSAGES: :[[@LINE+2]]:3: warning: Use OpType::create(builder, ...) instead of builder.create<OpType>(...) [mlir-op-builder]
+ // CHECK-FIXES: mlir::ModuleOp::create(ib )
+ ib.create<mlir::ModuleOp>();
+}
More information about the cfe-commits
mailing list