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,54 @@ +//===--- 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 "llvm/ADT/SmallVector.h" + +namespace clang { +namespace tidy { + +struct ParInfo { + /// Function parameter + const ParmVarDecl *Par; + + /// Is function parameter Referenced? + bool IsReferenced; + + /// Can function parameter be const? + bool CanBeConst; +}; + +/// 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: + SmallVector Params; + void addPar(const ParmVarDecl *Par); + void ref(const DeclRefExpr *Ref); + void markCanNotBeConst(const Expr *E, bool Addr); + struct ParInfo *getParInfo(const Decl *D); + 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,177 @@ +//===--- 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) { + // TODO: This checker doesn't handle C++ to start with. There are problems + // for example with parameters to template functions. + if (getLangOpts().CPlusPlus) + return; + + // Function declaration. Diagnose nonconst parameters and clear Params. + Finder->addMatcher(functionDecl().bind("function"), this); + + // Add parameters to Params. + Finder->addMatcher(parmVarDecl().bind("par"), this); + + // Analyse parameter usage in function. + Finder->addMatcher(declRefExpr().bind("ref"), this); + Finder->addMatcher(binaryOperator(anything()).bind("assign"), this); + Finder->addMatcher( + unaryOperator(anyOf(hasOperatorName("++"), hasOperatorName("--")), + has(unaryOperator(hasOperatorName("*")))) + .bind("incdec"), + this); + Finder->addMatcher(varDecl(hasInitializer(anything())).bind("variable"), + this); + Finder->addMatcher(callExpr().bind("call"), this); + Finder->addMatcher(returnStmt().bind("return"), this); +} + +void NonConstParameterCheck::check(const MatchFinder::MatchResult &Result) { + if (Result.Nodes.getNodeAs("function")) { + diagnoseNonConstParameters(); + Params.clear(); + } else if (auto *Par = Result.Nodes.getNodeAs("par")) { + addPar(Par); + } else if (auto *Ref = Result.Nodes.getNodeAs("ref")) { + ref(Ref); + } else if (auto *Assign = Result.Nodes.getNodeAs("assign")) { + if (Assign->isAssignmentOp()) + markCanNotBeConst(Assign, false); + } else if (auto *VD = Result.Nodes.getNodeAs("variable")) { + 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); + } else if (auto *CE = Result.Nodes.getNodeAs("call")) { + // TODO: Handle function calls better + for (auto *Arg : CE->arguments()) { + markCanNotBeConst(Arg->IgnoreParenCasts(), true); + } + } else if (auto *U = Result.Nodes.getNodeAs("incdec")) { + markCanNotBeConst(U, false); + } else if (auto *R = Result.Nodes.getNodeAs("return")) { + markCanNotBeConst(R->getRetValue(), true); + } +} + +void NonConstParameterCheck::addPar(const ParmVarDecl *Par) { + struct ParInfo PI; + PI.Par = Par; + PI.IsReferenced = false; + PI.CanBeConst = true; + Params.push_back(PI); +} + +void NonConstParameterCheck::ref(const DeclRefExpr *Ref) { + struct ParInfo *PI = getParInfo(Ref->getDecl()); + if (PI) + PI->IsReferenced = true; +} + +struct ParInfo *NonConstParameterCheck::getParInfo(const Decl *D) { + for (struct ParInfo &ParamInfo : Params) { + if (ParamInfo.Par == D) + return &ParamInfo; + } + return nullptr; +} + +void NonConstParameterCheck::onEndOfTranslationUnit() { + diagnoseNonConstParameters(); +} + +void NonConstParameterCheck::diagnoseNonConstParameters() { + for (struct ParInfo &ParamInfo : Params) { + // Unused parameter => there are other warnings about this. + if (!ParamInfo.IsReferenced) + continue; + + // Parameter can't be const. + if (!ParamInfo.CanBeConst) + continue; + + // Parameter is not a nonconst integer/float pointer. + const Type *T = ParamInfo.Par->getType().getTypePtr(); + if (!T->isPointerType() || T->getPointeeType().isConstQualified() || + !(T->getPointeeType().getTypePtr()->isIntegerType() || + T->getPointeeType().getTypePtr()->isFloatingType())) + continue; + + diag(ParamInfo.Par->getLocation(), "parameter '%0' can be const") + << ParamInfo.Par->getName() + << FixItHint::CreateInsertion(ParamInfo.Par->getLocStart(), "const "); + } +} + +void NonConstParameterCheck::markCanNotBeConst(const Expr *E, bool Addr) { + if (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 (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 (auto *C = dyn_cast(E)) { + markCanNotBeConst(C->getTrueExpr(), Addr); + markCanNotBeConst(C->getFalseExpr(), Addr); + } else if (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 (auto *A = dyn_cast(E)) { + markCanNotBeConst(A->getBase(), true); + } else if (auto *CLE = dyn_cast(E)) { + markCanNotBeConst(CLE->getInitializer(), true); + } else if (auto *ILE = dyn_cast(E)) { + for (unsigned I = 0U; I < ILE->getNumInits(); ++I) + markCanNotBeConst(ILE->getInit(I), true); + } else if (Addr) { + // Referencing parameter. + if (auto *D = dyn_cast(E)) { + if (auto *Par = dyn_cast(D->getDecl())) { + struct ParInfo *PI = getParInfo(Par); + if (PI) + PI->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,41 @@ +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.c =================================================================== --- test/clang-tidy/readability-non-const-parameter.c +++ test/clang-tidy/readability-non-const-parameter.c @@ -0,0 +1,215 @@ +// 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]; +} +