https://github.com/hjanuschka updated https://github.com/llvm/llvm-project/pull/182068
>From f77db6e05948bdf4a64fa88e1eaae1a7c334991a Mon Sep 17 00:00:00 2001 From: Helmut Januschka <[email protected]> Date: Wed, 18 Feb 2026 18:13:54 +0100 Subject: [PATCH] [clang-tidy] Add readability-pointer-to-ref check Add new clang-tidy check that finds function parameters that are pointers but are always dereferenced without null checks, suggesting they should be references instead. Using references for non-nullable parameters makes the contract explicit and improves readability. For example: ```cpp // Before void process(Foo *P) { P->bar(); P->X = 42; } // Suggested -- use a reference void process(Foo &P) { P.bar(); P.X = 42; } ``` The check skips parameters that are: - null-checked (if(ptr), ptr != nullptr, etc.) - passed to other functions as pointers - used in pointer arithmetic or array subscript - void pointers or pointers to incomplete types - parameters of virtual methods - function pointers - unnamed or never dereferenced No fix-its are provided since changing the signature requires updating all call sites, which may span translation units. --- .../clang-tidy/readability/CMakeLists.txt | 1 + .../readability/PointerToRefCheck.cpp | 199 ++++++++++++++++++ .../readability/PointerToRefCheck.h | 31 +++ .../readability/ReadabilityTidyModule.cpp | 3 + clang-tools-extra/docs/ReleaseNotes.rst | 7 + .../docs/clang-tidy/checks/list.rst | 1 + .../checks/readability/pointer-to-ref.rst | 43 ++++ .../checkers/readability/pointer-to-ref.cpp | 86 ++++++++ 8 files changed, 371 insertions(+) create mode 100644 clang-tools-extra/clang-tidy/readability/PointerToRefCheck.cpp create mode 100644 clang-tools-extra/clang-tidy/readability/PointerToRefCheck.h create mode 100644 clang-tools-extra/docs/clang-tidy/checks/readability/pointer-to-ref.rst create mode 100644 clang-tools-extra/test/clang-tidy/checkers/readability/pointer-to-ref.cpp diff --git a/clang-tools-extra/clang-tidy/readability/CMakeLists.txt b/clang-tools-extra/clang-tidy/readability/CMakeLists.txt index f1f3cde32feff..8f591ada34ed9 100644 --- a/clang-tools-extra/clang-tidy/readability/CMakeLists.txt +++ b/clang-tools-extra/clang-tidy/readability/CMakeLists.txt @@ -37,6 +37,7 @@ add_clang_library(clangTidyReadabilityModule STATIC NamespaceCommentCheck.cpp NonConstParameterCheck.cpp OperatorsRepresentationCheck.cpp + PointerToRefCheck.cpp QualifiedAutoCheck.cpp ReadabilityTidyModule.cpp RedundantAccessSpecifiersCheck.cpp diff --git a/clang-tools-extra/clang-tidy/readability/PointerToRefCheck.cpp b/clang-tools-extra/clang-tidy/readability/PointerToRefCheck.cpp new file mode 100644 index 0000000000000..8a7f978a344d5 --- /dev/null +++ b/clang-tools-extra/clang-tidy/readability/PointerToRefCheck.cpp @@ -0,0 +1,199 @@ +//===----------------------------------------------------------------------===// +// +// 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 "PointerToRefCheck.h" +#include "clang/AST/ASTContext.h" +#include "clang/AST/RecursiveASTVisitor.h" +#include "clang/ASTMatchers/ASTMatchFinder.h" + +using namespace clang::ast_matchers; + +namespace clang::tidy::readability { + +namespace { + +/// Visitor that classifies how a pointer parameter is used in a function body. +/// It detects: +/// - Dereferences (operator*, operator->) +/// - Null checks (if(ptr), ptr != nullptr, assert(ptr), etc.) +/// - Pointer arithmetic +/// - Being passed to other functions as a pointer +/// - Address-of or array subscript usage +class PointerUsageVisitor : public RecursiveASTVisitor<PointerUsageVisitor> { +public: + explicit PointerUsageVisitor(const ParmVarDecl *Param) : Param(Param) {} + + bool isDereferenced() const { return Dereferenced; } + bool isNullChecked() const { return NullChecked; } + bool isUsedAsPointer() const { return UsedAsPointer; } + + bool VisitUnaryOperator(const UnaryOperator *UO) { + if (UO->getOpcode() == UO_Deref && refersToParam(UO->getSubExpr())) + Dereferenced = true; + return true; + } + + bool VisitMemberExpr(const MemberExpr *ME) { + if (ME->isArrow() && refersToParam(ME->getBase())) + Dereferenced = true; + return true; + } + + // Detect null checks: if(ptr), ptr == nullptr, !ptr, etc. + bool VisitImplicitCastExpr(const ImplicitCastExpr *ICE) { + if (ICE->getCastKind() == CK_PointerToBoolean && + refersToParam(ICE->getSubExpr())) + NullChecked = true; + return true; + } + + bool VisitBinaryOperator(const BinaryOperator *BO) { + const Expr *LHS = BO->getLHS()->IgnoreImplicit(); + const Expr *RHS = BO->getRHS()->IgnoreImplicit(); + + // ptr == nullptr, nullptr == ptr, ptr != nullptr, etc. + if (BO->isEqualityOp()) { + if ((refersToParam(LHS) && isNullExpr(RHS)) || + (refersToParam(RHS) && isNullExpr(LHS))) + NullChecked = true; + } + + // Pointer comparison (relational). + if (BO->isRelationalOp() && (refersToParam(LHS) || refersToParam(RHS))) + UsedAsPointer = true; + + // Pointer arithmetic: ptr + n, ptr - n. + if ((BO->getOpcode() == BO_Add || BO->getOpcode() == BO_Sub) && + (refersToParam(LHS) || refersToParam(RHS))) + UsedAsPointer = true; + + // Reassignment: ptr = something. + if (BO->getOpcode() == BO_Assign && refersToParam(LHS)) + UsedAsPointer = true; + + return true; + } + + // Detect being passed to a function expecting a pointer. + bool VisitCallExpr(const CallExpr *CE) { + for (unsigned I = 0; I < CE->getNumArgs(); ++I) + if (refersToParam(CE->getArg(I)->IgnoreImplicit())) + UsedAsPointer = true; + return true; + } + + // Detect array subscript: ptr[i] + bool VisitArraySubscriptExpr(const ArraySubscriptExpr *ASE) { + if (refersToParam(ASE->getBase()->IgnoreImplicit())) + UsedAsPointer = true; + return true; + } + + // Detect address-taken or assigned to another pointer. + bool VisitDeclRefExpr(const DeclRefExpr *DRE) { + // We handle specific patterns above; just track raw usage for + // cases we might miss (e.g. storing to another variable). + return true; + } + +private: + bool refersToParam(const Expr *E) const { + if (!E) + return false; + const auto *DRE = dyn_cast<DeclRefExpr>(E->IgnoreImplicit()); + return DRE && DRE->getDecl() == Param; + } + + static bool isNullExpr(const Expr *E) { + if (!E) + return false; + E = E->IgnoreImplicit(); + if (isa<GNUNullExpr>(E)) + return true; + if (const auto *IL = dyn_cast<IntegerLiteral>(E)) + return IL->getValue() == 0; + if (isa<CXXNullPtrLiteralExpr>(E)) + return true; + return false; + } + + const ParmVarDecl *Param; + bool Dereferenced = false; + bool NullChecked = false; + bool UsedAsPointer = false; +}; + +} // namespace + +void PointerToRefCheck::registerMatchers(MatchFinder *Finder) { + // Match function definitions with at least one pointer parameter. + // Match each pointer parameter individually. + Finder->addMatcher( + parmVarDecl(hasType(pointerType()), + hasAncestor(functionDecl(isDefinition(), unless(isImplicit()), + unless(isDeleted())) + .bind("func"))) + .bind("param"), + this); +} + +void PointerToRefCheck::check(const MatchFinder::MatchResult &Result) { + const auto *Func = Result.Nodes.getNodeAs<FunctionDecl>("func"); + const auto *Param = Result.Nodes.getNodeAs<ParmVarDecl>("param"); + if (!Func || !Param || !Func->hasBody()) + return; + + // Skip system headers. + if (Result.SourceManager->isInSystemHeader(Param->getLocation())) + return; + + // Skip variadic functions, main, and operator overloads. + if (Func->isVariadic() || Func->isMain()) + return; + if (isa<CXXMethodDecl>(Func) && + cast<CXXMethodDecl>(Func)->isOverloadedOperator()) + return; + + // Skip if the parameter is unnamed (can't be used). + if (!Param->getIdentifier()) + return; + + // Skip void pointers (too generic). + const auto *PT = Param->getType()->getAs<PointerType>(); + if (!PT || PT->getPointeeType()->isVoidType()) + return; + + // Skip pointers to incomplete types. + if (PT->getPointeeType()->isIncompleteType()) + return; + + // Skip virtual methods (changing signature breaks polymorphism). + if (const auto *MD = dyn_cast<CXXMethodDecl>(Func)) + if (MD->isVirtual()) + return; + + // Skip callbacks and function pointers (typedef params). + if (Param->getType()->isFunctionPointerType()) + return; + + // Analyze usage in the body. + PointerUsageVisitor Visitor(Param); + Visitor.TraverseStmt(Func->getBody()); + + // Only flag if the pointer is dereferenced but never null-checked + // and not used as a raw pointer (arithmetic, passed to functions, etc.). + if (Visitor.isDereferenced() && !Visitor.isNullChecked() && + !Visitor.isUsedAsPointer()) { + diag(Param->getLocation(), + "pointer parameter '%0' can be a reference; it is always " + "dereferenced but never checked for null") + << Param->getName(); + } +} + +} // namespace clang::tidy::readability diff --git a/clang-tools-extra/clang-tidy/readability/PointerToRefCheck.h b/clang-tools-extra/clang-tidy/readability/PointerToRefCheck.h new file mode 100644 index 0000000000000..cc6da410f507f --- /dev/null +++ b/clang-tools-extra/clang-tidy/readability/PointerToRefCheck.h @@ -0,0 +1,31 @@ +//===----------------------------------------------------------------------===// +// +// 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_READABILITY_POINTERTOREFCHECK_H +#define LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_READABILITY_POINTERTOREFCHECK_H + +#include "../ClangTidyCheck.h" + +namespace clang::tidy::readability { + +/// Finds function parameters that are pointers but are always dereferenced +/// without null checks, suggesting they should be references instead. +/// +/// For the user-facing documentation see: +/// https://clang.llvm.org/extra/clang-tidy/checks/readability/pointer-to-ref.html +class PointerToRefCheck : public ClangTidyCheck { +public: + PointerToRefCheck(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::readability + +#endif // LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_READABILITY_POINTERTOREFCHECK_H diff --git a/clang-tools-extra/clang-tidy/readability/ReadabilityTidyModule.cpp b/clang-tools-extra/clang-tidy/readability/ReadabilityTidyModule.cpp index c582dc98eac6b..c0dff0822e4fc 100644 --- a/clang-tools-extra/clang-tidy/readability/ReadabilityTidyModule.cpp +++ b/clang-tools-extra/clang-tidy/readability/ReadabilityTidyModule.cpp @@ -39,6 +39,7 @@ #include "NamedParameterCheck.h" #include "NonConstParameterCheck.h" #include "OperatorsRepresentationCheck.h" +#include "PointerToRefCheck.h" #include "QualifiedAutoCheck.h" #include "RedundantAccessSpecifiersCheck.h" #include "RedundantCastingCheck.h" @@ -134,6 +135,8 @@ class ReadabilityModule : public ClangTidyModule { "readability-misplaced-array-index"); CheckFactories.registerCheck<OperatorsRepresentationCheck>( "readability-operators-representation"); + CheckFactories.registerCheck<PointerToRefCheck>( + "readability-pointer-to-ref"); CheckFactories.registerCheck<QualifiedAutoCheck>( "readability-qualified-auto"); CheckFactories.registerCheck<RedundantAccessSpecifiersCheck>( diff --git a/clang-tools-extra/docs/ReleaseNotes.rst b/clang-tools-extra/docs/ReleaseNotes.rst index 68bab88146241..78f482aa34f61 100644 --- a/clang-tools-extra/docs/ReleaseNotes.rst +++ b/clang-tools-extra/docs/ReleaseNotes.rst @@ -139,6 +139,13 @@ New checks Finds and removes redundant conversions from ``std::[w|u8|u16|u32]string_view`` to ``std::[...]string`` in call expressions expecting ``std::[...]string_view``. +- New :doc:`readability-pointer-to-ref + <clang-tidy/checks/readability/pointer-to-ref>` check. + + Finds function parameters that are pointers but are always + dereferenced without null checks, suggesting they should be + references instead. + - New :doc:`readability-trailing-comma <clang-tidy/checks/readability/trailing-comma>` check. diff --git a/clang-tools-extra/docs/clang-tidy/checks/list.rst b/clang-tools-extra/docs/clang-tidy/checks/list.rst index c475870ed7b31..7d01e5f0343ff 100644 --- a/clang-tools-extra/docs/clang-tidy/checks/list.rst +++ b/clang-tools-extra/docs/clang-tidy/checks/list.rst @@ -405,6 +405,7 @@ Clang-Tidy Checks :doc:`readability-named-parameter <readability/named-parameter>`, "Yes" :doc:`readability-non-const-parameter <readability/non-const-parameter>`, "Yes" :doc:`readability-operators-representation <readability/operators-representation>`, "Yes" + :doc:`readability-pointer-to-ref <readability/pointer-to-ref>`, :doc:`readability-qualified-auto <readability/qualified-auto>`, "Yes" :doc:`readability-redundant-access-specifiers <readability/redundant-access-specifiers>`, "Yes" :doc:`readability-redundant-casting <readability/redundant-casting>`, "Yes" diff --git a/clang-tools-extra/docs/clang-tidy/checks/readability/pointer-to-ref.rst b/clang-tools-extra/docs/clang-tidy/checks/readability/pointer-to-ref.rst new file mode 100644 index 0000000000000..aa6fd9589ac12 --- /dev/null +++ b/clang-tools-extra/docs/clang-tidy/checks/readability/pointer-to-ref.rst @@ -0,0 +1,43 @@ +.. title:: clang-tidy - readability-pointer-to-ref + +readability-pointer-to-ref +========================== + +Finds function parameters that are pointers but are always +dereferenced without null checks, suggesting they should be +references instead. + +Using references instead of pointers for non-nullable parameters +makes the contract explicit: the caller must provide a valid object. +This improves readability and can prevent null pointer bugs. + +.. code-block:: c++ + + // Before -- pointer that is never null-checked + void process(Foo *P) { + P->bar(); + P->X = 42; + } + + // After -- use a reference instead + void process(Foo &P) { + P.bar(); + P.X = 42; + } + +The check will not flag a parameter if it: + +- is checked for null (``if (P)``, ``P != nullptr``, etc.), +- is passed to another function as a pointer argument, +- is used in pointer arithmetic or array subscript, +- is a ``void`` pointer or pointer to an incomplete type, +- is a parameter of a virtual method, +- is a function pointer, +- is unnamed, or +- is never dereferenced in the function body. + +.. note:: + + This check does not provide fix-its because changing a + parameter from pointer to reference requires updating all + call sites, which may span multiple translation units. diff --git a/clang-tools-extra/test/clang-tidy/checkers/readability/pointer-to-ref.cpp b/clang-tools-extra/test/clang-tidy/checkers/readability/pointer-to-ref.cpp new file mode 100644 index 0000000000000..dd5604ec8c6eb --- /dev/null +++ b/clang-tools-extra/test/clang-tidy/checkers/readability/pointer-to-ref.cpp @@ -0,0 +1,86 @@ +// RUN: %check_clang_tidy %s readability-pointer-to-ref %t + +struct Foo { + int X; + void bar(); +}; + +// Positive: always dereferenced, never null-checked. +void derefArrow(Foo *P) { +// CHECK-MESSAGES: :[[@LINE-1]]:22: warning: pointer parameter 'P' can be a reference + P->bar(); + P->X = 42; +} + +// Positive: dereference via unary operator. +int derefStar(int *P) { +// CHECK-MESSAGES: :[[@LINE-1]]:20: warning: pointer parameter 'P' can be a reference + return *P + 1; +} + +// Positive: multiple parameters, only the dereferenced one flagged. +void twoParams(int *A, int *B) { +// CHECK-MESSAGES: :[[@LINE-1]]:21: warning: pointer parameter 'A' can be a reference + *A = 10; + if (B) + *B = 20; +} + +// Negative: null-checked before use. +void nullChecked(Foo *P) { + if (P) + P->bar(); +} + +// Negative: compared to nullptr. +void comparedToNull(Foo *P) { + if (P != nullptr) + P->bar(); +} + +// Negative: passed to another function (used as raw pointer). +void takePtr(Foo *); +void passedAlong(Foo *P) { + P->bar(); + takePtr(P); +} + +// Negative: used as array (subscript). +void asArray(int *P) { + P[0] = 1; + P[1] = 2; +} + +// Negative: void pointer (too generic). +void voidPtr(void *P) { +} + +// Negative: unnamed parameter. +void unnamed(int *) { +} + +// Negative: no dereference at all. +void noDereference(int *P) { +} + +// Negative: parameter reassigned. +void reassigned(int *P) { + int X = 0; + P = &X; + *P = 42; +} + +// Negative: virtual method (signature must match base). +struct Base { + virtual void vmethod(int *P); +}; + +// Negative: function pointer parameter. +void funcPtr(void (*P)(int)) { + P(42); +} + +// Negative: pointer arithmetic. +void arithmetic(int *P) { + *(P + 1) = 0; +} _______________________________________________ cfe-commits mailing list [email protected] https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
