https://github.com/zeyi2 updated https://github.com/llvm/llvm-project/pull/182543
>From 4ae63e8eea272b9059ea77429988ebc7e0ef26db Mon Sep 17 00:00:00 2001 From: mtx <[email protected]> Date: Tue, 3 Feb 2026 00:24:05 +0800 Subject: [PATCH 1/7] ~ --- .../bugprone/BugproneTidyModule.cpp | 3 + .../clang-tidy/bugprone/CMakeLists.txt | 1 + .../bugprone/MissingEndComparisonCheck.cpp | 107 ++++++++++++++++++ .../bugprone/MissingEndComparisonCheck.h | 34 ++++++ clang-tools-extra/docs/ReleaseNotes.rst | 5 + .../bugprone/missing-end-comparison.rst | 6 + .../docs/clang-tidy/checks/list.rst | 1 + .../bugprone/missing-end-comparison.cpp | 94 +++++++++++++++ 8 files changed, 251 insertions(+) create mode 100644 clang-tools-extra/clang-tidy/bugprone/MissingEndComparisonCheck.cpp create mode 100644 clang-tools-extra/clang-tidy/bugprone/MissingEndComparisonCheck.h create mode 100644 clang-tools-extra/docs/clang-tidy/checks/bugprone/missing-end-comparison.rst create mode 100644 clang-tools-extra/test/clang-tidy/checkers/bugprone/missing-end-comparison.cpp diff --git a/clang-tools-extra/clang-tidy/bugprone/BugproneTidyModule.cpp b/clang-tools-extra/clang-tidy/bugprone/BugproneTidyModule.cpp index 4150442c25d61..8ddc7f5ce8306 100644 --- a/clang-tools-extra/clang-tidy/bugprone/BugproneTidyModule.cpp +++ b/clang-tools-extra/clang-tidy/bugprone/BugproneTidyModule.cpp @@ -51,6 +51,7 @@ #include "MisplacedOperatorInStrlenInAllocCheck.h" #include "MisplacedPointerArithmeticInAllocCheck.h" #include "MisplacedWideningCastCheck.h" +#include "MissingEndComparisonCheck.h" #include "MoveForwardingReferenceCheck.h" #include "MultiLevelImplicitPointerConversionCheck.h" #include "MultipleNewInOneExpressionCheck.h" @@ -176,6 +177,8 @@ class BugproneModule : public ClangTidyModule { "bugprone-incorrect-enable-if"); CheckFactories.registerCheck<IncorrectEnableSharedFromThisCheck>( "bugprone-incorrect-enable-shared-from-this"); + CheckFactories.registerCheck<MissingEndComparisonCheck>( + "bugprone-missing-end-comparison"); CheckFactories.registerCheck<UnintendedCharOstreamOutputCheck>( "bugprone-unintended-char-ostream-output"); CheckFactories.registerCheck<ReturnConstRefFromParameterCheck>( diff --git a/clang-tools-extra/clang-tidy/bugprone/CMakeLists.txt b/clang-tools-extra/clang-tidy/bugprone/CMakeLists.txt index db1256d91d311..66f94c20c3a06 100644 --- a/clang-tools-extra/clang-tidy/bugprone/CMakeLists.txt +++ b/clang-tools-extra/clang-tidy/bugprone/CMakeLists.txt @@ -37,6 +37,7 @@ add_clang_library(clangTidyBugproneModule STATIC IncorrectEnableIfCheck.cpp IncorrectEnableSharedFromThisCheck.cpp InvalidEnumDefaultInitializationCheck.cpp + MissingEndComparisonCheck.cpp UnintendedCharOstreamOutputCheck.cpp ReturnConstRefFromParameterCheck.cpp SuspiciousStringviewDataUsageCheck.cpp diff --git a/clang-tools-extra/clang-tidy/bugprone/MissingEndComparisonCheck.cpp b/clang-tools-extra/clang-tidy/bugprone/MissingEndComparisonCheck.cpp new file mode 100644 index 0000000000000..e7071d94f0ee6 --- /dev/null +++ b/clang-tools-extra/clang-tidy/bugprone/MissingEndComparisonCheck.cpp @@ -0,0 +1,107 @@ +//===--- MissingEndComparisonCheck.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 "MissingEndComparisonCheck.h" +#include "clang/AST/ASTContext.h" +#include "clang/ASTMatchers/ASTMatchFinder.h" +#include "clang/Lex/Lexer.h" + +using namespace clang::ast_matchers; + +namespace clang::tidy::bugprone { + +void MissingEndComparisonCheck::registerMatchers(MatchFinder *Finder) { + // List of standard algorithms that return an iterator and should be compared + // to the end iterator. + // Note: Algorithms returning pairs (like equal_range, mismatch) are excluded + // because std::pair doesn't implicitly convert to bool, so they wouldn't + // match CK_PointerToBoolean anyway. + auto StandardIteratorAlgorithms = functionDecl(hasAnyName( + "::std::find", "::std::find_if", "::std::find_if_not", "::std::search", + "::std::search_n", "::std::find_end", "::std::find_first_of", + "::std::lower_bound", "::std::upper_bound", "::std::partition_point", + "::std::min_element", "::std::max_element", "::std::adjacent_find", + "::std::is_sorted_until")); + + // Matcher 1: Implicit cast from pointer to boolean. + // This catches cases where the algorithm returns a raw pointer (e.g., + // finding in a C-array) and it's used in a boolean context. + Finder->addMatcher( + implicitCastExpr( + hasCastKind(CK_PointerToBoolean), + hasSourceExpression(ignoringParenImpCasts( + callExpr(callee(StandardIteratorAlgorithms)).bind("call")))) + .bind("cast"), + this); + + // Matcher 2: Explicit/Implicit conversion via operator bool. + // This catches cases where the returned iterator has an explicit or implicit + // conversion to bool (e.g., some custom iterators). + Finder->addMatcher( + cxxMemberCallExpr( + callee(cxxConversionDecl(returns(booleanType()))), + on(ignoringParenImpCasts( + callExpr(callee(StandardIteratorAlgorithms)).bind("call")))) + .bind("cast"), + this); +} + +void MissingEndComparisonCheck::check(const MatchFinder::MatchResult &Result) { + const auto *Call = Result.Nodes.getNodeAs<CallExpr>("call"); + const auto *Cast = Result.Nodes.getNodeAs<Expr>("cast"); + + if (!Call || !Cast) + return; + + // Most standard algorithms take the end iterator as the second argument. + // Check if we have enough arguments. + if (Call->getNumArgs() < 2) + return; + + const Expr *EndIterArg = Call->getArg(1); + + // If the second argument is nullptr/NULL, the user might be intentionally + // checking against nullptr (though odd for std algorithms, it's possible + // for raw pointers). + if (EndIterArg->isNullPointerConstant(*Result.Context, + Expr::NPC_ValueDependentIsNull)) { + return; + } + + auto Diag = diag(Cast->getBeginLoc(), + "result of standard algorithm used in boolean context; did " + "you mean to compare with the end iterator?"); + + // Try to generate a fix-it. + // We want to rewrite the expression 'E' to 'E != EndIter'. + // 'Cast' is the boolean expression (e.g. the implicit cast or the bool conversion call). + // However, simply appending '!= End' to the end of the Cast's range might be tricky + // if there are precedence issues, but usually != has lower precedence than function calls + // and higher than assignment/logic. + + // Get the source text of the end iterator argument. + StringRef EndIterText = Lexer::getSourceText( + CharSourceRange::getTokenRange(EndIterArg->getSourceRange()), + *Result.SourceManager, Result.Context->getLangOpts()); + + if (EndIterText.empty()) + return; + + // Check if the end iterator expression is safe to duplicate. + // (Simple variable, member access, call to .end()). + // For now, we'll be conservative. If it looks complex, skip the fix-it. + // A simple heuristic: if it contains side effects or is too long, skip. + // But we can just provide the hint. + + Diag << FixItHint::CreateInsertion( + Lexer::getLocForEndOfToken(Cast->getEndLoc(), 0, *Result.SourceManager, + Result.Context->getLangOpts()), + (" != " + EndIterText).str()); +} + +} // namespace clang::tidy::bugprone \ No newline at end of file diff --git a/clang-tools-extra/clang-tidy/bugprone/MissingEndComparisonCheck.h b/clang-tools-extra/clang-tidy/bugprone/MissingEndComparisonCheck.h new file mode 100644 index 0000000000000..75a2afab5a3a9 --- /dev/null +++ b/clang-tools-extra/clang-tidy/bugprone/MissingEndComparisonCheck.h @@ -0,0 +1,34 @@ +//===----------------------------------------------------------------------===// +// +// 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_BUGPRONE_MISSINGENDCOMPARISONCHECK_H +#define LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_BUGPRONE_MISSINGENDCOMPARISONCHECK_H + +#include "../ClangTidyCheck.h" + +namespace clang::tidy::bugprone { + +/// Detects usage of the result of standard algorithms (like std::find) in a +/// boolean context without comparing it with the end iterator. +/// +/// For the user-facing documentation see: +/// https://clang.llvm.org/extra/clang-tidy/checks/bugprone/missing-end-comparison.html +class MissingEndComparisonCheck : public ClangTidyCheck { +public: + MissingEndComparisonCheck(StringRef Name, ClangTidyContext *Context) + : ClangTidyCheck(Name, Context) {} + void registerMatchers(ast_matchers::MatchFinder *Finder) override; + void check(const ast_matchers::MatchFinder::MatchResult &Result) override; + bool isLanguageVersionSupported(const LangOptions &LangOpts) const override { + return LangOpts.CPlusPlus; + } +}; + +} // namespace clang::tidy::bugprone + +#endif // LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_BUGPRONE_MISSINGENDCOMPARISONCHECK_H diff --git a/clang-tools-extra/docs/ReleaseNotes.rst b/clang-tools-extra/docs/ReleaseNotes.rst index 754880bd1a381..fbe0a4d99cdad 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:`bugprone-missing-end-comparison + <clang-tidy/checks/bugprone/missing-end-comparison>` check. + + FIXME: Write a short description. + - New :doc:`llvm-use-vector-utils <clang-tidy/checks/llvm/use-vector-utils>` check. diff --git a/clang-tools-extra/docs/clang-tidy/checks/bugprone/missing-end-comparison.rst b/clang-tools-extra/docs/clang-tidy/checks/bugprone/missing-end-comparison.rst new file mode 100644 index 0000000000000..90894dce48786 --- /dev/null +++ b/clang-tools-extra/docs/clang-tidy/checks/bugprone/missing-end-comparison.rst @@ -0,0 +1,6 @@ +.. title:: clang-tidy - bugprone-missing-end-comparison + +bugprone-missing-end-comparison +=============================== + +FIXME: Describe what patterns does the check detect and why. Give examples. diff --git a/clang-tools-extra/docs/clang-tidy/checks/list.rst b/clang-tools-extra/docs/clang-tidy/checks/list.rst index 25d1354fc4c20..8d3470340a248 100644 --- a/clang-tools-extra/docs/clang-tidy/checks/list.rst +++ b/clang-tools-extra/docs/clang-tidy/checks/list.rst @@ -121,6 +121,7 @@ Clang-Tidy Checks :doc:`bugprone-misplaced-operator-in-strlen-in-alloc <bugprone/misplaced-operator-in-strlen-in-alloc>`, "Yes" :doc:`bugprone-misplaced-pointer-arithmetic-in-alloc <bugprone/misplaced-pointer-arithmetic-in-alloc>`, "Yes" :doc:`bugprone-misplaced-widening-cast <bugprone/misplaced-widening-cast>`, + :doc:`bugprone-missing-end-comparison <bugprone/missing-end-comparison>`, "Yes" :doc:`bugprone-move-forwarding-reference <bugprone/move-forwarding-reference>`, "Yes" :doc:`bugprone-multi-level-implicit-pointer-conversion <bugprone/multi-level-implicit-pointer-conversion>`, :doc:`bugprone-multiple-new-in-one-expression <bugprone/multiple-new-in-one-expression>`, diff --git a/clang-tools-extra/test/clang-tidy/checkers/bugprone/missing-end-comparison.cpp b/clang-tools-extra/test/clang-tidy/checkers/bugprone/missing-end-comparison.cpp new file mode 100644 index 0000000000000..1632a04c3ab15 --- /dev/null +++ b/clang-tools-extra/test/clang-tidy/checkers/bugprone/missing-end-comparison.cpp @@ -0,0 +1,94 @@ +// RUN: %check_clang_tidy %s bugprone-missing-end-comparison %t -- -- -std=c++17 + +namespace std { + template<typename T> struct iterator_traits; + struct forward_iterator_tag {}; + + typedef long int ptrdiff_t; + typedef decltype(nullptr) nullptr_t; + + template<typename T> + struct vector { + typedef T* iterator; + typedef const T* const_iterator; + iterator begin(); + iterator end(); + const_iterator begin() const; + const_iterator end() const; + }; + + template<class InputIt, class T> + InputIt find(InputIt first, InputIt last, const T& value); + + template<class ForwardIt, class T> + ForwardIt lower_bound(ForwardIt first, ForwardIt last, const T& value); + + template<class ForwardIt, class ForwardIt2> + ForwardIt search(ForwardIt first, ForwardIt last, ForwardIt first2, ForwardIt2 last2); + + template<class ForwardIt> + ForwardIt min_element(ForwardIt first, ForwardIt last); +} + +struct CustomIterator { + int* ptr; + using iterator_category = std::forward_iterator_tag; + using value_type = int; + using difference_type = long; + using pointer = int*; + using reference = int&; + + int& operator*() const { return *ptr; } + CustomIterator& operator++() { ++ptr; return *this; } + bool operator==(const CustomIterator& other) const { return ptr == other.ptr; } + bool operator!=(const CustomIterator& other) const { return ptr != other.ptr; } + + explicit operator bool() const { return ptr != nullptr; } +}; + +void test_raw_pointers() { + int arr[] = {1, 2, 3}; + int* begin = arr; + int* end = arr + 3; + + if (std::find(begin, end, 2)) {} + // CHECK-MESSAGES: :[[@LINE-1]]:7: warning: result of standard algorithm used in boolean context; did you mean to compare with the end iterator? [bugprone-missing-end-comparison] + // CHECK-FIXES: if (std::find(begin, end, 2) != end) {} + + while (std::lower_bound(begin, end, 2)) { break; } + // CHECK-MESSAGES: :[[@LINE-1]]:10: warning: result of standard algorithm used in boolean context + // CHECK-FIXES: while (std::lower_bound(begin, end, 2) != end) { break; } + + if (std::find(begin, end, 2) != end) {} +} + +void test_vector() { + std::vector<int> v; + if (std::find(v.begin(), v.end(), 2)) {} + // CHECK-MESSAGES: :[[@LINE-1]]:7: warning: result of standard algorithm used in boolean context + // CHECK-FIXES: if (std::find(v.begin(), v.end(), 2) != v.end()) {} + + if (std::min_element(v.begin(), v.end())) {} + // CHECK-MESSAGES: :[[@LINE-1]]:7: warning: result of standard algorithm used in boolean context + // CHECK-FIXES: if (std::min_element(v.begin(), v.end()) != v.end()) {} +} + +void test_custom_iterator() { + CustomIterator begin{nullptr}, end{nullptr}; + if (std::find(begin, end, 2)) {} + // CHECK-MESSAGES: :[[@LINE-1]]:7: warning: result of standard algorithm used in boolean context + // CHECK-FIXES: if (std::find(begin, end, 2) != end) {} +} + +void test_complex_end() { + int arr[] = {1, 2, 3}; + if (std::find(arr, arr + 3, 2)) {} + // CHECK-MESSAGES: :[[@LINE-1]]:7: warning: result of standard algorithm used in boolean context + // CHECK-FIXES: if (std::find(arr, arr + 3, 2) != arr + 3) {} +} + +void test_sentinel() { + int* ptr = nullptr; + if (std::find<int*>(ptr, nullptr, 10)) {} + // No warning expected for nullptr sentinel +} \ No newline at end of file >From a279a89481a7ffedd4977f3b14e8353ec35ed39a Mon Sep 17 00:00:00 2001 From: mtx <[email protected]> Date: Fri, 20 Feb 2026 21:08:43 +0800 Subject: [PATCH 2/7] ~~ --- .../bugprone/MissingEndComparisonCheck.cpp | 220 ++++++++++++------ clang-tools-extra/docs/ReleaseNotes.rst | 3 +- .../bugprone/missing-end-comparison.rst | 65 +++++- .../bugprone/missing-end-comparison.cpp | 142 +++++++++-- 4 files changed, 332 insertions(+), 98 deletions(-) diff --git a/clang-tools-extra/clang-tidy/bugprone/MissingEndComparisonCheck.cpp b/clang-tools-extra/clang-tidy/bugprone/MissingEndComparisonCheck.cpp index e7071d94f0ee6..42ff99b121f84 100644 --- a/clang-tools-extra/clang-tidy/bugprone/MissingEndComparisonCheck.cpp +++ b/clang-tools-extra/clang-tidy/bugprone/MissingEndComparisonCheck.cpp @@ -1,4 +1,4 @@ -//===--- MissingEndComparisonCheck.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. @@ -10,98 +10,166 @@ #include "clang/AST/ASTContext.h" #include "clang/ASTMatchers/ASTMatchFinder.h" #include "clang/Lex/Lexer.h" +#include "clang/Tooling/FixIt.h" using namespace clang::ast_matchers; namespace clang::tidy::bugprone { +namespace { + +constexpr llvm::StringRef IteratorAlgorithms[] = { + "::std::find", "::std::find_if", + "::std::find_if_not", "::std::search", + "::std::search_n", "::std::find_end", + "::std::find_first_of", "::std::lower_bound", + "::std::upper_bound", "::std::partition_point", + "::std::min_element", "::std::max_element", + "::std::adjacent_find", "::std::is_sorted_until"}; + +constexpr llvm::StringRef RangeAlgorithms[] = { + "::std::ranges::find", "::std::ranges::find_if", + "::std::ranges::find_if_not", "::std::ranges::lower_bound", + "::std::ranges::upper_bound", "::std::ranges::min_element", + "::std::ranges::max_element"}; + +} // namespace + void MissingEndComparisonCheck::registerMatchers(MatchFinder *Finder) { - // List of standard algorithms that return an iterator and should be compared - // to the end iterator. - // Note: Algorithms returning pairs (like equal_range, mismatch) are excluded - // because std::pair doesn't implicitly convert to bool, so they wouldn't - // match CK_PointerToBoolean anyway. - auto StandardIteratorAlgorithms = functionDecl(hasAnyName( - "::std::find", "::std::find_if", "::std::find_if_not", "::std::search", - "::std::search_n", "::std::find_end", "::std::find_first_of", - "::std::lower_bound", "::std::upper_bound", "::std::partition_point", - "::std::min_element", "::std::max_element", "::std::adjacent_find", - "::std::is_sorted_until")); - - // Matcher 1: Implicit cast from pointer to boolean. - // This catches cases where the algorithm returns a raw pointer (e.g., - // finding in a C-array) and it's used in a boolean context. - Finder->addMatcher( - implicitCastExpr( - hasCastKind(CK_PointerToBoolean), - hasSourceExpression(ignoringParenImpCasts( - callExpr(callee(StandardIteratorAlgorithms)).bind("call")))) - .bind("cast"), - this); - - // Matcher 2: Explicit/Implicit conversion via operator bool. - // This catches cases where the returned iterator has an explicit or implicit - // conversion to bool (e.g., some custom iterators). + const auto StdAlgoCall = callExpr( + callee(functionDecl(hasAnyName(IteratorAlgorithms), isInStdNamespace()))); + + const auto RangesCall = cxxOperatorCallExpr( + hasOverloadedOperatorName("()"), + hasArgument(0, declRefExpr(to( + varDecl(hasAnyName(RangeAlgorithms)).bind("cpo"))))); + + const auto AnyAlgoCall = + getLangOpts().CPlusPlus20 + ? expr(anyOf(StdAlgoCall, RangesCall)).bind("algoCall") + : expr(StdAlgoCall).bind("algoCall"); + + // Captures implicit pointer-to-bool casts and operator bool() calls. + const auto IsBoolUsage = anyOf( + implicitCastExpr(hasCastKind(CK_PointerToBoolean), + hasSourceExpression(ignoringParenImpCasts(AnyAlgoCall))), + cxxMemberCallExpr(callee(cxxConversionDecl(returns(booleanType()))), + on(ignoringParenImpCasts(AnyAlgoCall)))); + + // Captures variable usage: `auto it = std::find(...); if (it)` + // FIXME: This only handles variables initialized directly by the algorithm. + // We may need to introduce more accurate dataflow analysis in the future. + const auto VarWithAlgoInit = + varDecl(hasInitializer(ignoringParenImpCasts(AnyAlgoCall))); + + const auto IsVariableBoolUsage = + anyOf(implicitCastExpr(hasCastKind(CK_PointerToBoolean), + hasSourceExpression(ignoringParenImpCasts( + declRefExpr(to(VarWithAlgoInit))))), + cxxMemberCallExpr( + callee(cxxConversionDecl(returns(booleanType()))), + on(ignoringParenImpCasts(declRefExpr(to(VarWithAlgoInit)))))); + Finder->addMatcher( - cxxMemberCallExpr( - callee(cxxConversionDecl(returns(booleanType()))), - on(ignoringParenImpCasts( - callExpr(callee(StandardIteratorAlgorithms)).bind("call")))) - .bind("cast"), - this); + expr(anyOf(IsBoolUsage, IsVariableBoolUsage)).bind("boolOp"), this); } void MissingEndComparisonCheck::check(const MatchFinder::MatchResult &Result) { - const auto *Call = Result.Nodes.getNodeAs<CallExpr>("call"); - const auto *Cast = Result.Nodes.getNodeAs<Expr>("cast"); + const auto *Call = Result.Nodes.getNodeAs<CallExpr>("algoCall"); + const auto *BoolOp = Result.Nodes.getNodeAs<Expr>("boolOp"); + const auto *CPO = Result.Nodes.getNodeAs<VarDecl>("cpo"); - if (!Call || !Cast) + if (!Call || !BoolOp) return; - // Most standard algorithms take the end iterator as the second argument. - // Check if we have enough arguments. - if (Call->getNumArgs() < 2) - return; + std::string EndExprText; + + if (!CPO) { + if (Call->getNumArgs() < 2) + return; - const Expr *EndIterArg = Call->getArg(1); - - // If the second argument is nullptr/NULL, the user might be intentionally - // checking against nullptr (though odd for std algorithms, it's possible - // for raw pointers). - if (EndIterArg->isNullPointerConstant(*Result.Context, - Expr::NPC_ValueDependentIsNull)) { + const Expr *EndArg = Call->getArg(1); + // Filters nullptr, we assume the intent might be a valid check against null + if (EndArg->IgnoreParenCasts()->isNullPointerConstant( + *Result.Context, Expr::NPC_ValueDependentIsNull)) return; + + EndExprText = tooling::fixit::getText(*EndArg, *Result.Context).str(); + } else { + const FunctionDecl *Callee = Call->getDirectCallee(); + if (!Callee || Callee->getNumParams() == 0) + return; + + // Range overloads take a reference (R&&), Iterator overloads pass by value. + const bool IsIterPair = + !Callee->getParamDecl(0)->getType()->isReferenceType(); + + if (IsIterPair) { + if (Call->getNumArgs() < 3) + return; + // find(CPO, Iter, Sent, Val...) -> Sent is Arg 2. + const Expr *EndArg = Call->getArg(2); + EndExprText = tooling::fixit::getText(*EndArg, *Result.Context).str(); + } else { + if (Call->getNumArgs() < 2) + return; + // find(CPO, Range, Val, Proj) -> Range is Arg 1. + const Expr *RangeArg = Call->getArg(1); + // Avoid potential side-effects + const Expr *InnerRange = RangeArg->IgnoreParenImpCasts(); + if (isa<DeclRefExpr>(InnerRange) || isa<MemberExpr>(InnerRange)) { + const StringRef RangeText = + tooling::fixit::getText(*RangeArg, *Result.Context); + if (!RangeText.empty()) + EndExprText = ("std::ranges::end(" + RangeText + ")").str(); + } + } } - auto Diag = diag(Cast->getBeginLoc(), - "result of standard algorithm used in boolean context; did " - "you mean to compare with the end iterator?"); - - // Try to generate a fix-it. - // We want to rewrite the expression 'E' to 'E != EndIter'. - // 'Cast' is the boolean expression (e.g. the implicit cast or the bool conversion call). - // However, simply appending '!= End' to the end of the Cast's range might be tricky - // if there are precedence issues, but usually != has lower precedence than function calls - // and higher than assignment/logic. - - // Get the source text of the end iterator argument. - StringRef EndIterText = Lexer::getSourceText( - CharSourceRange::getTokenRange(EndIterArg->getSourceRange()), - *Result.SourceManager, Result.Context->getLangOpts()); - - if (EndIterText.empty()) - return; + bool IsNegated = false; + const UnaryOperator *NotOp = nullptr; + const Expr *CurrentExpr = BoolOp; + while (true) { + auto Parents = Result.Context->getParents(*CurrentExpr); + if (Parents.empty()) + break; + if (const auto *P = Parents[0].get<ParenExpr>()) { + CurrentExpr = P; + continue; + } + if (const auto *U = Parents[0].get<UnaryOperator>()) { + if (U->getOpcode() == UO_LNot) { + NotOp = U; + IsNegated = true; + } + } + break; + } - // Check if the end iterator expression is safe to duplicate. - // (Simple variable, member access, call to .end()). - // For now, we'll be conservative. If it looks complex, skip the fix-it. - // A simple heuristic: if it contains side effects or is too long, skip. - // But we can just provide the hint. - - Diag << FixItHint::CreateInsertion( - Lexer::getLocForEndOfToken(Cast->getEndLoc(), 0, *Result.SourceManager, - Result.Context->getLangOpts()), - (" != " + EndIterText).str()); + const auto Diag = + diag(BoolOp->getBeginLoc(), + "result of standard algorithm used in boolean context; did " + "you mean to compare with the end iterator?"); + + if (!EndExprText.empty()) { + if (IsNegated) { + // !it -> (it == end) + Diag << FixItHint::CreateReplacement(NotOp->getOperatorLoc(), "("); + Diag << FixItHint::CreateInsertion( + Lexer::getLocForEndOfToken(BoolOp->getEndLoc(), 0, + *Result.SourceManager, + Result.Context->getLangOpts()), + " == " + EndExprText + ")"); + } else { + // it -> (it != end) + Diag << FixItHint::CreateInsertion(BoolOp->getBeginLoc(), "("); + Diag << FixItHint::CreateInsertion( + Lexer::getLocForEndOfToken(BoolOp->getEndLoc(), 0, + *Result.SourceManager, + Result.Context->getLangOpts()), + " != " + EndExprText + ")"); + } + } } -} // namespace clang::tidy::bugprone \ No newline at end of file +} // namespace clang::tidy::bugprone diff --git a/clang-tools-extra/docs/ReleaseNotes.rst b/clang-tools-extra/docs/ReleaseNotes.rst index 1d4c581fc2ddf..b89e2c0c60bf3 100644 --- a/clang-tools-extra/docs/ReleaseNotes.rst +++ b/clang-tools-extra/docs/ReleaseNotes.rst @@ -105,7 +105,8 @@ New checks - New :doc:`bugprone-missing-end-comparison <clang-tidy/checks/bugprone/missing-end-comparison>` check. - FIXME: Write a short description. + Finds instances where the result of a standard algorithm is used in a boolean + context without being compared to the end iterator. - New :doc:`bugprone-unsafe-to-allow-exceptions <clang-tidy/checks/bugprone/unsafe-to-allow-exceptions>` check. diff --git a/clang-tools-extra/docs/clang-tidy/checks/bugprone/missing-end-comparison.rst b/clang-tools-extra/docs/clang-tidy/checks/bugprone/missing-end-comparison.rst index 90894dce48786..4f1ac0d7657ab 100644 --- a/clang-tools-extra/docs/clang-tidy/checks/bugprone/missing-end-comparison.rst +++ b/clang-tools-extra/docs/clang-tidy/checks/bugprone/missing-end-comparison.rst @@ -3,4 +3,67 @@ bugprone-missing-end-comparison =============================== -FIXME: Describe what patterns does the check detect and why. Give examples. +Finds instances where the result of a standard algorithm is used in a boolean +context without being compared to the end iterator. + +Standard algorithms such as `std::find`, `std::search`, and `std::lower_bound` +return an iterator to the element if found, or the end iterator otherwise. + +Using the result directly in a boolean context (like an `if` statement) is +almost always a bug, as it only checks if the iterator itself evaluates to +`true`, which may always be true for many iterator types (including pointers). + +Examples: + +.. code-block:: c++ + + int arr[] = {1, 2, 3}; + int* begin = std::begin(arr); + int* end = std::end(arr); + + // Problematic: + if (std::find(begin, end, 2)) { + // ... + } + + // Fixed by the check: + if ((std::find(begin, end, 2) != end)) { + // ... + } + + // C++20 ranges: + std::vector<int> v = {1, 2, 3}; + if (std::ranges::find(v, 2)) { // Problematic + // ... + } + + // Fixed by the check: + if ((std::ranges::find(v, 2) != std::ranges::end(v))) { + // ... + } + +The check also handles range-based algorithms introduced in C++20. + +Supported algorithms: + +- `std::find` +- `std::find_if` +- `std::find_if_not` +- `std::search` +- `std::search_n` +- `std::find_end` +- `std::find_first_of` +- `std::lower_bound` +- `std::upper_bound` +- `std::partition_point` +- `std::min_element` +- `std::max_element` +- `std::adjacent_find` +- `std::is_sorted_until` +- `std::ranges::find` +- `std::ranges::find_if` +- `std::ranges::find_if_not` +- `std::ranges::lower_bound` +- `std::ranges::upper_bound` +- `std::ranges::min_element` +- `std::ranges::max_element` diff --git a/clang-tools-extra/test/clang-tidy/checkers/bugprone/missing-end-comparison.cpp b/clang-tools-extra/test/clang-tidy/checkers/bugprone/missing-end-comparison.cpp index 1632a04c3ab15..6bbeef5cbde6f 100644 --- a/clang-tools-extra/test/clang-tidy/checkers/bugprone/missing-end-comparison.cpp +++ b/clang-tools-extra/test/clang-tidy/checkers/bugprone/missing-end-comparison.cpp @@ -1,9 +1,9 @@ -// RUN: %check_clang_tidy %s bugprone-missing-end-comparison %t -- -- -std=c++17 +// RUN: %check_clang_tidy -std=c++20 %s bugprone-missing-end-comparison %t namespace std { template<typename T> struct iterator_traits; struct forward_iterator_tag {}; - + typedef long int ptrdiff_t; typedef decltype(nullptr) nullptr_t; @@ -28,6 +28,28 @@ namespace std { template<class ForwardIt> ForwardIt min_element(ForwardIt first, ForwardIt last); + + template<class InputIt1, class InputIt2> + struct pair { + InputIt1 first; + InputIt2 second; + }; + + namespace ranges { + template<typename T> + void* begin(T& t); + template<typename T> + void* end(T& t); + + struct FindFn { + template<typename Range, typename T> + void* operator()(Range&& r, const T& value) const; + + template<typename I, typename S, typename T> + void* operator()(I first, S last, const T& value) const; + }; + inline constexpr FindFn find; + } } struct CustomIterator { @@ -42,7 +64,7 @@ struct CustomIterator { CustomIterator& operator++() { ++ptr; return *this; } bool operator==(const CustomIterator& other) const { return ptr == other.ptr; } bool operator!=(const CustomIterator& other) const { return ptr != other.ptr; } - + explicit operator bool() const { return ptr != nullptr; } }; @@ -53,42 +75,122 @@ void test_raw_pointers() { if (std::find(begin, end, 2)) {} // CHECK-MESSAGES: :[[@LINE-1]]:7: warning: result of standard algorithm used in boolean context; did you mean to compare with the end iterator? [bugprone-missing-end-comparison] - // CHECK-FIXES: if (std::find(begin, end, 2) != end) {} + // CHECK-FIXES: if ((std::find(begin, end, 2) != end)) {} while (std::lower_bound(begin, end, 2)) { break; } - // CHECK-MESSAGES: :[[@LINE-1]]:10: warning: result of standard algorithm used in boolean context - // CHECK-FIXES: while (std::lower_bound(begin, end, 2) != end) { break; } + // CHECK-MESSAGES: :[[@LINE-1]]:10: warning: result of standard algorithm used in boolean context; did you mean to compare with the end iterator? [bugprone-missing-end-comparison] + // CHECK-FIXES: while ((std::lower_bound(begin, end, 2) != end)) { break; } - if (std::find(begin, end, 2) != end) {} + if (!std::find(begin, end, 2)) {} + // CHECK-MESSAGES: :[[@LINE-1]]:8: warning: result of standard algorithm used in boolean context; did you mean to compare with the end iterator? [bugprone-missing-end-comparison] + // CHECK-FIXES: if ((std::find(begin, end, 2) == end)) {} } void test_vector() { std::vector<int> v; if (std::find(v.begin(), v.end(), 2)) {} - // CHECK-MESSAGES: :[[@LINE-1]]:7: warning: result of standard algorithm used in boolean context - // CHECK-FIXES: if (std::find(v.begin(), v.end(), 2) != v.end()) {} + // CHECK-MESSAGES: :[[@LINE-1]]:7: warning: result of standard algorithm used in boolean context; did you mean to compare with the end iterator? [bugprone-missing-end-comparison] + // CHECK-FIXES: if ((std::find(v.begin(), v.end(), 2) != v.end())) {} if (std::min_element(v.begin(), v.end())) {} - // CHECK-MESSAGES: :[[@LINE-1]]:7: warning: result of standard algorithm used in boolean context - // CHECK-FIXES: if (std::min_element(v.begin(), v.end()) != v.end()) {} + // CHECK-MESSAGES: :[[@LINE-1]]:7: warning: result of standard algorithm used in boolean context; did you mean to compare with the end iterator? [bugprone-missing-end-comparison] + // CHECK-FIXES: if ((std::min_element(v.begin(), v.end()) != v.end())) {} +} + +void test_variable_tracking() { + int arr[] = {1, 2, 3}; + auto it = std::find(arr, arr + 3, 2); + if (it) {} + // CHECK-MESSAGES: :[[@LINE-1]]:7: warning: result of standard algorithm used in boolean context; did you mean to compare with the end iterator? [bugprone-missing-end-comparison] + // CHECK-FIXES: if ((it != arr + 3)) {} + + if (!it) {} + // CHECK-MESSAGES: :[[@LINE-1]]:8: warning: result of standard algorithm used in boolean context; did you mean to compare with the end iterator? [bugprone-missing-end-comparison] + // CHECK-FIXES: if ((it == arr + 3)) {} +} + +void test_ranges() { + std::vector<int> v; + if (std::ranges::find(v, 2)) {} + // CHECK-MESSAGES: :[[@LINE-1]]:7: warning: result of standard algorithm used in boolean context; did you mean to compare with the end iterator? [bugprone-missing-end-comparison] + // CHECK-FIXES: if ((std::ranges::find(v, 2) != std::ranges::end(v))) {} + + auto it = std::ranges::find(v, 2); + if (it) {} + // CHECK-MESSAGES: :[[@LINE-1]]:7: warning: result of standard algorithm used in boolean context; did you mean to compare with the end iterator? [bugprone-missing-end-comparison] + // CHECK-FIXES: if ((it != std::ranges::end(v))) {} +} + +void test_ranges_iterator_pair() { + int arr[] = {1, 2, 3}; + int *begin = arr; + int *end = arr + 3; + if (std::ranges::find(begin, end, 2)) {} + // CHECK-MESSAGES: :[[@LINE-1]]:7: warning: result of standard algorithm used in boolean context; did you mean to compare with the end iterator? [bugprone-missing-end-comparison] + // CHECK-FIXES: if ((std::ranges::find(begin, end, 2) != end)) {} +} + +void test_side_effects() { + std::vector<int> get_vec(); + if (std::ranges::find(get_vec(), 2)) {} + // CHECK-MESSAGES: :[[@LINE-1]]:7: warning: result of standard algorithm used in boolean context; did you mean to compare with the end iterator? [bugprone-missing-end-comparison] +} + +void test_negative() { + std::vector<int> v; + if (std::find(v.begin(), v.end(), 2) != v.end()) {} + if (std::ranges::find(v, 2) == std::ranges::end(v)) {} + auto it = std::find(v.begin(), v.end(), 2); +} + +void test_nested_parens() { + int arr[] = {1, 2, 3}; + if (!((std::find(arr, arr + 3, 2)))) {} + // CHECK-MESSAGES: :[[@LINE-1]]:8: warning: result of standard algorithm used in boolean context; did you mean to compare with the end iterator? [bugprone-missing-end-comparison] + // CHECK-FIXES: if ((((std::find(arr, arr + 3, 2))) == arr + 3)) {} +} + +struct Data { std::vector<int> v; }; +void test_member_expr(Data& d) { + if (std::ranges::find(d.v, 2)) {} + // CHECK-MESSAGES: :[[@LINE-1]]:7: warning: result of standard algorithm used in boolean context; did you mean to compare with the end iterator? [bugprone-missing-end-comparison] + // CHECK-FIXES: if ((std::ranges::find(d.v, 2) != std::ranges::end(d.v))) {} +} + +void test_nullptr_comparison() { + if (std::find((int*)nullptr, (int*)nullptr, 2)) {} +} + +void test_double_negation() { + int arr[] = {1, 2, 3}; + auto it = std::find(arr, arr + 3, 2); + if (!!it) {} + // CHECK-MESSAGES: :[[@LINE-1]]:9: warning: result of standard algorithm used in boolean context; did you mean to compare with the end iterator? [bugprone-missing-end-comparison] + // CHECK-FIXES: if (!(it == arr + 3)) {} } void test_custom_iterator() { CustomIterator begin{nullptr}, end{nullptr}; if (std::find(begin, end, 2)) {} // CHECK-MESSAGES: :[[@LINE-1]]:7: warning: result of standard algorithm used in boolean context - // CHECK-FIXES: if (std::find(begin, end, 2) != end) {} + // CHECK-FIXES: if ((std::find(begin, end, 2) != end)) {} } -void test_complex_end() { +void test_search() { int arr[] = {1, 2, 3}; - if (std::find(arr, arr + 3, 2)) {} + int* begin = arr; + int* end = arr + 3; + + if (std::search(begin, end, begin, end)) {} // CHECK-MESSAGES: :[[@LINE-1]]:7: warning: result of standard algorithm used in boolean context - // CHECK-FIXES: if (std::find(arr, arr + 3, 2) != arr + 3) {} + // CHECK-FIXES: if ((std::search(begin, end, begin, end) != end)) {} } -void test_sentinel() { - int* ptr = nullptr; - if (std::find<int*>(ptr, nullptr, 10)) {} - // No warning expected for nullptr sentinel -} \ No newline at end of file +namespace other { + bool find(int* b, int* e, int v); +} + +void test_other_namespace() { + int arr[] = {1}; + if (other::find(arr, arr + 1, 1)) {} +} >From 06b5a27bdc5fca2b63cd6d43cf6b9077d538581e Mon Sep 17 00:00:00 2001 From: mitchell <[email protected]> Date: Sat, 21 Feb 2026 12:54:09 +0800 Subject: [PATCH 3/7] Apply suggestions from code review Co-authored-by: EugeneZelenko <[email protected]> --- clang-tools-extra/docs/ReleaseNotes.rst | 2 +- .../clang-tidy/checks/bugprone/missing-end-comparison.rst | 8 ++++---- 2 files changed, 5 insertions(+), 5 deletions(-) diff --git a/clang-tools-extra/docs/ReleaseNotes.rst b/clang-tools-extra/docs/ReleaseNotes.rst index b89e2c0c60bf3..3beaef6b538fa 100644 --- a/clang-tools-extra/docs/ReleaseNotes.rst +++ b/clang-tools-extra/docs/ReleaseNotes.rst @@ -105,7 +105,7 @@ New checks - New :doc:`bugprone-missing-end-comparison <clang-tidy/checks/bugprone/missing-end-comparison>` check. - Finds instances where the result of a standard algorithm is used in a boolean + Finds instances where the result of a standard algorithm is used in a Boolean context without being compared to the end iterator. - New :doc:`bugprone-unsafe-to-allow-exceptions diff --git a/clang-tools-extra/docs/clang-tidy/checks/bugprone/missing-end-comparison.rst b/clang-tools-extra/docs/clang-tidy/checks/bugprone/missing-end-comparison.rst index 4f1ac0d7657ab..c2c93e8f52325 100644 --- a/clang-tools-extra/docs/clang-tidy/checks/bugprone/missing-end-comparison.rst +++ b/clang-tools-extra/docs/clang-tidy/checks/bugprone/missing-end-comparison.rst @@ -3,15 +3,15 @@ bugprone-missing-end-comparison =============================== -Finds instances where the result of a standard algorithm is used in a boolean +Finds instances where the result of a standard algorithm is used in a Boolean context without being compared to the end iterator. -Standard algorithms such as `std::find`, `std::search`, and `std::lower_bound` +Standard algorithms such as ``std::find``, ``std::search``, and ``std::lower_bound`` return an iterator to the element if found, or the end iterator otherwise. -Using the result directly in a boolean context (like an `if` statement) is +Using the result directly in a Boolean context (like an ``if`` statement) is almost always a bug, as it only checks if the iterator itself evaluates to -`true`, which may always be true for many iterator types (including pointers). +``true``, which may always be true for many iterator types (including pointers). Examples: >From 46e59626253789f837d0d5273da22786efe65c78 Mon Sep 17 00:00:00 2001 From: mtx <[email protected]> Date: Sat, 21 Feb 2026 13:48:54 +0800 Subject: [PATCH 4/7] fix docs --- .../bugprone/missing-end-comparison.rst | 49 ++++++++++--------- 1 file changed, 25 insertions(+), 24 deletions(-) diff --git a/clang-tools-extra/docs/clang-tidy/checks/bugprone/missing-end-comparison.rst b/clang-tools-extra/docs/clang-tidy/checks/bugprone/missing-end-comparison.rst index c2c93e8f52325..6353bc5b2a3b8 100644 --- a/clang-tools-extra/docs/clang-tidy/checks/bugprone/missing-end-comparison.rst +++ b/clang-tools-extra/docs/clang-tidy/checks/bugprone/missing-end-comparison.rst @@ -6,12 +6,13 @@ bugprone-missing-end-comparison Finds instances where the result of a standard algorithm is used in a Boolean context without being compared to the end iterator. -Standard algorithms such as ``std::find``, ``std::search``, and ``std::lower_bound`` -return an iterator to the element if found, or the end iterator otherwise. +Standard algorithms such as ``std::find``, ``std::search``, and +``std::lower_bound`` return an iterator to the element if found, or the end +iterator otherwise. Using the result directly in a Boolean context (like an ``if`` statement) is almost always a bug, as it only checks if the iterator itself evaluates to -``true``, which may always be true for many iterator types (including pointers). +``true``, which may always be true for many iterator types. Examples: @@ -46,24 +47,24 @@ The check also handles range-based algorithms introduced in C++20. Supported algorithms: -- `std::find` -- `std::find_if` -- `std::find_if_not` -- `std::search` -- `std::search_n` -- `std::find_end` -- `std::find_first_of` -- `std::lower_bound` -- `std::upper_bound` -- `std::partition_point` -- `std::min_element` -- `std::max_element` -- `std::adjacent_find` -- `std::is_sorted_until` -- `std::ranges::find` -- `std::ranges::find_if` -- `std::ranges::find_if_not` -- `std::ranges::lower_bound` -- `std::ranges::upper_bound` -- `std::ranges::min_element` -- `std::ranges::max_element` +- ``std::find`` +- ``std::find_if`` +- ``std::find_if_not`` +- ``std::search`` +- ``std::search_n`` +- ``std::find_end`` +- ``std::find_first_of`` +- ``std::lower_bound`` +- ``std::upper_bound`` +- ``std::partition_point`` +- ``std::min_element`` +- ``std::max_element`` +- ``std::adjacent_find`` +- ``std::is_sorted_until`` +- ``std::ranges::find`` +- ``std::ranges::find_if`` +- ``std::ranges::find_if_not`` +- ``std::ranges::lower_bound`` +- ``std::ranges::upper_bound`` +- ``std::ranges::min_element`` +- ``std::ranges::max_element`` >From a22e8990be9b69be1f40b75f8f1bc75b466ba1b2 Mon Sep 17 00:00:00 2001 From: mtx <[email protected]> Date: Sat, 21 Feb 2026 14:26:46 +0800 Subject: [PATCH 5/7] Fix execution releated issues --- .../bugprone/MissingEndComparisonCheck.cpp | 13 ++++++- .../bugprone/missing-end-comparison.cpp | 36 +++++++++++++++++++ 2 files changed, 48 insertions(+), 1 deletion(-) diff --git a/clang-tools-extra/clang-tidy/bugprone/MissingEndComparisonCheck.cpp b/clang-tools-extra/clang-tidy/bugprone/MissingEndComparisonCheck.cpp index 42ff99b121f84..9e351f7c97a4c 100644 --- a/clang-tools-extra/clang-tidy/bugprone/MissingEndComparisonCheck.cpp +++ b/clang-tools-extra/clang-tidy/bugprone/MissingEndComparisonCheck.cpp @@ -88,7 +88,18 @@ void MissingEndComparisonCheck::check(const MatchFinder::MatchResult &Result) { if (Call->getNumArgs() < 2) return; - const Expr *EndArg = Call->getArg(1); + unsigned EndIdx = 1; + const Expr *FirstArg = Call->getArg(0); + if (const auto *Record = + FirstArg->getType().getNonReferenceType()->getAsCXXRecordDecl()) { + if (Record->getName().ends_with("_policy")) + EndIdx = 2; + } + + if (Call->getNumArgs() <= EndIdx) + return; + + const Expr *EndArg = Call->getArg(EndIdx); // Filters nullptr, we assume the intent might be a valid check against null if (EndArg->IgnoreParenCasts()->isNullPointerConstant( *Result.Context, Expr::NPC_ValueDependentIsNull)) diff --git a/clang-tools-extra/test/clang-tidy/checkers/bugprone/missing-end-comparison.cpp b/clang-tools-extra/test/clang-tidy/checkers/bugprone/missing-end-comparison.cpp index 6bbeef5cbde6f..c27013a7714ec 100644 --- a/clang-tools-extra/test/clang-tidy/checkers/bugprone/missing-end-comparison.cpp +++ b/clang-tools-extra/test/clang-tidy/checkers/bugprone/missing-end-comparison.cpp @@ -20,6 +20,19 @@ namespace std { template<class InputIt, class T> InputIt find(InputIt first, InputIt last, const T& value); + namespace execution { + struct sequenced_policy {}; + struct parallel_policy {}; + inline constexpr sequenced_policy seq; + inline constexpr parallel_policy par; + } + + template<class ExecutionPolicy, class InputIt, class T> + InputIt find(ExecutionPolicy&& policy, InputIt first, InputIt last, const T& value); + + template<class ExecutionPolicy, class ForwardIt, class T> + ForwardIt lower_bound(ExecutionPolicy&& policy, ForwardIt first, ForwardIt last, const T& value); + template<class ForwardIt, class T> ForwardIt lower_bound(ForwardIt first, ForwardIt last, const T& value); @@ -194,3 +207,26 @@ void test_other_namespace() { int arr[] = {1}; if (other::find(arr, arr + 1, 1)) {} } + +void test_execution_policy() { + int arr[] = {1, 2, 3}; + int* begin = arr; + int* end = arr + 3; + + if (std::find(std::execution::seq, begin, end, 2)) {} + // CHECK-MESSAGES: :[[@LINE-1]]:7: warning: result of standard algorithm used in boolean context; did you mean to compare with the end iterator? [bugprone-missing-end-comparison] + // CHECK-FIXES: if ((std::find(std::execution::seq, begin, end, 2) != end)) {} + + if (std::find(std::execution::par, begin, end, 2)) {} + // CHECK-MESSAGES: :[[@LINE-1]]:7: warning: result of standard algorithm used in boolean context; did you mean to compare with the end iterator? [bugprone-missing-end-comparison] + // CHECK-FIXES: if ((std::find(std::execution::par, begin, end, 2) != end)) {} + + auto it = std::find(std::execution::seq, begin, end, 2); + if (it) {} + // CHECK-MESSAGES: :[[@LINE-1]]:7: warning: result of standard algorithm used in boolean context; did you mean to compare with the end iterator? [bugprone-missing-end-comparison] + // CHECK-FIXES: if ((it != end)) {} + + if (std::lower_bound(std::execution::seq, begin, end, 2)) {} + // CHECK-MESSAGES: :[[@LINE-1]]:7: warning: result of standard algorithm used in boolean context; did you mean to compare with the end iterator? [bugprone-missing-end-comparison] + // CHECK-FIXES: if ((std::lower_bound(std::execution::seq, begin, end, 2) != end)) {} +} >From 3905ac32c21e6ef39e86e262091a2d11c08d4ae1 Mon Sep 17 00:00:00 2001 From: mtx <[email protected]> Date: Sun, 22 Feb 2026 00:55:12 +0800 Subject: [PATCH 6/7] more algorithms support, better example, more testcases --- .../bugprone/MissingEndComparisonCheck.cpp | 9 ++- .../bugprone/missing-end-comparison.rst | 42 +++++----- .../bugprone/missing-end-comparison.cpp | 76 +++++++++++++++++++ 3 files changed, 104 insertions(+), 23 deletions(-) diff --git a/clang-tools-extra/clang-tidy/bugprone/MissingEndComparisonCheck.cpp b/clang-tools-extra/clang-tidy/bugprone/MissingEndComparisonCheck.cpp index 9e351f7c97a4c..9699ab48faf84 100644 --- a/clang-tools-extra/clang-tidy/bugprone/MissingEndComparisonCheck.cpp +++ b/clang-tools-extra/clang-tidy/bugprone/MissingEndComparisonCheck.cpp @@ -28,10 +28,11 @@ constexpr llvm::StringRef IteratorAlgorithms[] = { "::std::adjacent_find", "::std::is_sorted_until"}; constexpr llvm::StringRef RangeAlgorithms[] = { - "::std::ranges::find", "::std::ranges::find_if", - "::std::ranges::find_if_not", "::std::ranges::lower_bound", - "::std::ranges::upper_bound", "::std::ranges::min_element", - "::std::ranges::max_element"}; + "::std::ranges::find", "::std::ranges::find_if", + "::std::ranges::find_if_not", "::std::ranges::lower_bound", + "::std::ranges::upper_bound", "::std::ranges::min_element", + "::std::ranges::max_element", "::std::ranges::find_first_of", + "::std::ranges::adjacent_find", "::std::ranges::is_sorted_until"}; } // namespace diff --git a/clang-tools-extra/docs/clang-tidy/checks/bugprone/missing-end-comparison.rst b/clang-tools-extra/docs/clang-tidy/checks/bugprone/missing-end-comparison.rst index 6353bc5b2a3b8..e4381382a136a 100644 --- a/clang-tools-extra/docs/clang-tidy/checks/bugprone/missing-end-comparison.rst +++ b/clang-tools-extra/docs/clang-tidy/checks/bugprone/missing-end-comparison.rst @@ -18,29 +18,30 @@ Examples: .. code-block:: c++ - int arr[] = {1, 2, 3}; - int* begin = std::begin(arr); - int* end = std::end(arr); + void example() { + int arr[] = {1, 2, 3}; + int* begin = std::begin(arr); + int* end = std::end(arr); - // Problematic: - if (std::find(begin, end, 2)) { - // ... - } + if (std::find(begin, end, 2)) { + // ... + } - // Fixed by the check: - if ((std::find(begin, end, 2) != end)) { - // ... - } + // Fixed by the check: + if ((std::find(begin, end, 2) != end)) { + // ... + } - // C++20 ranges: - std::vector<int> v = {1, 2, 3}; - if (std::ranges::find(v, 2)) { // Problematic - // ... - } + // C++20 ranges: + int v[] = {1, 2, 3}; + if (std::ranges::find(v, 2)) { + // ... + } - // Fixed by the check: - if ((std::ranges::find(v, 2) != std::ranges::end(v))) { - // ... + // Fixed by the check: + if ((std::ranges::find(v, 2) != std::ranges::end(v))) { + // ... + } } The check also handles range-based algorithms introduced in C++20. @@ -68,3 +69,6 @@ Supported algorithms: - ``std::ranges::upper_bound`` - ``std::ranges::min_element`` - ``std::ranges::max_element`` +- ``std::ranges::find_first_of`` +- ``std::ranges::adjacent_find`` +- ``std::ranges::is_sorted_until`` diff --git a/clang-tools-extra/test/clang-tidy/checkers/bugprone/missing-end-comparison.cpp b/clang-tools-extra/test/clang-tidy/checkers/bugprone/missing-end-comparison.cpp index c27013a7714ec..8e581a88772bb 100644 --- a/clang-tools-extra/test/clang-tidy/checkers/bugprone/missing-end-comparison.cpp +++ b/clang-tools-extra/test/clang-tidy/checkers/bugprone/missing-end-comparison.cpp @@ -62,6 +62,30 @@ namespace std { void* operator()(I first, S last, const T& value) const; }; inline constexpr FindFn find; + + struct FindFirstOfFn { + template<typename R1, typename R2> + void* operator()(R1&& r1, R2&& r2) const; + template<typename I1, typename S1, typename I2, typename S2> + void* operator()(I1 f1, S1 l1, I2 f2, S2 l2) const; + }; + inline constexpr FindFirstOfFn find_first_of; + + struct AdjacentFindFn { + template<typename R> + void* operator()(R&& r) const; + template<typename I, typename S> + void* operator()(I f, S l) const; + }; + inline constexpr AdjacentFindFn adjacent_find; + + struct IsSortedUntilFn { + template<typename R> + void* operator()(R&& r) const; + template<typename I, typename S> + void* operator()(I f, S l) const; + }; + inline constexpr IsSortedUntilFn is_sorted_until; } } @@ -132,6 +156,35 @@ void test_ranges() { if (it) {} // CHECK-MESSAGES: :[[@LINE-1]]:7: warning: result of standard algorithm used in boolean context; did you mean to compare with the end iterator? [bugprone-missing-end-comparison] // CHECK-FIXES: if ((it != std::ranges::end(v))) {} + + std::vector<int> v1, v2; + int arr[] = {1, 2, 3}; + int *begin = arr; + int *end = arr + 3; + + if (std::ranges::find_first_of(v1, v2)) {} + // CHECK-MESSAGES: :[[@LINE-1]]:7: warning: result of standard algorithm used in boolean context; did you mean to compare with the end iterator? [bugprone-missing-end-comparison] + // CHECK-FIXES: if ((std::ranges::find_first_of(v1, v2) != std::ranges::end(v1))) {} + + if (std::ranges::find_first_of(begin, end, begin, end)) {} + // CHECK-MESSAGES: :[[@LINE-1]]:7: warning: result of standard algorithm used in boolean context; did you mean to compare with the end iterator? [bugprone-missing-end-comparison] + // CHECK-FIXES: if ((std::ranges::find_first_of(begin, end, begin, end) != end)) {} + + if (std::ranges::adjacent_find(v1)) {} + // CHECK-MESSAGES: :[[@LINE-1]]:7: warning: result of standard algorithm used in boolean context; did you mean to compare with the end iterator? [bugprone-missing-end-comparison] + // CHECK-FIXES: if ((std::ranges::adjacent_find(v1) != std::ranges::end(v1))) {} + + if (std::ranges::adjacent_find(begin, end)) {} + // CHECK-MESSAGES: :[[@LINE-1]]:7: warning: result of standard algorithm used in boolean context; did you mean to compare with the end iterator? [bugprone-missing-end-comparison] + // CHECK-FIXES: if ((std::ranges::adjacent_find(begin, end) != end)) {} + + if (std::ranges::is_sorted_until(v1)) {} + // CHECK-MESSAGES: :[[@LINE-1]]:7: warning: result of standard algorithm used in boolean context; did you mean to compare with the end iterator? [bugprone-missing-end-comparison] + // CHECK-FIXES: if ((std::ranges::is_sorted_until(v1) != std::ranges::end(v1))) {} + + if (std::ranges::is_sorted_until(begin, end)) {} + // CHECK-MESSAGES: :[[@LINE-1]]:7: warning: result of standard algorithm used in boolean context; did you mean to compare with the end iterator? [bugprone-missing-end-comparison] + // CHECK-FIXES: if ((std::ranges::is_sorted_until(begin, end) != end)) {} } void test_ranges_iterator_pair() { @@ -230,3 +283,26 @@ void test_execution_policy() { // CHECK-MESSAGES: :[[@LINE-1]]:7: warning: result of standard algorithm used in boolean context; did you mean to compare with the end iterator? [bugprone-missing-end-comparison] // CHECK-FIXES: if ((std::lower_bound(std::execution::seq, begin, end, 2) != end)) {} } + +void test_loops() { + int arr[] = {1, 2, 3}; + int *begin = arr; + int *end = arr + 3; + + while (std::find(begin, end, 2)) { break; } + // CHECK-MESSAGES: :[[@LINE-1]]:10: warning: result of standard algorithm used in boolean context; did you mean to compare with the end iterator? [bugprone-missing-end-comparison] + // CHECK-FIXES: while ((std::find(begin, end, 2) != end)) { break; } + + do { } while (std::find(begin, end, 2)); + // CHECK-MESSAGES: :[[@LINE-1]]:17: warning: result of standard algorithm used in boolean context; did you mean to compare with the end iterator? [bugprone-missing-end-comparison] + // CHECK-FIXES: do { } while ((std::find(begin, end, 2) != end)); + + for (auto it = std::find(begin, end, 2); it; ) { break; } + // CHECK-MESSAGES: :[[@LINE-1]]:44: warning: result of standard algorithm used in boolean context; did you mean to compare with the end iterator? [bugprone-missing-end-comparison] + // CHECK-FIXES: for (auto it = std::find(begin, end, 2); (it != end); ) { break; } + + std::vector<int> v; + for (auto it = std::ranges::find(v, 2); !it; ) { break; } + // CHECK-MESSAGES: :[[@LINE-1]]:44: warning: result of standard algorithm used in boolean context; did you mean to compare with the end iterator? [bugprone-missing-end-comparison] + // CHECK-FIXES: for (auto it = std::ranges::find(v, 2); (it == std::ranges::end(v)); ) { break; } +} >From 9cac8bf7a310e4631eb6eb24d6aa981384b096e0 Mon Sep 17 00:00:00 2001 From: mtx <[email protected]> Date: Sun, 22 Feb 2026 01:57:03 +0800 Subject: [PATCH 7/7] ~~ --- .../bugprone/MissingEndComparisonCheck.cpp | 70 ++++++++++++++----- .../bugprone/missing-end-comparison.cpp | 6 ++ 2 files changed, 58 insertions(+), 18 deletions(-) diff --git a/clang-tools-extra/clang-tidy/bugprone/MissingEndComparisonCheck.cpp b/clang-tools-extra/clang-tidy/bugprone/MissingEndComparisonCheck.cpp index 9699ab48faf84..2957e6920be09 100644 --- a/clang-tools-extra/clang-tidy/bugprone/MissingEndComparisonCheck.cpp +++ b/clang-tools-extra/clang-tidy/bugprone/MissingEndComparisonCheck.cpp @@ -34,6 +34,16 @@ constexpr llvm::StringRef RangeAlgorithms[] = { "::std::ranges::max_element", "::std::ranges::find_first_of", "::std::ranges::adjacent_find", "::std::ranges::is_sorted_until"}; +AST_MATCHER(DeclStmt, isConditionVariableStatement) { + return !match( + declStmt(hasAncestor(stmt(anyOf( + ifStmt(hasConditionVariableStatement(equalsNode(&Node))), + whileStmt(hasConditionVariableStatement(equalsNode(&Node))), + forStmt(hasConditionVariableStatement(equalsNode(&Node))))))), + Node, Finder->getASTContext()) + .empty(); +} + } // namespace void MissingEndComparisonCheck::registerMatchers(MatchFinder *Finder) { @@ -61,7 +71,8 @@ void MissingEndComparisonCheck::registerMatchers(MatchFinder *Finder) { // FIXME: This only handles variables initialized directly by the algorithm. // We may need to introduce more accurate dataflow analysis in the future. const auto VarWithAlgoInit = - varDecl(hasInitializer(ignoringParenImpCasts(AnyAlgoCall))); + varDecl(hasInitializer(ignoringParenImpCasts(AnyAlgoCall))) + .bind("initVar"); const auto IsVariableBoolUsage = anyOf(implicitCastExpr(hasCastKind(CK_PointerToBoolean), @@ -163,25 +174,48 @@ void MissingEndComparisonCheck::check(const MatchFinder::MatchResult &Result) { "result of standard algorithm used in boolean context; did " "you mean to compare with the end iterator?"); - if (!EndExprText.empty()) { - if (IsNegated) { - // !it -> (it == end) - Diag << FixItHint::CreateReplacement(NotOp->getOperatorLoc(), "("); - Diag << FixItHint::CreateInsertion( - Lexer::getLocForEndOfToken(BoolOp->getEndLoc(), 0, - *Result.SourceManager, - Result.Context->getLangOpts()), - " == " + EndExprText + ")"); - } else { - // it -> (it != end) - Diag << FixItHint::CreateInsertion(BoolOp->getBeginLoc(), "("); - Diag << FixItHint::CreateInsertion( - Lexer::getLocForEndOfToken(BoolOp->getEndLoc(), 0, - *Result.SourceManager, - Result.Context->getLangOpts()), - " != " + EndExprText + ")"); + if (EndExprText.empty()) + return; + + // Suppress fix-it if the expression is part of a variable declaration or a + // condition variable declaration. + if (const auto *InitVar = Result.Nodes.getNodeAs<VarDecl>("initVar")) { + if (InitVar->getType()->isBooleanType()) + return; + + const auto &Parents = Result.Context->getParents(*InitVar); + if (!Parents.empty()) { + if (const auto *ParentDecl = Parents[0].get<DeclStmt>()) { + if (!match(declStmt(isConditionVariableStatement()), *ParentDecl, + *Result.Context) + .empty()) { + return; + } + } } } + + const auto &Parents = Result.Context->getParents(*BoolOp); + if (!Parents.empty() && Parents[0].get<VarDecl>()) + return; + + if (IsNegated) { + // !it -> (it == end) + Diag << FixItHint::CreateReplacement(NotOp->getOperatorLoc(), "("); + Diag << FixItHint::CreateInsertion( + Lexer::getLocForEndOfToken(BoolOp->getEndLoc(), 0, + *Result.SourceManager, + Result.Context->getLangOpts()), + " == " + EndExprText + ")"); + } else { + // it -> (it != end) + Diag << FixItHint::CreateInsertion(BoolOp->getBeginLoc(), "("); + Diag << FixItHint::CreateInsertion( + Lexer::getLocForEndOfToken(BoolOp->getEndLoc(), 0, + *Result.SourceManager, + Result.Context->getLangOpts()), + " != " + EndExprText + ")"); + } } } // namespace clang::tidy::bugprone diff --git a/clang-tools-extra/test/clang-tidy/checkers/bugprone/missing-end-comparison.cpp b/clang-tools-extra/test/clang-tidy/checkers/bugprone/missing-end-comparison.cpp index 8e581a88772bb..1bf9be4700779 100644 --- a/clang-tools-extra/test/clang-tidy/checkers/bugprone/missing-end-comparison.cpp +++ b/clang-tools-extra/test/clang-tidy/checkers/bugprone/missing-end-comparison.cpp @@ -306,3 +306,9 @@ void test_loops() { // CHECK-MESSAGES: :[[@LINE-1]]:44: warning: result of standard algorithm used in boolean context; did you mean to compare with the end iterator? [bugprone-missing-end-comparison] // CHECK-FIXES: for (auto it = std::ranges::find(v, 2); (it == std::ranges::end(v)); ) { break; } } + +void test_invalid_fixit() { + int arr[] = {1, 2, 3}; + if (int* it2 = std::find(arr, arr + 3, 2)) {} + // CHECK-MESSAGES: :[[@LINE-1]]:12: warning: result of standard algorithm used in boolean context; did you mean to compare with the end iterator? [bugprone-missing-end-comparison] +} _______________________________________________ cfe-commits mailing list [email protected] https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
