shuaiwang created this revision.
Herald added subscribers: cfe-commits, mgorny, klimek.
shuaiwang added a reviewer: hokein.
Repository:
rCTE Clang Tools Extra
https://reviews.llvm.org/D45679
Files:
clang-tidy/readability/CMakeLists.txt
clang-tidy/readability/ReadabilityTidyModule.cpp
clang-tidy/readability/UnmodifiedNonConstVariableCheck.cpp
clang-tidy/readability/UnmodifiedNonConstVariableCheck.h
clang-tidy/utils/ASTUtils.cpp
clang-tidy/utils/ASTUtils.h
docs/ReleaseNotes.rst
docs/clang-tidy/checks/list.rst
docs/clang-tidy/checks/readability-unmodified-non-const-variable.rst
test/clang-tidy/readability-unmodified-non-const-variable.cpp
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 <class T1, class T2>
+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<int, int> p1;
+ p1.set(10, 20);
+ return p1.first + p1.second;
+}
+
+int followNonConstReference() {
+ pair<int, int> p1;
+ auto& p2 = p1;
+ p2.first = 10;
+ return p1.first;
+}
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: 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-header-anon-namespaces>
fuchsia-default-arguments
+ fuchsia-header-anon-namespaces (redirects to google-build-namespaces) <fuchsia-header-anon-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/ReleaseNotes.rst
===================================================================
--- docs/ReleaseNotes.rst
+++ docs/ReleaseNotes.rst
@@ -57,6 +57,11 @@
Improvements to clang-tidy
--------------------------
+- New :doc:`readability-unmodified-non-const-variable
+ <clang-tidy/checks/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 <https://abseil.io>`_
library.
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<void(const MatchFinder::MatchResult &)> Func)
+ : Func(std::move(Func)) {}
+ void run(const MatchFinder::MatchResult &Result) override { Func(Result); }
+
+private:
+ std::function<void(const MatchFinder::MatchResult &)> 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>("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>("decl"))))
+ .bind("expr")),
+ Stm, *Context);
+ for (const auto& ExprNode : Exprs) {
+ if (isModified(*ExprNode.getNodeAs<Expr>("expr"), Stm, Context))
+ return true;
+ }
+ }
+
+ return false;
+}
+
} // namespace utils
} // namespace tidy
} // namespace clang
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<VarDecl>("decl");
+ if (Decl->Decl::getLocStart().isMacroID())
+ return;
+ const auto *Compound = Result.Nodes.getNodeAs<Stmt>("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>("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/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<StringCompareCheck>(
"readability-string-compare");
+ CheckFactories.registerCheck<UnmodifiedNonConstVariableCheck>(
+ "readability-unmodified-non-const-variable");
CheckFactories.registerCheck<readability::NamedParameterCheck>(
"readability-named-parameter");
CheckFactories.registerCheck<NonConstParameterCheck>(
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
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
