https://github.com/jpienaar updated https://github.com/llvm/llvm-project/pull/179882
>From 15b62ca167070f8800f66c01eff513a82c898438 Mon Sep 17 00:00:00 2001 From: Jacques Pienaar <[email protected]> Date: Thu, 19 Mar 2026 16:18:42 +0000 Subject: [PATCH 1/2] Check for flagging ops that should not be passed reference. This is for example for cases like mlir::Op derived ones which are value types. The check itself is parameterized so that different base classes can be checked against. --- .../llvm/AvoidPassingAsRefCheck.cpp | 77 +++++++++++++++++++ .../clang-tidy/llvm/AvoidPassingAsRefCheck.h | 37 +++++++++ .../clang-tidy/llvm/CMakeLists.txt | 1 + .../clang-tidy/llvm/LLVMTidyModule.cpp | 3 + clang-tools-extra/docs/ReleaseNotes.rst | 6 ++ .../docs/clang-tidy/checks/list.rst | 1 + .../checks/llvm/avoid-passing-as-ref.rst | 27 +++++++ .../checkers/llvm/avoid-passing-as-ref.cpp | 33 ++++++++ 8 files changed, 185 insertions(+) create mode 100644 clang-tools-extra/clang-tidy/llvm/AvoidPassingAsRefCheck.cpp create mode 100644 clang-tools-extra/clang-tidy/llvm/AvoidPassingAsRefCheck.h create mode 100644 clang-tools-extra/docs/clang-tidy/checks/llvm/avoid-passing-as-ref.rst create mode 100644 clang-tools-extra/test/clang-tidy/checkers/llvm/avoid-passing-as-ref.cpp diff --git a/clang-tools-extra/clang-tidy/llvm/AvoidPassingAsRefCheck.cpp b/clang-tools-extra/clang-tidy/llvm/AvoidPassingAsRefCheck.cpp new file mode 100644 index 0000000000000..2918a9ba6ee9c --- /dev/null +++ b/clang-tools-extra/clang-tidy/llvm/AvoidPassingAsRefCheck.cpp @@ -0,0 +1,77 @@ +//===--- AvoidPassingAsRefCheck.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 "AvoidPassingAsRefCheck.h" +#include "../utils/OptionsUtils.h" +#include "clang/AST/ASTContext.h" +#include "clang/ASTMatchers/ASTMatchFinder.h" + +using namespace clang::ast_matchers; + +namespace clang::tidy::llvm_check { + +AvoidPassingAsRefCheck::AvoidPassingAsRefCheck(StringRef Name, + ClangTidyContext *Context) + : ClangTidyCheck(Name, Context), + ClassNames(Options.get("ClassNames", "::mlir::Op")) { + if (!utils::options::parseStringList(ClassNames, ClassNameList) && + !ClassNames.empty()) { + Context->getDiagnosticsEngine().Report( + Context->getDiagID(diag::err_invalid_option_value), SourceLocation{}) + << "invalid value for 'ClassNames'" << ClassNames; + } +} + +void AvoidPassingAsRefCheck::storeOptions(ClangTidyOptions::OptionMap &Opts) { + Options.store(Opts, "ClassNames", ClassNames); +} + +void AvoidPassingAsRefCheck::registerMatchers(MatchFinder *Finder) { + if (ClassNameList.empty()) + return; + + std::vector<ast_matchers::DeclarationMatcher> Matchers; + for (const auto &Name : ClassNameList) + Matchers.push_back(isSameOrDerivedFrom(Name)); + + // Match parameters that are references to classes derived from any class in + // ClassNameList. + Finder->addMatcher( + parmVarDecl( + hasType(qualType( + references(qualType(hasDeclaration( + cxxRecordDecl(anyOfArrayRef(Matchers)).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 AvoidPassingAsRefCheck::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; + + // We should verify if the type is exactly what we expect. + // The matcher `isSameOrDerivedFrom` handles inheritance. + + diag(Param->getLocation(), + "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/AvoidPassingAsRefCheck.h b/clang-tools-extra/clang-tidy/llvm/AvoidPassingAsRefCheck.h new file mode 100644 index 0000000000000..42647bcc91001 --- /dev/null +++ b/clang-tools-extra/clang-tidy/llvm/AvoidPassingAsRefCheck.h @@ -0,0 +1,37 @@ +//===--- AvoidPassingAsRefCheck.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_AVOIDPASSINGASREFCHECK_H +#define LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_LLVM_AVOIDPASSINGASREFCHECK_H + +#include "../ClangTidyCheck.h" +#include <string> +#include <vector> + +namespace clang::tidy::llvm_check { + +/// Flags function parameters of types that should be passed by value but are +/// passed by reference instead. +/// +/// For the user-facing documentation see: +/// http://clang.llvm.org/extra/clang-tidy/checks/llvm/avoid-passing-as-ref.html +class AvoidPassingAsRefCheck : public ClangTidyCheck { +public: + AvoidPassingAsRefCheck(StringRef Name, ClangTidyContext *Context); + void registerMatchers(ast_matchers::MatchFinder *Finder) override; + void check(const ast_matchers::MatchFinder::MatchResult &Result) override; + void storeOptions(ClangTidyOptions::OptionMap &Opts) override; + +private: + const std::string ClassNames; + std::vector<std::string> ClassNameList; +}; + +} // namespace clang::tidy::llvm_check + +#endif // LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_LLVM_AVOIDPASSINGASREFCHECK_H diff --git a/clang-tools-extra/clang-tidy/llvm/CMakeLists.txt b/clang-tools-extra/clang-tidy/llvm/CMakeLists.txt index a807f0ab65f87..407021aa1deca 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 + AvoidPassingAsRefCheck.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..6d57d473b1a92 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 "AvoidPassingAsRefCheck.h" #include "HeaderGuardCheck.h" #include "IncludeOrderCheck.h" #include "PreferIsaOrDynCastInConditionalsCheck.h" @@ -29,6 +30,8 @@ namespace { class LLVMModule : public ClangTidyModule { public: void addCheckFactories(ClangTidyCheckFactories &CheckFactories) override { + CheckFactories.registerCheck<AvoidPassingAsRefCheck>( + "llvm-avoid-passing-as-ref"); CheckFactories.registerCheck<readability::ElseAfterReturnCheck>( "llvm-else-after-return"); CheckFactories.registerCheck<LLVMHeaderGuardCheck>("llvm-header-guard"); diff --git a/clang-tools-extra/docs/ReleaseNotes.rst b/clang-tools-extra/docs/ReleaseNotes.rst index c9a170a9e8660..dbb4fafc0ad44 100644 --- a/clang-tools-extra/docs/ReleaseNotes.rst +++ b/clang-tools-extra/docs/ReleaseNotes.rst @@ -111,6 +111,12 @@ New checks Finds functions where throwing exceptions is unsafe but the function is still marked as potentially throwing. +- New :doc:`llvm-avoid-passing-as-ref + <clang-tidy/checks/llvm/avoid-passing-as-ref>` check. + + Flags function parameters of types that should be passed by value, but 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 ceab1e9414951..4143518ee7ea1 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-multiway-paths-covered <hicpp/multiway-paths-covered>`, :doc:`hicpp-signed-bitwise <hicpp/signed-bitwise>`, :doc:`linuxkernel-must-check-errs <linuxkernel/must-check-errs>`, + :doc:`llvm-avoid-passing-as-ref <llvm/avoid-passing-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-as-ref.rst b/clang-tools-extra/docs/clang-tidy/checks/llvm/avoid-passing-as-ref.rst new file mode 100644 index 0000000000000..9e35c08eb18a8 --- /dev/null +++ b/clang-tools-extra/docs/clang-tidy/checks/llvm/avoid-passing-as-ref.rst @@ -0,0 +1,27 @@ +.. title:: llvm-avoid-passing-as-ref + +Flags function parameters of types that should be passed by value, but are passed +by reference. By default, it flags types derived from ``mlir::Op`` as ``mlir::Op`` +derived classes are lightweight wrappers around a pointer and should be passed +by value. This check can be customized to flag other types using the `ClassNames` +option. + +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); + +Options +------- + +.. option:: ClassNames + + A semicolon-separated list of fully qualified class names that should be + passed by value. Default is ``"::mlir::Op"``. diff --git a/clang-tools-extra/test/clang-tidy/checkers/llvm/avoid-passing-as-ref.cpp b/clang-tools-extra/test/clang-tidy/checkers/llvm/avoid-passing-as-ref.cpp new file mode 100644 index 0000000000000..08f87dbb3582e --- /dev/null +++ b/clang-tools-extra/test/clang-tidy/checkers/llvm/avoid-passing-as-ref.cpp @@ -0,0 +1,33 @@ +// RUN: %check_clang_tidy %s llvm-avoid-passing-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-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-as-ref] +} + +// Good: passed by value +void goodFunction(MyOp op) {} + +// Good: not an Op +void otherFunction(const OtherClass &c) {} + +// Good: pointer to Op (if ever used) +void pointerFunction(MyOp *op) {} >From 09bd159953a726651c0cb729786727baef2c8c18 Mon Sep 17 00:00:00 2001 From: Jacques Pienaar <[email protected]> Date: Thu, 19 Mar 2026 16:24:09 +0000 Subject: [PATCH 2/2] Fix comment format --- clang-tools-extra/clang-tidy/llvm/AvoidPassingAsRefCheck.cpp | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/clang-tools-extra/clang-tidy/llvm/AvoidPassingAsRefCheck.cpp b/clang-tools-extra/clang-tidy/llvm/AvoidPassingAsRefCheck.cpp index 2918a9ba6ee9c..3b4db9b7883ae 100644 --- a/clang-tools-extra/clang-tidy/llvm/AvoidPassingAsRefCheck.cpp +++ b/clang-tools-extra/clang-tidy/llvm/AvoidPassingAsRefCheck.cpp @@ -66,9 +66,8 @@ void AvoidPassingAsRefCheck::check(const MatchFinder::MatchResult &Result) { if (!Param || !OpType) return; - // We should verify if the type is exactly what we expect. - // The matcher `isSameOrDerivedFrom` handles inheritance. - + // We should verify if the type is exactly what we expect. The matcher + // `isSameOrDerivedFrom` handles inheritance. diag(Param->getLocation(), "class '%0' should be passed by value, not by reference") << OpType->getName(); _______________________________________________ cfe-commits mailing list [email protected] https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
