Index: clang-tidy/readability/CMakeLists.txt =================================================================== --- clang-tidy/readability/CMakeLists.txt +++ clang-tidy/readability/CMakeLists.txt @@ -10,6 +10,7 @@ InconsistentDeclarationParameterNameCheck.cpp NamedParameterCheck.cpp NamespaceCommentCheck.cpp + NonConstParameterCheck.cpp ReadabilityTidyModule.cpp RedundantStringCStrCheck.cpp RedundantSmartptrGetCheck.cpp Index: clang-tidy/readability/NonConstParameterCheck.h =================================================================== --- clang-tidy/readability/NonConstParameterCheck.h +++ clang-tidy/readability/NonConstParameterCheck.h @@ -0,0 +1,60 @@ +//===--- NonConstParameterCheck.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_NON_CONST_PARAMETER_H +#define LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_READABILITY_NON_CONST_PARAMETER_H + +#include "../ClangTidy.h" +#include + +namespace clang { +namespace tidy { + +/// Warn when a pointer function parameter can be const. +/// +/// For the user-facing documentation see: +/// http://clang.llvm.org/extra/clang-tidy/checks/readability-non-const-parameter.html +class NonConstParameterCheck : public ClangTidyCheck { +public: + NonConstParameterCheck(StringRef Name, ClangTidyContext *Context) + : ClangTidyCheck(Name, Context) {} + void registerMatchers(ast_matchers::MatchFinder *Finder) override; + void check(const ast_matchers::MatchFinder::MatchResult &Result) override; + void onEndOfTranslationUnit() override; + +private: + /// Parameter info. + struct ParmInfo { + /// Is function parameter referenced? + bool IsReferenced; + + /// Can function parameter be const? + bool CanBeConst; + }; + + /// Track all nonconst integer/float parameters. + std::map Parameters; + + /// Add function parameter. + void addParm(const ParmVarDecl *Parm); + + /// Set IsReferenced. + void setReferenced(const DeclRefExpr *Ref); + + /// Set CanNotBeConst. + void markCanNotBeConst(const Expr *E, bool Addr); + + /// Diagnose non const parameters. + void diagnoseNonConstParameters(); +}; + +} // namespace tidy +} // namespace clang + +#endif // LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_READABILITY_NON_CONST_PARAMETER_H Index: clang-tidy/readability/NonConstParameterCheck.cpp =================================================================== --- clang-tidy/readability/NonConstParameterCheck.cpp +++ clang-tidy/readability/NonConstParameterCheck.cpp @@ -0,0 +1,164 @@ +//===--- NonConstParameterCheck.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 "NonConstParameterCheck.h" +#include "clang/AST/ASTContext.h" +#include "clang/ASTMatchers/ASTMatchFinder.h" + +using namespace clang::ast_matchers; + +namespace clang { +namespace tidy { + +void NonConstParameterCheck::registerMatchers(MatchFinder *Finder) { + // Add parameters to Parameters. + Finder->addMatcher(parmVarDecl(unless(isInstantiated())).bind("parm"), this); + + // Track unused parameters, there is Wunused-parameter about unused + // parameters. + Finder->addMatcher(declRefExpr().bind("ref"), this); + + // Analyse parameter usage in function. + Finder->addMatcher(stmt(anyOf(unaryOperator(anyOf(hasOperatorName("++"), + hasOperatorName("--"))), + binaryOperator(), callExpr(), returnStmt())) + .bind("mark"), + this); + Finder->addMatcher(varDecl(hasInitializer(anything())).bind("mark"), this); +} + +void NonConstParameterCheck::check(const MatchFinder::MatchResult &Result) { + if (const auto *Parm = Result.Nodes.getNodeAs("parm")) { + addParm(Parm); + } else if (const auto *Ref = Result.Nodes.getNodeAs("ref")) { + setReferenced(Ref); + } else if (const auto *S = Result.Nodes.getNodeAs("mark")) { + if (const auto *B = dyn_cast(S)) { + if (B->isAssignmentOp()) + markCanNotBeConst(B, false); + } else if (const auto *CE = dyn_cast(S)) { + // TODO: Handle function calls better. Typically, if a parameter is + // const then markCanNotBeConst() should not be called. + for (const auto *Arg : CE->arguments()) { + markCanNotBeConst(Arg->IgnoreParenCasts(), true); + } + } else if (const auto *R = dyn_cast(S)) { + markCanNotBeConst(R->getRetValue(), true); + } else if (const auto *U = dyn_cast(S)) { + markCanNotBeConst(U, true); + } + } else if (const auto *VD = Result.Nodes.getNodeAs("mark")) { + const Type *T = VD->getType().getTypePtr(); + if (T && T->isPointerType() && !T->getPointeeType().isConstQualified()) + markCanNotBeConst(VD->getInit(), true); + else if (T && T->isArrayType()) + markCanNotBeConst(VD->getInit(), true); + } +} + +void NonConstParameterCheck::addParm(const ParmVarDecl *Parm) { + // Only add nonconst integer/float pointer parameters. + const Type *T = Parm->getType().getTypePtr(); + if (!T->isPointerType() || T->getPointeeType().isConstQualified() || + !(T->getPointeeType().getTypePtr()->isIntegerType() || + T->getPointeeType().getTypePtr()->isFloatingType())) + return; + + ParmInfo PI; + PI.IsReferenced = false; + PI.CanBeConst = true; + Parameters[Parm] = PI; +} + +void NonConstParameterCheck::setReferenced(const DeclRefExpr *Ref) { + auto It = Parameters.find(dyn_cast(Ref->getDecl())); + if (It != Parameters.end()) + It->second.IsReferenced = true; +} + +void NonConstParameterCheck::onEndOfTranslationUnit() { + diagnoseNonConstParameters(); +} + +void NonConstParameterCheck::diagnoseNonConstParameters() { + for (auto It : Parameters) { + const ParmVarDecl *Par = It.first; + const ParmInfo &ParamInfo = It.second; + + // Unused parameter => there are other warnings about this. + if (!ParamInfo.IsReferenced) + continue; + + // Parameter can't be const. + if (!ParamInfo.CanBeConst) + continue; + + diag(Par->getLocation(), "parameter '%0' can be const") + << Par->getName() + << FixItHint::CreateInsertion(Par->getLocStart(), "const "); + } +} + +void NonConstParameterCheck::markCanNotBeConst(const Expr *E, bool Addr) { + if (!E) + return; + + if (const auto *Cast = dyn_cast(E)) { + // If expression is const then ignore usage. + const Type *T = Cast->getType().getTypePtr(); + if (T && T->isPointerType() && T->getPointeeType().isConstQualified()) + return; + } + + E = E->IgnoreParenCasts(); + + if (const auto *B = dyn_cast(E)) { + if (B->isAdditiveOp()) { + // p + 2 + markCanNotBeConst(B->getLHS(), Addr); + markCanNotBeConst(B->getRHS(), Addr); + } else if (B->isAssignmentOp()) { + markCanNotBeConst(B->getLHS(), false); + + // If LHS is not const then RHS can't be const. + const Type *T = B->getLHS()->getType().getTypePtr(); + if (T && T->isPointerType() && !T->getPointeeType().isConstQualified()) + markCanNotBeConst(B->getRHS(), true); + } + } else if (const auto *C = dyn_cast(E)) { + markCanNotBeConst(C->getTrueExpr(), Addr); + markCanNotBeConst(C->getFalseExpr(), Addr); + } else if (const auto *U = dyn_cast(E)) { + if (Addr && U->getOpcode() == UO_Deref) + return; + if (U->getOpcode() == UO_PreInc || U->getOpcode() == UO_PreDec || + U->getOpcode() == UO_PreInc || U->getOpcode() == UO_PreDec) { + markCanNotBeConst(U->getSubExpr(), false); + } else { + markCanNotBeConst(U->getSubExpr(), true); + } + } else if (const auto *A = dyn_cast(E)) { + markCanNotBeConst(A->getBase(), true); + } else if (const auto *CLE = dyn_cast(E)) { + markCanNotBeConst(CLE->getInitializer(), true); + } else if (const auto *ILE = dyn_cast(E)) { + for (unsigned I = 0U; I < ILE->getNumInits(); ++I) + markCanNotBeConst(ILE->getInit(I), true); + } else if (Addr) { + // Referencing parameter. + if (const auto *D = dyn_cast(E)) { + auto It = Parameters.find(dyn_cast(D->getDecl())); + if (It != Parameters.end()) + It->second.CanBeConst = false; + } + } +} + +} // namespace tidy +} // namespace clang Index: clang-tidy/readability/ReadabilityTidyModule.cpp =================================================================== --- clang-tidy/readability/ReadabilityTidyModule.cpp +++ clang-tidy/readability/ReadabilityTidyModule.cpp @@ -18,6 +18,7 @@ #include "ImplicitBoolCastCheck.h" #include "InconsistentDeclarationParameterNameCheck.h" #include "NamedParameterCheck.h" +#include "NonConstParameterCheck.h" #include "RedundantSmartptrGetCheck.h" #include "RedundantStringCStrCheck.h" #include "SimplifyBooleanExprCheck.h" @@ -48,6 +49,8 @@ "readability-uniqueptr-delete-release"); CheckFactories.registerCheck( "readability-named-parameter"); + CheckFactories.registerCheck( + "readability-non-const-parameter"); CheckFactories.registerCheck( "readability-redundant-smartptr-get"); CheckFactories.registerCheck( Index: docs/clang-tidy/checks/list.rst =================================================================== --- docs/clang-tidy/checks/list.rst +++ docs/clang-tidy/checks/list.rst @@ -71,6 +71,7 @@ readability-implicit-bool-cast readability-inconsistent-declaration-parameter-name readability-named-parameter + readability-non-const-parameter readability-redundant-smartptr-get readability-redundant-string-cstr readability-simplify-boolean-expr Index: docs/clang-tidy/checks/readability-non-const-parameter.rst =================================================================== --- docs/clang-tidy/checks/readability-non-const-parameter.rst +++ docs/clang-tidy/checks/readability-non-const-parameter.rst @@ -0,0 +1,37 @@ +readability-non-const-parameter +=============================== + +Finds function parameters that should be const. When const is used properly, +many mistakes can be avoided. Advantages when using const properly: + * avoid that data is unintentionally modified + * get additional warnings such as using uninitialized data + * easier for developers to see possible side effects + +clang-tidy is not strict about constness, it only warns when the constness will +make the function interface safer. + + // warning here; p should be const + char f1(char *p) { + return *p; + } + + // no warning; the declaration could be more const "const int * const p" but + // that does not make the interface safer. + int f2(const int *p) { + return *p; + } + + // no warning; making x const does not make the function interface safer + int f3(int x) { + return x; + } + + // no warning; Technically, p can be const ("const struct S *p"). But making + // p const could be misleading. People might think that it's safe to pass + // const data to this function. + struct S { int *a; int *b; }; + int f3(struct S *p) { + *(p->a) = 0; + } + + Index: test/clang-tidy/readability-non-const-parameter.cpp =================================================================== --- test/clang-tidy/readability-non-const-parameter.cpp +++ test/clang-tidy/readability-non-const-parameter.cpp @@ -0,0 +1,211 @@ +// RUN: %check_clang_tidy %s readability-non-const-parameter %t + +// Currently the checker only warns about pointer arguments. +// +// It can be defined both that the data is const and that the pointer is const, +// the checker only checks if the data can be const-specified. +// +// It does not warn about pointers to records or function pointers. + +// Some external function where first argument is nonconst and second is const. +char *strcpy1(char *dest, const char *src); +unsigned my_strcpy(char *buf, const char *s); +unsigned my_strlen(const char *buf); + +// CHECK-MESSAGES: :[[@LINE+1]]:29: warning: parameter 'last' can be const [readability-non-const-parameter] +void warn1(int *first, int *last) { + // CHECK-FIXES: {{^}}void warn1(int *first, const int *last) {{{$}} + *first = 0; + if (first < last) { + } // <- last can be const +} + +// TODO: warning should be written here +void warn2(char *p) { + char buf[10]; + strcpy1(buf, p); +} + +// CHECK-MESSAGES: :[[@LINE+1]]:19: warning: parameter 'p' can be const [readability-non-const-parameter] +void assign1(int *p) { + // CHECK-FIXES: {{^}}void assign1(const int *p) {{{$}} + const int *q; + q = p; +} + +// CHECK-MESSAGES: :[[@LINE+1]]:19: warning: parameter 'p' can be const [readability-non-const-parameter] +void assign2(int *p) { + // CHECK-FIXES: {{^}}void assign2(const int *p) {{{$}} + const int *q; + q = p + 1; +} + +void assign3(int *p) { + *p = 0; +} + +void assign4(int *p) { + *p += 2; +} + +void assign5(char *p) { + p[0] = 0; +} + +void assign6(int *p) { + int *q; + q = p++; +} + +void assign7(char *p) { + char *a, *b; + a = b = p; +} + +void assign8(char *a, char *b) { + char *x; + x = (a ? a : b); +} + +void assign9(unsigned char *str, const unsigned int i) { + unsigned char *p; + for (p = str + i; *p;) { + } +} + +void assign10(int *buf) { + int i, *p; + for (i = 0, p = buf; i < 10; i++, p++) { + *p = 1; + } +} + +// CHECK-MESSAGES: :[[@LINE+1]]:17: warning: parameter 'p' can be const [readability-non-const-parameter] +void init1(int *p) { + // CHECK-FIXES: {{^}}void init1(const int *p) {{{$}} + const int *q = p; +} + +// CHECK-MESSAGES: :[[@LINE+1]]:17: warning: parameter 'p' can be const [readability-non-const-parameter] +void init2(int *p) { + // CHECK-FIXES: {{^}}void init2(const int *p) {{{$}} + const int *q = p + 1; +} + +void init3(int *p) { + int *q = p; +} + +void init4(float *p) { + int *q = (int *)p; +} + +void init5(int *p) { + int *i = p ? p : 0; +} + +void init6(int *p) { + int *a[] = {p, p, 0}; +} + +void init7(int *p, int x) { + for (int *q = p + x - 1; 0; q++) + ; +} + +// CHECK-MESSAGES: :[[@LINE+1]]:18: warning: parameter 'p' can be const [readability-non-const-parameter] +int return1(int *p) { + // CHECK-FIXES: {{^}}int return1(const int *p) {{{$}} + return *p; +} + +// CHECK-MESSAGES: :[[@LINE+1]]:25: warning: parameter 'p' can be const [readability-non-const-parameter] +const int *return2(int *p) { + // CHECK-FIXES: {{^}}const int *return2(const int *p) {{{$}} + return p; +} + +// CHECK-MESSAGES: :[[@LINE+1]]:25: warning: parameter 'p' can be const [readability-non-const-parameter] +const int *return3(int *p) { + // CHECK-FIXES: {{^}}const int *return3(const int *p) {{{$}} + return p + 1; +} + +// CHECK-MESSAGES: :[[@LINE+1]]:27: warning: parameter 'p' can be const [readability-non-const-parameter] +const char *return4(char *p) { + // CHECK-FIXES: {{^}}const char *return4(const char *p) {{{$}} + return p ? p : ""; +} + +char *return5(char *s) { + return s; +} + +char *return6(char *s) { + return s + 1; +} + +char *return7(char *a, char *b) { + return a ? a : b; +} + +char return8(int *p) { + return ++(*p); +} + +void dontwarn1(int *p) { + ++(*p); +} + +int dontwarn2(_Atomic(int) * p) { + return *p; +} + +void callFunction1(char *p) { + strcpy1(p, "abc"); +} + +void callFunction2(char *p) { + strcpy1(&p[0], "abc"); +} + +void callFunction3(char *p) { + strcpy1(p + 2, "abc"); +} + +char *callFunction4(char *p) { + return strcpy1(p, "abc"); +} + +unsigned callFunction5(char *buf) { + unsigned len = my_strlen(buf); + return len + my_strcpy(buf, "abc"); +} + +void f6(int **p); +void callFunction6(int *p) { f6(&p); } + +typedef union { void *v; } t; +void f7(t obj); +void callFunction7(int *p) { + f7((t){p}); +} + +// Don't warn about nonconst function pointers that can be const. +void functionpointer(double f(double), int x) { + f(x); +} + +// Don't warn about nonconst record pointers that can be const. +struct XY { + int *x; + int *y; +}; +void recordpointer(struct XY *xy) { + *(xy->x) = 0; +} + +// avoid fp for const pointer array +void constPointerArray(const char *remapped[][2]) { + const char *name = remapped[0][0]; +}