Index: clang-tidy/readability/CMakeLists.txt =================================================================== --- clang-tidy/readability/CMakeLists.txt +++ clang-tidy/readability/CMakeLists.txt @@ -29,6 +29,7 @@ StaticDefinitionInAnonymousNamespaceCheck.cpp StringCompareCheck.cpp UniqueptrDeleteReleaseCheck.cpp + UnmodifiedNonConstVariableCheck.cpp LINK_LIBS clangAST Index: clang-tidy/readability/ReadabilityTidyModule.cpp =================================================================== --- clang-tidy/readability/ReadabilityTidyModule.cpp +++ clang-tidy/readability/ReadabilityTidyModule.cpp @@ -36,6 +36,7 @@ #include "StaticDefinitionInAnonymousNamespaceCheck.h" #include "StringCompareCheck.h" #include "UniqueptrDeleteReleaseCheck.h" +#include "UnmodifiedNonConstVariableCheck.h" namespace clang { namespace tidy { @@ -78,6 +79,8 @@ "readability-static-definition-in-anonymous-namespace"); CheckFactories.registerCheck( "readability-string-compare"); + CheckFactories.registerCheck( + "readability-unmodified-non-const-variable"); CheckFactories.registerCheck( "readability-named-parameter"); CheckFactories.registerCheck( Index: clang-tidy/readability/UnmodifiedNonConstVariableCheck.h =================================================================== --- /dev/null +++ clang-tidy/readability/UnmodifiedNonConstVariableCheck.h @@ -0,0 +1,47 @@ +//===--- UnmodifiedNonConstVariableCheck.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_READABILITY_UNMODIFIEDNONCONSTVARIABLECHECK_H +#define LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_READABILITY_UNMODIFIEDNONCONSTVARIABLECHECK_H + +#include "../ClangTidy.h" + +namespace clang { +namespace tidy { +namespace readability { + +/// Finds declarations of non-const variables that never get modified. +/// +/// For the user-facing documentation see: +/// http://clang.llvm.org/extra/clang-tidy/checks/readability-unmodified-non-const-variable.html +class UnmodifiedNonConstVariableCheck : public ClangTidyCheck { +public: + UnmodifiedNonConstVariableCheck(StringRef Name, ClangTidyContext *Context) + : ClangTidyCheck(Name, Context), + MaxPerTranslationUnit(Options.get("MaxPerTranslationUnit", 50)), + IgnorePointers(Options.get("IgnorePointers", false)) {} + void registerMatchers(ast_matchers::MatchFinder *Finder) override; + void check(const ast_matchers::MatchFinder::MatchResult &Result) override; + void onStartOfTranslationUnit() override { Count = 0; } + void storeOptions(ClangTidyOptions::OptionMap &Opts) override { + Options.store(Opts, "MaxPerTranslationUnit", MaxPerTranslationUnit); + Options.store(Opts, "IgnorePointers", IgnorePointers); + } + +private: + const int MaxPerTranslationUnit; + const bool IgnorePointers; + int Count; +}; + +} // namespace readability +} // namespace tidy +} // namespace clang + +#endif // LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_READABILITY_UNMODIFIEDNONCONSTVARIABLECHECK_H Index: clang-tidy/readability/UnmodifiedNonConstVariableCheck.cpp =================================================================== --- /dev/null +++ clang-tidy/readability/UnmodifiedNonConstVariableCheck.cpp @@ -0,0 +1,57 @@ +//===--- UnmodifiedNonConstVariableCheck.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 "UnmodifiedNonConstVariableCheck.h" +#include "../utils/ASTUtils.h" +#include "clang/AST/ASTContext.h" +#include "clang/ASTMatchers/ASTMatchFinder.h" + +using namespace clang::ast_matchers; + +namespace clang { +namespace tidy { +namespace readability { + +void UnmodifiedNonConstVariableCheck::registerMatchers(MatchFinder *Finder) { + const auto ConstTypes = qualType( + anyOf(isConstQualified(), referenceType(pointee(isConstQualified())))); + const auto TypeFilter = hasType( + IgnorePointers ? ConstTypes : qualType(anyOf(ConstTypes, pointerType()))); + const auto DeclFilter = + anyOf(parmVarDecl(), isImplicit(), isConstexpr(), TypeFilter); + Finder->addMatcher( + varDecl(unless(DeclFilter), hasAncestor(compoundStmt().bind("stmt"))) + .bind("decl"), + this); +} + +void UnmodifiedNonConstVariableCheck::check( + const MatchFinder::MatchResult &Result) { + if (++Count > MaxPerTranslationUnit) + return; + + const auto *Decl = Result.Nodes.getNodeAs("decl"); + if (Decl->Decl::getLocStart().isMacroID()) + return; + const auto *Compound = Result.Nodes.getNodeAs("stmt"); + const auto Exprs = + match(findAll(declRefExpr(to(varDecl(equalsNode(Decl)))).bind("expr")), + *Compound, *Result.Context); + for (const auto &Node : Exprs) { + if (utils::isModified(*Node.getNodeAs("expr"), *Compound, + Result.Context)) + return; + } + diag(Decl->getLocation(), + "declaring a non-const variable but never modified it"); +} + +} // namespace readability +} // namespace tidy +} // namespace clang Index: clang-tidy/utils/ASTUtils.h =================================================================== --- clang-tidy/utils/ASTUtils.h +++ clang-tidy/utils/ASTUtils.h @@ -27,6 +27,10 @@ bool exprHasBitFlagWithSpelling(const Expr *Flags, const SourceManager &SM, const LangOptions &LangOpts, StringRef FlagName); + +// Checks whether `Exp` is (potentially) modified within `Stm`. +bool isModified(const Expr& Exp, const Stmt& Stm, ASTContext* Context); + } // namespace utils } // namespace tidy } // namespace clang Index: clang-tidy/utils/ASTUtils.cpp =================================================================== --- clang-tidy/utils/ASTUtils.cpp +++ clang-tidy/utils/ASTUtils.cpp @@ -67,6 +67,96 @@ return true; } +namespace { +class MatchCallbackAdaptor : public MatchFinder::MatchCallback { +public: + explicit MatchCallbackAdaptor( + std::function Func) + : Func(std::move(Func)) {} + void run(const MatchFinder::MatchResult &Result) override { Func(Result); } + +private: + std::function Func; +}; +} // namespace + +bool isModified(const Expr& Exp, const Stmt& Stm, ASTContext* Context) { + // LHS of any assignment operators. + const auto AsAssignmentLhs = binaryOperator( + anyOf(hasOperatorName("="), hasOperatorName("+="), hasOperatorName("-="), + hasOperatorName("*="), hasOperatorName("/="), hasOperatorName("%="), + hasOperatorName("^="), hasOperatorName("&="), hasOperatorName("|="), + hasOperatorName(">>="), hasOperatorName("<<=")), + hasLHS(equalsNode(&Exp))); + + // Operand of increment/decrement operators. + const auto AsIncDecOperand = + unaryOperator(anyOf(hasOperatorName("++"), hasOperatorName("--")), + hasUnaryOperand(equalsNode(&Exp))); + + // Invoking non-const member function. + const auto NonConstMethod = cxxMethodDecl(unless(isConst())); + const auto AsNonConstThis = expr( + anyOf(cxxMemberCallExpr(callee(NonConstMethod), on(equalsNode(&Exp))), + cxxOperatorCallExpr(callee(NonConstMethod), + hasArgument(0, equalsNode(&Exp))))); + + // Used as non-const-ref argument when calling a function. + const auto NonConstRefType = + referenceType(pointee(unless(isConstQualified()))); + const auto NonConstRefParam = forEachArgumentWithParam( + equalsNode(&Exp), parmVarDecl(hasType(qualType(NonConstRefType)))); + const auto AsNonConstRefArg = anyOf( + callExpr(NonConstRefParam), cxxConstructExpr(NonConstRefParam)); + + // Taking address of 'Exp'. + const auto AsAmpersandOperand = + unaryOperator(hasOperatorName("&"), hasUnaryOperand(equalsNode(&Exp))); + const auto AsPointerFromArrayDecay = + castExpr(hasCastKind(CK_ArrayToPointerDecay), has(equalsNode(&Exp))); + + // Check whether 'Exp' is directly modified as a whole. + MatchFinder Finder; + bool HasMatch = false; + MatchCallbackAdaptor Callback( + [&HasMatch](const MatchFinder::MatchResult&) { HasMatch = true; }); + Finder.addMatcher(findAll(expr(anyOf( + AsAssignmentLhs, AsIncDecOperand, + AsNonConstThis, AsNonConstRefArg, + AsAmpersandOperand, AsPointerFromArrayDecay))), + &Callback); + Finder.match(Stm, *Context); + if (HasMatch) return true; + + // Check whether any member of 'Exp' is modified. + const auto MemberExprs = match( + findAll(memberExpr(hasObjectExpression(equalsNode(&Exp))).bind("expr")), + Stm, *Context); + for (const auto& Node : MemberExprs) { + if (isModified(*Node.getNodeAs("expr"), Stm, Context)) return true; + } + + // If 'Exp' is bound to a non-const reference, check all declRefExpr to that. + const auto Decls = match( + stmt(forEachDescendant( + varDecl(hasType(referenceType(pointee(unless(isConstQualified())))), + hasInitializer(equalsNode(&Exp))) + .bind("decl"))), + Stm, *Context); + for (const auto& DeclNode : Decls) { + const auto Exprs = match( + findAll(declRefExpr(to(equalsNode(DeclNode.getNodeAs("decl")))) + .bind("expr")), + Stm, *Context); + for (const auto& ExprNode : Exprs) { + if (isModified(*ExprNode.getNodeAs("expr"), Stm, Context)) + return true; + } + } + + return false; +} + } // namespace utils } // namespace tidy } // namespace clang Index: docs/ReleaseNotes.rst =================================================================== --- docs/ReleaseNotes.rst +++ docs/ReleaseNotes.rst @@ -57,6 +57,11 @@ Improvements to clang-tidy -------------------------- +- New :doc:`readability-unmodified-non-const-variable + ` check + + Finds declarations of non-const variables that never get modified. + - New module `abseil` for checks related to the `Abseil `_ library. Index: docs/clang-tidy/checks/list.rst =================================================================== --- docs/clang-tidy/checks/list.rst +++ docs/clang-tidy/checks/list.rst @@ -91,8 +91,8 @@ cppcoreguidelines-pro-type-vararg cppcoreguidelines-slicing cppcoreguidelines-special-member-functions - fuchsia-header-anon-namespaces (redirects to google-build-namespaces) fuchsia-default-arguments + fuchsia-header-anon-namespaces (redirects to google-build-namespaces) fuchsia-multiple-inheritance fuchsia-overloaded-operator fuchsia-statically-constructed-objects @@ -229,4 +229,5 @@ readability-static-definition-in-anonymous-namespace readability-string-compare readability-uniqueptr-delete-release + readability-unmodified-non-const-variable zircon-temporary-objects Index: docs/clang-tidy/checks/readability-unmodified-non-const-variable.rst =================================================================== --- /dev/null +++ docs/clang-tidy/checks/readability-unmodified-non-const-variable.rst @@ -0,0 +1,15 @@ +.. title:: clang-tidy - readability-unmodified-non-const-variable + +readability-unmodified-non-const-variable +========================================= + +Finds declarations of non-const variables that never get modified. + +For example: + +.. code-block:: c++ + + int simple() { + int x = 10; // x is declared as non-const but is never modified. + return x + 1; + } Index: test/clang-tidy/readability-unmodified-non-const-variable.cpp =================================================================== --- /dev/null +++ test/clang-tidy/readability-unmodified-non-const-variable.cpp @@ -0,0 +1,47 @@ +// RUN: %check_clang_tidy %s readability-unmodified-non-const-variable %t + +template +struct pair { + T1 first; + T2 second; + + void set(T1 f, T2 s); +}; + +void touch(int&); +void touch(int*); + +int simple() { + int x = 10; + // CHECK-MESSAGES: [[@LINE-1]]:7: warning: declaring a non-const variable but never modified it [readability-unmodified-non-const-variable] + return x + 1; +} + +void modified1() { + int x = 10; + touch(x); +} + +void modified2() { + int x = 10; + touch(&x); +} + +int modified3() { + int x = 10; + x += 20; + return x; +} + +int callNonConstMember() { + pair p1; + p1.set(10, 20); + return p1.first + p1.second; +} + +int followNonConstReference() { + pair p1; + auto& p2 = p1; + p2.first = 10; + return p1.first; +}