Index: clang-tidy/bugprone/BugproneTidyModule.cpp =================================================================== --- clang-tidy/bugprone/BugproneTidyModule.cpp +++ clang-tidy/bugprone/BugproneTidyModule.cpp @@ -24,6 +24,7 @@ #include "IncorrectRoundingsCheck.h" #include "IntegerDivisionCheck.h" #include "LambdaFunctionNameCheck.h" +#include "LoopVariableMutationsCheck.h" #include "MacroParenthesesCheck.h" #include "MacroRepeatedSideEffectsCheck.h" #include "MisplacedOperatorInStrlenInAllocCheck.h" @@ -84,6 +85,8 @@ "bugprone-integer-division"); CheckFactories.registerCheck( "bugprone-lambda-function-name"); + CheckFactories.registerCheck( + "bugprone-loop-variable-mutations"); CheckFactories.registerCheck( "bugprone-macro-parentheses"); CheckFactories.registerCheck( Index: clang-tidy/bugprone/CMakeLists.txt =================================================================== --- clang-tidy/bugprone/CMakeLists.txt +++ clang-tidy/bugprone/CMakeLists.txt @@ -15,6 +15,7 @@ IncorrectRoundingsCheck.cpp IntegerDivisionCheck.cpp LambdaFunctionNameCheck.cpp + LoopVariableMutationsCheck.cpp MacroParenthesesCheck.cpp MacroRepeatedSideEffectsCheck.cpp MisplacedOperatorInStrlenInAllocCheck.cpp Index: clang-tidy/bugprone/LoopVariableMutationsCheck.h =================================================================== --- /dev/null +++ clang-tidy/bugprone/LoopVariableMutationsCheck.h @@ -0,0 +1,35 @@ +//===--- LoopVariableMutationsCheck.h - clang-tidy---------------*- C++ -*-===// +// +// The LLVM Compiler Infrastructure +// +// This file is distributed under the University of Illinois Open Source +// License. See LICENSE.TXT for details. +// +//===----------------------------------------------------------------------===// + +#ifndef LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_BUGPRONE_LOOPVARIABLEMUTATIONSCHECK_H +#define LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_BUGPRONE_LOOPVARIABLEMUTATIONSCHECK_H + +#include "../ClangTidy.h" + +namespace clang { +namespace tidy { +namespace bugprone { + +/// FIXME: Write a short description. +/// +/// For the user-facing documentation see: +/// http://clang.llvm.org/extra/clang-tidy/checks/bugprone-loop-variable-mutations.html +class LoopVariableMutationsCheck : public ClangTidyCheck { +public: + LoopVariableMutationsCheck(StringRef Name, ClangTidyContext *Context) + : ClangTidyCheck(Name, Context) {} + void registerMatchers(ast_matchers::MatchFinder *Finder) override; + void check(const ast_matchers::MatchFinder::MatchResult &Result) override; +}; + +} // namespace bugprone +} // namespace tidy +} // namespace clang + +#endif // LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_BUGPRONE_LOOPVARIABLEMUTATIONSCHECK_H Index: clang-tidy/bugprone/LoopVariableMutationsCheck.cpp =================================================================== --- /dev/null +++ clang-tidy/bugprone/LoopVariableMutationsCheck.cpp @@ -0,0 +1,125 @@ +//===--- LoopVariableMutationsCheck.cpp - clang-tidy-----------------------===// +// +// The LLVM Compiler Infrastructure +// +// This file is distributed under the University of Illinois Open Source +// License. See LICENSE.TXT for details. +// +//===----------------------------------------------------------------------===// + +#include "LoopVariableMutationsCheck.h" +#include "clang/AST/ASTContext.h" +#include "clang/ASTMatchers/ASTMatchFinder.h" +#include "clang/Analysis/Analyses/ExprMutationAnalyzer.h" +#include +#include + +using namespace clang::ast_matchers; + +namespace clang { +namespace tidy { +namespace bugprone { + +namespace { + +struct LoopVariableMutations { + const Stmt *MutationInCond = nullptr; + static constexpr auto MutationInCondDsc = + llvm::StringLiteral("mutation in loop condition occurs here"); + const Stmt *MutationInBody = nullptr; + static constexpr auto MutationInBodyDsc = + llvm::StringLiteral("mutation in loop body occurs here"); + const Stmt *MutationInIncr = nullptr; + static constexpr auto MutationInIncrDsc = + llvm::StringLiteral("mutation in loop iteration expression occurs here"); + + bool hasMoreThanOneMutation() const { + // Check that more than one of these is not a nullptr. + const int MutationCount = int(bool(MutationInCond)) + + int(bool(MutationInBody)) + + int(bool(MutationInIncr)); + return MutationCount > 1; + } + + static constexpr auto InAllThree = + llvm::StringLiteral("variable is mutated in the loop condition, loop " + "body and loop iteration expression"); + static constexpr auto InCondAndBody = llvm::StringLiteral( + "variable is mutated in the loop condition and loop body"); + static constexpr auto InBodyAndIncr = llvm::StringLiteral( + "variable is mutated in the loop body and loop iteration expression"); + static constexpr auto InCondAndIncr = + llvm::StringLiteral("variable is mutated in the loop condition and loop " + "iteration expression"); + + llvm::StringLiteral describe() const { + if (MutationInCond && MutationInBody && MutationInIncr) + return InAllThree; + else if (MutationInBody && MutationInIncr) + return InBodyAndIncr; + else if (MutationInCond && MutationInBody) + return InCondAndBody; + else if (MutationInCond && MutationInIncr) + return InCondAndIncr; + llvm_unreachable("are those all the combinations?"); + } + + LoopVariableMutations(const ForStmt &Loop, const VarDecl &Variable, + ASTContext &Context) { + for (const auto &PossibleMutation : + std::initializer_list>{ + {Loop.getCond(), MutationInCond}, + {Loop.getBody(), MutationInBody}, + {Loop.getInc(), MutationInIncr}}) { + if (PossibleMutation.first) { + PossibleMutation.second = + ExprMutationAnalyzer(*PossibleMutation.first, Context) + .findDeclMutation(&Variable); + } + } + } +}; + +constexpr llvm::StringLiteral LoopVariableMutations::MutationInCondDsc; +constexpr llvm::StringLiteral LoopVariableMutations::MutationInBodyDsc; +constexpr llvm::StringLiteral LoopVariableMutations::MutationInIncrDsc; + +} // namespace + +void LoopVariableMutationsCheck::registerMatchers(MatchFinder *Finder) { + auto matchVarDeclRefExpr = forEachDescendant( + declRefExpr(hasDeclaration(varDecl().bind("variable")))); + Finder->addMatcher(forStmt(eachOf(hasIncrement(matchVarDeclRefExpr), + hasCondition(matchVarDeclRefExpr))) + .bind("loop"), + this); +} + +void LoopVariableMutationsCheck::check(const MatchFinder::MatchResult &Result) { + const auto *Loop = Result.Nodes.getNodeAs("loop"); + const auto *Variable = Result.Nodes.getNodeAs("variable"); + assert(Loop && Variable && "Should have both of these."); + + // Analyze the mutations of the variable in the loop. + LoopVariableMutations A(*Loop, *Variable, *Result.Context); + + if (!A.hasMoreThanOneMutation()) + return; // Great, only mutated in one (or zero!) place. + + diag(Variable->getLocation(), A.describe(), DiagnosticIDs::Warning); + + diag(Loop->getForLoc(), "loop in question", DiagnosticIDs::Note); + + for (const auto &Mutation : std::initializer_list< + std::pair>{ + {A.MutationInCond, A.MutationInCondDsc}, + {A.MutationInBody, A.MutationInBodyDsc}, + {A.MutationInIncr, A.MutationInIncrDsc}}) { + if (Mutation.first) + diag(Mutation.first->getBeginLoc(), Mutation.second, DiagnosticIDs::Note); + } +} + +} // namespace bugprone +} // namespace tidy +} // namespace clang Index: docs/ReleaseNotes.rst =================================================================== --- docs/ReleaseNotes.rst +++ docs/ReleaseNotes.rst @@ -57,6 +57,11 @@ Improvements to clang-tidy -------------------------- +- New :doc:`bugprone-loop-variable-mutations + ` check. + + FIXME: add release notes. + - New :doc:`abseil-duration-division ` check. Index: docs/clang-tidy/checks/bugprone-loop-variable-mutations.rst =================================================================== --- /dev/null +++ docs/clang-tidy/checks/bugprone-loop-variable-mutations.rst @@ -0,0 +1,6 @@ +.. title:: clang-tidy - bugprone-loop-variable-mutations + +bugprone-loop-variable-mutations +================================ + +FIXME: Describe what patterns does the check detect and why. Give examples. Index: docs/clang-tidy/checks/list.rst =================================================================== --- docs/clang-tidy/checks/list.rst +++ docs/clang-tidy/checks/list.rst @@ -9,8 +9,8 @@ abseil-no-internal-dependencies abseil-no-namespace abseil-redundant-strcat-calls - abseil-string-find-startswith abseil-str-cat-append + abseil-string-find-startswith android-cloexec-accept android-cloexec-accept4 android-cloexec-creat @@ -38,6 +38,7 @@ bugprone-incorrect-roundings bugprone-integer-division bugprone-lambda-function-name + bugprone-loop-variable-mutations bugprone-macro-parentheses bugprone-macro-repeated-side-effects bugprone-misplaced-operator-in-strlen-in-alloc Index: test/clang-tidy/bugprone-loop-variable-mutations.cpp =================================================================== --- /dev/null +++ test/clang-tidy/bugprone-loop-variable-mutations.cpp @@ -0,0 +1,14 @@ +// RUN: %check_clang_tidy %s bugprone-loop-variable-mutations %t + +// FIXME: Add something that triggers the check here. +void f(); +// CHECK-MESSAGES: :[[@LINE-1]]:6: warning: function 'f' is insufficiently awesome [bugprone-loop-variable-mutations] + +// FIXME: Verify the applied fix. +// * Make the CHECK patterns specific enough and try to make verified lines +// unique to avoid incorrect matches. +// * Use {{}} for regular expressions. +// CHECK-FIXES: {{^}}void awesome_f();{{$}} + +// FIXME: Add something that doesn't trigger the check here. +void awesome_f2();