https://github.com/hjanuschka updated https://github.com/llvm/llvm-project/pull/182068
>From a6fd34509b86b0a0e7c4d2ad64138b71f22563c6 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 | 222 ++++++++++++++++++ .../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 | 50 ++++ .../checkers/readability/pointer-to-ref.cpp | 177 ++++++++++++++ 8 files changed, 492 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..89633efab697a --- /dev/null +++ b/clang-tools-extra/clang-tidy/readability/PointerToRefCheck.cpp @@ -0,0 +1,222 @@ +//===----------------------------------------------------------------------===// +// +// 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: + PointerUsageVisitor(const ParmVarDecl *Param, ASTContext &Ctx) + : Param(Param), Ctx(Ctx) {} + + bool isDereferenced() const { return Dereferenced; } + bool isNullChecked() const { return NullChecked; } + bool isUsedAsPointer() const { return UsedAsPointer; } + + bool VisitUnaryOperator(const UnaryOperator *UO) { + const Expr *SubExpr = UO->getSubExpr(); + if (UO->getOpcode() == UO_Deref && refersToParam(SubExpr)) + Dereferenced = true; + if (UO->getOpcode() == UO_AddrOf && refersToParam(SubExpr)) + UsedAsPointer = 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()) { + bool LHSIsParam = refersToParam(LHS); + bool RHSIsParam = refersToParam(RHS); + if ((LHSIsParam && RHS->isNullPointerConstant( + Ctx, Expr::NPC_ValueDependentIsNotNull)) || + (RHSIsParam && LHS->isNullPointerConstant( + Ctx, Expr::NPC_ValueDependentIsNotNull))) + NullChecked = true; + // Comparing pointer to another non-null pointer (e.g. p == q). + else if (LHSIsParam || RHSIsParam) + UsedAsPointer = 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 delete expression: delete ptr. + bool VisitCXXDeleteExpr(const CXXDeleteExpr *DE) { + if (refersToParam(DE->getArgument())) + UsedAsPointer = true; + return true; + } + + // Detect return of pointer value. + bool VisitReturnStmt(const ReturnStmt *RS) { + if (RS->getRetValue() && refersToParam(RS->getRetValue())) + UsedAsPointer = true; + return true; + } + + // Detect pointer stored to a variable: int *q = ptr; + bool VisitVarDecl(const VarDecl *VD) { + if (VD != Param && VD->hasInit() && refersToParam(VD->getInit())) + UsedAsPointer = true; + return true; + } + + // Detect pointer passed to constructor: SomeClass obj(ptr); + bool VisitCXXConstructExpr(const CXXConstructExpr *CE) { + for (unsigned I = 0; I < CE->getNumArgs(); ++I) + if (refersToParam(CE->getArg(I))) + UsedAsPointer = true; + return true; + } + + // Detect pointer captured by lambda. + bool TraverseLambdaExpr(LambdaExpr *LE) { + for (const LambdaCapture &Capture : LE->captures()) { + if (Capture.capturesVariable() && + Capture.getCapturedVar() == Param) + UsedAsPointer = true; + } + // Don't descend into the lambda body -- it's a different scope. + return true; + } + + // Skip unevaluated contexts like sizeof. + bool TraverseUnaryExprOrTypeTraitExpr(UnaryExprOrTypeTraitExpr *E) { + // Do not descend into sizeof/alignof -- the pointer is not actually used. + return true; + } + +private: + bool refersToParam(const Expr *E) const { + if (!E) + return false; + const auto *DRE = dyn_cast<DeclRefExpr>(E->IgnoreParenImpCasts()); + return DRE && DRE->getDecl() == Param; + } + + const ParmVarDecl *Param; + ASTContext &Ctx; + bool Dereferenced = false; + bool NullChecked = false; + bool UsedAsPointer = false; +}; + +} // namespace + +void PointerToRefCheck::registerMatchers(MatchFinder *Finder) { + // Match pointer parameters in non-trivial function definitions, + // excluding void pointers and function pointers. + Finder->addMatcher( + parmVarDecl( + hasType(pointerType( + pointee(unless(voidType()), unless(functionType())))), + hasParent(functionDecl( + isDefinition(), unless(isImplicit()), unless(isDeleted()), + unless(isVariadic()), unless(isMain()), + unless(cxxMethodDecl(isVirtual())), + unless(cxxMethodDecl(isOverloadedOperator()))) + .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 unnamed parameters (can't be used). + if (!Param->getIdentifier()) + return; + + // Skip pointers to incomplete types (can't check at matcher level). + const auto *PT = Param->getType()->getAs<PointerType>(); + if (!PT || PT->getPointeeType()->isIncompleteType()) + return; + + // Skip extern "C" functions (C ABI compatibility). + if (Func->isExternC()) + return; + + // Analyze usage in the body. + PointerUsageVisitor Visitor(Param, *Result.Context); + 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..1d8e497c1de0b --- /dev/null +++ b/clang-tools-extra/docs/clang-tidy/checks/readability/pointer-to-ref.rst @@ -0,0 +1,50 @@ +.. 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, +- is in an ``extern "C"`` function, +- is used in a ``delete`` expression, +- is returned from the function, +- is stored to another variable, +- is passed to a constructor, +- is captured by a lambda, +- has its address taken (``&P``), +- is compared to another non-null pointer, 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..f5bd15c741e30 --- /dev/null +++ b/clang-tools-extra/test/clang-tidy/checkers/readability/pointer-to-ref.cpp @@ -0,0 +1,177 @@ +// 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; +} + +// Positive: dereference through parentheses. +int derefParen(int *P) { +// CHECK-MESSAGES: :[[@LINE-1]]:21: warning: pointer parameter 'P' can be a reference + return *(P) + 1; +} + +// Positive: (*P)->member pattern. +struct Bar { + int val; +}; +int derefThenArrow(Bar **P) { +// CHECK-MESSAGES: :[[@LINE-1]]:24: warning: pointer parameter 'P' can be a reference + return (*P)->val; +} + +// 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: compared using == nullptr. +void equalNull(Foo *P) { + if (P == nullptr) + return; + P->bar(); +} + +// Negative: negation null check (!P). +void negationCheck(Foo *P) { + if (!P) + return; + 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; +} + +// Negative: address-of usage. +void addressOf(int *P) { + int **PP = &P; + **PP = 42; +} + +// Negative: extern "C" linkage. +extern "C" void cLinkage(int *P) { + *P = 10; +} + +// Negative: delete expression. +void deletePtr(Foo *P) { + P->bar(); + delete P; +} + +// Negative: return pointer value. +int *returnPtr(int *P) { + *P = 10; + return P; +} + +// Negative: stored to another variable. +void storedToVar(int *P) { + int *Q = P; + *Q = 42; +} + +// Negative: passed to constructor. +struct Wrapper { + Wrapper(int *P); +}; +void passedToCtor(int *P) { + *P = 10; + Wrapper W(P); +} + +// Negative: used in sizeof (unevaluated context). +void sizeofUsage(int *P) { + (void)sizeof(*P); +} + +// Negative: pointer compared to another pointer (not null). +void comparedToOtherPtr(int *P, int *Q) { + if (P == Q) + return; + *P = 10; +} + +// Negative: used in lambda capture by value (passed along). +void lambdaCapture(int *P) { + *P = 10; + auto F = [P]() { return *P; }; + (void)F; +} + +// Negative: operator overload. +struct MyClass { + bool operator==(const MyClass *Other) const; +}; _______________________________________________ cfe-commits mailing list [email protected] https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
