[clang-tools-extra] [mlir] [clang-tidy][mlir] Add check for passing value type by reference. (PR #179882)
Jacques Pienaar via cfe-commits
cfe-commits at lists.llvm.org
Thu Feb 5 09:10:48 PST 2026
https://github.com/jpienaar updated https://github.com/llvm/llvm-project/pull/179882
>From 758e5c109b59098f9b72ae2a0b0e7201b5cfc0a5 Mon Sep 17 00:00:00 2001
From: Jacques Pienaar <jacques+gh at japienaar.info>
Date: Wed, 4 Feb 2026 11:22:48 +0000
Subject: [PATCH 1/3] [clang-tidy][mlir] Add check for passing value type by
reference.
This instance is effectively a wrapper around a pointer and should be
passed by value.
There are a few existing such cases in MLIR upstream. Didn't fix along
with this to avoid mixing discussions.
---
.../llvm/AvoidPassingMlirOpAsRefCheck.cpp | 51 +++++++++++++++++++
.../llvm/AvoidPassingMlirOpAsRefCheck.h | 31 +++++++++++
.../clang-tidy/llvm/CMakeLists.txt | 1 +
.../clang-tidy/llvm/LLVMTidyModule.cpp | 3 ++
clang-tools-extra/docs/ReleaseNotes.rst | 5 ++
.../docs/clang-tidy/checks/list.rst | 1 +
.../llvm/avoid-passing-mlir-op-as-ref.rst | 21 ++++++++
.../llvm/avoid-passing-mlir-op-as-ref.cpp | 33 ++++++++++++
mlir/lib/Dialect/OpenMP/IR/OpenMPDialect.cpp | 4 +-
.../SCF/Transforms/ParallelLoopFusion.cpp | 2 +-
mlir/unittests/TableGen/OpBuildGen.cpp | 2 +-
11 files changed, 150 insertions(+), 4 deletions(-)
create mode 100644 clang-tools-extra/clang-tidy/llvm/AvoidPassingMlirOpAsRefCheck.cpp
create mode 100644 clang-tools-extra/clang-tidy/llvm/AvoidPassingMlirOpAsRefCheck.h
create mode 100644 clang-tools-extra/docs/clang-tidy/checks/llvm/avoid-passing-mlir-op-as-ref.rst
create mode 100644 clang-tools-extra/test/clang-tidy/checkers/llvm/avoid-passing-mlir-op-as-ref.cpp
diff --git a/clang-tools-extra/clang-tidy/llvm/AvoidPassingMlirOpAsRefCheck.cpp b/clang-tools-extra/clang-tidy/llvm/AvoidPassingMlirOpAsRefCheck.cpp
new file mode 100644
index 0000000000000..dc8d14632582a
--- /dev/null
+++ b/clang-tools-extra/clang-tidy/llvm/AvoidPassingMlirOpAsRefCheck.cpp
@@ -0,0 +1,51 @@
+//===--- AvoidPassingMlirOpAsRefCheck.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 "AvoidPassingMlirOpAsRefCheck.h"
+#include "clang/AST/ASTContext.h"
+#include "clang/ASTMatchers/ASTMatchFinder.h"
+
+using namespace clang::ast_matchers;
+
+namespace clang::tidy::llvm_check {
+
+void AvoidPassingMlirOpAsRefCheck::registerMatchers(MatchFinder *Finder) {
+ // Match parameters that are references to classes derived from mlir::Op.
+ Finder->addMatcher(
+ parmVarDecl(
+ hasType(qualType(
+ references(qualType(hasDeclaration(
+ cxxRecordDecl(isSameOrDerivedFrom("::mlir::Op"))
+ .bind("op_type")))),
+ // We want to avoid matching `Op *&` (reference to pointer to Op)
+ // which is not common for Op but possible.
+ unless(references(pointerType())))),
+ unless(isImplicit()),
+ unless(hasAncestor(cxxConstructorDecl(
+ anyOf(isCopyConstructor(), isMoveConstructor())))),
+ unless(hasAncestor(cxxMethodDecl(isCopyAssignmentOperator()))),
+ unless(hasAncestor(cxxMethodDecl(isMoveAssignmentOperator()))),
+ decl().bind("param")),
+ this);
+}
+
+void AvoidPassingMlirOpAsRefCheck::check(
+ const MatchFinder::MatchResult &Result) {
+ const auto *Param = Result.Nodes.getNodeAs<ParmVarDecl>("param");
+ const auto *OpType = Result.Nodes.getNodeAs<CXXRecordDecl>("op_type");
+
+ // Exclude if we can't find definitions.
+ if (!Param || !OpType)
+ return;
+
+ diag(Param->getLocation(),
+ "MLIR Op class '%0' should be passed by value, not by reference")
+ << OpType->getName();
+}
+
+} // namespace clang::tidy::llvm_check
diff --git a/clang-tools-extra/clang-tidy/llvm/AvoidPassingMlirOpAsRefCheck.h b/clang-tools-extra/clang-tidy/llvm/AvoidPassingMlirOpAsRefCheck.h
new file mode 100644
index 0000000000000..ff9a1172f52d8
--- /dev/null
+++ b/clang-tools-extra/clang-tidy/llvm/AvoidPassingMlirOpAsRefCheck.h
@@ -0,0 +1,31 @@
+//===--- AvoidPassingMlirOpAsRefCheck.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_AVOIDPASSINGMLIROPASREFCHECK_H
+#define LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_LLVM_AVOIDPASSINGMLIROPASREFCHECK_H
+
+#include "../ClangTidyCheck.h"
+
+namespace clang::tidy::llvm_check {
+
+/// Flags function parameters of a type derived from mlir::Op that are passed by
+/// reference.
+///
+/// For the user-facing documentation see:
+/// http://clang.llvm.org/extra/clang-tidy/checks/llvm/avoid-passing-mlir-op-as-ref.html
+class AvoidPassingMlirOpAsRefCheck : public ClangTidyCheck {
+public:
+ AvoidPassingMlirOpAsRefCheck(StringRef Name, ClangTidyContext *Context)
+ : ClangTidyCheck(Name, Context) {}
+ void registerMatchers(ast_matchers::MatchFinder *Finder) override;
+ void check(const ast_matchers::MatchFinder::MatchResult &Result) override;
+};
+
+} // namespace clang::tidy::llvm_check
+
+#endif // LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_LLVM_AVOIDPASSINGMLIROPASREFCHECK_H
diff --git a/clang-tools-extra/clang-tidy/llvm/CMakeLists.txt b/clang-tools-extra/clang-tidy/llvm/CMakeLists.txt
index a807f0ab65f87..919ebffd19ac8 100644
--- a/clang-tools-extra/clang-tidy/llvm/CMakeLists.txt
+++ b/clang-tools-extra/clang-tidy/llvm/CMakeLists.txt
@@ -4,6 +4,7 @@ set(LLVM_LINK_COMPONENTS
)
add_clang_library(clangTidyLLVMModule STATIC
+ AvoidPassingMlirOpAsRefCheck.cpp
HeaderGuardCheck.cpp
IncludeOrderCheck.cpp
LLVMTidyModule.cpp
diff --git a/clang-tools-extra/clang-tidy/llvm/LLVMTidyModule.cpp b/clang-tools-extra/clang-tidy/llvm/LLVMTidyModule.cpp
index c180574bdeed6..22a2d4a747c96 100644
--- a/clang-tools-extra/clang-tidy/llvm/LLVMTidyModule.cpp
+++ b/clang-tools-extra/clang-tidy/llvm/LLVMTidyModule.cpp
@@ -11,6 +11,7 @@
#include "../readability/ElseAfterReturnCheck.h"
#include "../readability/NamespaceCommentCheck.h"
#include "../readability/QualifiedAutoCheck.h"
+#include "AvoidPassingMlirOpAsRefCheck.h"
#include "HeaderGuardCheck.h"
#include "IncludeOrderCheck.h"
#include "PreferIsaOrDynCastInConditionalsCheck.h"
@@ -48,6 +49,8 @@ class LLVMModule : public ClangTidyModule {
"llvm-type-switch-case-types");
CheckFactories.registerCheck<UseNewMlirOpBuilderCheck>(
"llvm-use-new-mlir-op-builder");
+ CheckFactories.registerCheck<AvoidPassingMlirOpAsRefCheck>(
+ "llvm-avoid-passing-mlir-op-as-ref");
CheckFactories.registerCheck<UseRangesCheck>("llvm-use-ranges");
CheckFactories.registerCheck<UseVectorUtilsCheck>("llvm-use-vector-utils");
}
diff --git a/clang-tools-extra/docs/ReleaseNotes.rst b/clang-tools-extra/docs/ReleaseNotes.rst
index ea80502735ede..ef62284e0e453 100644
--- a/clang-tools-extra/docs/ReleaseNotes.rst
+++ b/clang-tools-extra/docs/ReleaseNotes.rst
@@ -97,6 +97,11 @@ Improvements to clang-tidy
New checks
^^^^^^^^^^
+- New :doc:`avoid-passing-mlir-op-as-ref`
+ <clang-tidy/checks/llvm/avoid-passing-mlir-op-as-ref>` check.
+
+ Finds cases where ``mlir::Op`` derived classes are passed by reference.
+
- New :doc:`llvm-type-switch-case-types
<clang-tidy/checks/llvm/type-switch-case-types>` check.
diff --git a/clang-tools-extra/docs/clang-tidy/checks/list.rst b/clang-tools-extra/docs/clang-tidy/checks/list.rst
index 0eabd9929dc39..db2ae177b8eee 100644
--- a/clang-tools-extra/docs/clang-tidy/checks/list.rst
+++ b/clang-tools-extra/docs/clang-tidy/checks/list.rst
@@ -245,6 +245,7 @@ Clang-Tidy Checks
:doc:`hicpp-no-assembler <hicpp/no-assembler>`,
:doc:`hicpp-signed-bitwise <hicpp/signed-bitwise>`,
:doc:`linuxkernel-must-check-errs <linuxkernel/must-check-errs>`,
+ :doc:`llvm-avoid-passing-mlir-op-as-ref <llvm/avoid-passing-mlir-op-as-ref>`,
:doc:`llvm-header-guard <llvm/header-guard>`,
:doc:`llvm-include-order <llvm/include-order>`, "Yes"
:doc:`llvm-namespace-comment <llvm/namespace-comment>`,
diff --git a/clang-tools-extra/docs/clang-tidy/checks/llvm/avoid-passing-mlir-op-as-ref.rst b/clang-tools-extra/docs/clang-tidy/checks/llvm/avoid-passing-mlir-op-as-ref.rst
new file mode 100644
index 0000000000000..882ab18c4ccd5
--- /dev/null
+++ b/clang-tools-extra/docs/clang-tidy/checks/llvm/avoid-passing-mlir-op-as-ref.rst
@@ -0,0 +1,21 @@
+.. title:: clang-tidy - llvm-avoid-passing-mlir-op-as-ref
+
+llvm-avoid-passing-mlir-op-as-ref
+=================================
+
+Flags function parameters of a type derived from ``mlir::Op`` that are passed by
+reference. ``mlir::Op`` derived classes are lightweight wrappers around a pointer
+and should be passed by value.
+
+Example
+-------
+
+.. code-block:: c++
+
+ // Bad: passed by reference
+ void processOp(const MyOp &op);
+ void mutateOp(MyOp &op);
+
+ // Good: passed by value
+ void processOp(MyOp op);
+ void mutateOp(MyOp op);
diff --git a/clang-tools-extra/test/clang-tidy/checkers/llvm/avoid-passing-mlir-op-as-ref.cpp b/clang-tools-extra/test/clang-tidy/checkers/llvm/avoid-passing-mlir-op-as-ref.cpp
new file mode 100644
index 0000000000000..f76d6ebd3ce0e
--- /dev/null
+++ b/clang-tools-extra/test/clang-tidy/checkers/llvm/avoid-passing-mlir-op-as-ref.cpp
@@ -0,0 +1,33 @@
+// RUN: %check_clang_tidy %s llvm-avoid-passing-mlir-op-as-ref %t
+
+namespace mlir {
+template <typename ConcreteType, template <typename T> class... Traits>
+class Op {
+public:
+ // Minimal definition to satisfy isSameOrDerivedFrom
+};
+} // namespace mlir
+
+class MyOp : public mlir::Op<MyOp> {
+ using Op::Op;
+};
+class OtherClass {};
+
+// Should trigger warning
+void badFunction(const MyOp &op) {
+ // CHECK-MESSAGES: :[[@LINE-1]]:30: warning: MLIR Op class 'MyOp' should be passed by value, not by reference [llvm-avoid-passing-mlir-op-as-ref]
+}
+
+// Should trigger warning logic for non-const ref too
+void badFunctionMutable(MyOp &op) {
+ // CHECK-MESSAGES: :[[@LINE-1]]:31: warning: MLIR Op class 'MyOp' should be passed by value, not by reference [llvm-avoid-passing-mlir-op-as-ref]
+}
+
+// Good: passed by value
+void goodFunction(MyOp op) {}
+
+// Good: not an Op
+void otherFunction(const OtherClass &c) {}
+
+// Good: pointer to Op (not common nor necessarily good practice)
+void pointerFunction(MyOp *op) {}
diff --git a/mlir/lib/Dialect/OpenMP/IR/OpenMPDialect.cpp b/mlir/lib/Dialect/OpenMP/IR/OpenMPDialect.cpp
index c3916219d1c93..786073533bfb1 100644
--- a/mlir/lib/Dialect/OpenMP/IR/OpenMPDialect.cpp
+++ b/mlir/lib/Dialect/OpenMP/IR/OpenMPDialect.cpp
@@ -1548,7 +1548,7 @@ verifyCopyprivateVarList(Operation *op, OperandRange copyprivateVars,
funcOp = llvmFuncOp;
auto getNumArguments = [&] {
- return std::visit([](auto &f) { return f.getNumArguments(); }, *funcOp);
+ return std::visit([](auto f) { return f.getNumArguments(); }, *funcOp);
};
auto getArgumentType = [&](unsigned i) {
@@ -2526,7 +2526,7 @@ void ParallelOp::build(OpBuilder &builder, OperationState &state,
}
template <typename OpType>
-static LogicalResult verifyPrivateVarList(OpType &op) {
+static LogicalResult verifyPrivateVarList(OpType op) {
auto privateVars = op.getPrivateVars();
auto privateSyms = op.getPrivateSymsAttr();
diff --git a/mlir/lib/Dialect/SCF/Transforms/ParallelLoopFusion.cpp b/mlir/lib/Dialect/SCF/Transforms/ParallelLoopFusion.cpp
index 4ea832177c4f9..45e1537c049a2 100644
--- a/mlir/lib/Dialect/SCF/Transforms/ParallelLoopFusion.cpp
+++ b/mlir/lib/Dialect/SCF/Transforms/ParallelLoopFusion.cpp
@@ -163,7 +163,7 @@ static bool isFusionLegal(ParallelOp firstPloop, ParallelOp secondPloop,
/// Prepends operations of firstPloop's body into secondPloop's body.
/// Updates secondPloop with new loop.
-static void fuseIfLegal(ParallelOp firstPloop, ParallelOp &secondPloop,
+static void fuseIfLegal(ParallelOp firstPloop, ParallelOp secondPloop,
OpBuilder builder,
llvm::function_ref<bool(Value, Value)> mayAlias) {
Block *block1 = firstPloop.getBody();
diff --git a/mlir/unittests/TableGen/OpBuildGen.cpp b/mlir/unittests/TableGen/OpBuildGen.cpp
index 09430336de994..5f275ae060cc0 100644
--- a/mlir/unittests/TableGen/OpBuildGen.cpp
+++ b/mlir/unittests/TableGen/OpBuildGen.cpp
@@ -46,7 +46,7 @@ class OpBuildGenTest : public ::testing::Test {
// Verify that `op` has the given set of result types, operands, and
// attributes.
template <typename OpTy>
- void verifyOp(OpTy &&concreteOp, std::vector<Type> resultTypes,
+ void verifyOp(OpTy concreteOp, std::vector<Type> resultTypes,
std::vector<Value> operands,
std::vector<NamedAttribute> attrs) {
ASSERT_NE(concreteOp, nullptr);
>From e691eeb528c3befc2a88eba8e51abcbb2ed9b288 Mon Sep 17 00:00:00 2001
From: Jacques Pienaar <jacques+gh at japienaar.info>
Date: Thu, 5 Feb 2026 08:34:06 +0000
Subject: [PATCH 2/3] Fix typoe and too long line
---
clang-tools-extra/docs/ReleaseNotes.rst | 2 +-
.../clang-tidy/checks/llvm/avoid-passing-mlir-op-as-ref.rst | 6 +++---
2 files changed, 4 insertions(+), 4 deletions(-)
diff --git a/clang-tools-extra/docs/ReleaseNotes.rst b/clang-tools-extra/docs/ReleaseNotes.rst
index ef62284e0e453..30774b81dfdac 100644
--- a/clang-tools-extra/docs/ReleaseNotes.rst
+++ b/clang-tools-extra/docs/ReleaseNotes.rst
@@ -97,7 +97,7 @@ Improvements to clang-tidy
New checks
^^^^^^^^^^
-- New :doc:`avoid-passing-mlir-op-as-ref`
+- New :doc:`avoid-passing-mlir-op-as-ref
<clang-tidy/checks/llvm/avoid-passing-mlir-op-as-ref>` check.
Finds cases where ``mlir::Op`` derived classes are passed by reference.
diff --git a/clang-tools-extra/docs/clang-tidy/checks/llvm/avoid-passing-mlir-op-as-ref.rst b/clang-tools-extra/docs/clang-tidy/checks/llvm/avoid-passing-mlir-op-as-ref.rst
index 882ab18c4ccd5..9a64f71d7b729 100644
--- a/clang-tools-extra/docs/clang-tidy/checks/llvm/avoid-passing-mlir-op-as-ref.rst
+++ b/clang-tools-extra/docs/clang-tidy/checks/llvm/avoid-passing-mlir-op-as-ref.rst
@@ -3,9 +3,9 @@
llvm-avoid-passing-mlir-op-as-ref
=================================
-Flags function parameters of a type derived from ``mlir::Op`` that are passed by
-reference. ``mlir::Op`` derived classes are lightweight wrappers around a pointer
-and should be passed by value.
+Flags function parameters of a type derived from ``mlir::Op`` that are passed
+by reference. ``mlir::Op`` derived classes are lightweight wrappers around a
+pointer and should be passed by value.
Example
-------
>From 7fdced08b1987457fecca7ca117e21e151333a34 Mon Sep 17 00:00:00 2001
From: Jacques Pienaar <jacques+gh at japienaar.info>
Date: Thu, 5 Feb 2026 17:10:29 +0000
Subject: [PATCH 3/3] Sync release notes
---
clang-tools-extra/docs/ReleaseNotes.rst | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/clang-tools-extra/docs/ReleaseNotes.rst b/clang-tools-extra/docs/ReleaseNotes.rst
index 30774b81dfdac..6046215cf0a46 100644
--- a/clang-tools-extra/docs/ReleaseNotes.rst
+++ b/clang-tools-extra/docs/ReleaseNotes.rst
@@ -100,7 +100,8 @@ New checks
- New :doc:`avoid-passing-mlir-op-as-ref
<clang-tidy/checks/llvm/avoid-passing-mlir-op-as-ref>` check.
- Finds cases where ``mlir::Op`` derived classes are passed by reference.
+ Flags function parameters of a type derived from ``mlir::Op`` that are passed
+ by reference.
- New :doc:`llvm-type-switch-case-types
<clang-tidy/checks/llvm/type-switch-case-types>` check.
More information about the cfe-commits
mailing list