danielmarjamaki created this revision.
danielmarjamaki added reviewers: alexfh, aaron.ballman, rsmith.
danielmarjamaki added subscribers: cfe-commits, sberg.
This is a new clang-tidy checker that will warn when function parameters should
be const.
See also related discussions in http://reviews.llvm.org/D12359
As usual, I am testing this checker on the debian projects. I have so far
triaged 20 warnings from this clang-tidy checker and they were all true
positives.
http://reviews.llvm.org/D15332
Files:
clang-tidy/readability/CMakeLists.txt
clang-tidy/readability/NonConstParameterCheck.cpp
clang-tidy/readability/NonConstParameterCheck.h
clang-tidy/readability/ReadabilityTidyModule.cpp
docs/clang-tidy/checks/list.rst
docs/clang-tidy/checks/readability-non-const-parameter.rst
test/clang-tidy/readability-non-const-parameter.c
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];
+}
+
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: 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: 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::NamedParameterCheck>(
"readability-named-parameter");
+ CheckFactories.registerCheck<NonConstParameterCheck>(
+ "readability-non-const-parameter");
CheckFactories.registerCheck<RedundantSmartptrGetCheck>(
"readability-redundant-smartptr-get");
CheckFactories.registerCheck<RedundantStringCStrCheck>(
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<struct ParInfo, 5> 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<FunctionDecl>("function")) {
+ diagnoseNonConstParameters();
+ Params.clear();
+ } else if (auto *Par = Result.Nodes.getNodeAs<ParmVarDecl>("par")) {
+ addPar(Par);
+ } else if (auto *Ref = Result.Nodes.getNodeAs<DeclRefExpr>("ref")) {
+ ref(Ref);
+ } else if (auto *Assign = Result.Nodes.getNodeAs<BinaryOperator>("assign")) {
+ if (Assign->isAssignmentOp())
+ markCanNotBeConst(Assign, false);
+ } else if (auto *VD = Result.Nodes.getNodeAs<VarDecl>("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<CallExpr>("call")) {
+ // TODO: Handle function calls better
+ for (auto *Arg : CE->arguments()) {
+ markCanNotBeConst(Arg->IgnoreParenCasts(), true);
+ }
+ } else if (auto *U = Result.Nodes.getNodeAs<UnaryOperator>("incdec")) {
+ markCanNotBeConst(U, false);
+ } else if (auto *R = Result.Nodes.getNodeAs<ReturnStmt>("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<ImplicitCastExpr>(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<BinaryOperator>(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<ConditionalOperator>(E)) {
+ markCanNotBeConst(C->getTrueExpr(), Addr);
+ markCanNotBeConst(C->getFalseExpr(), Addr);
+ } else if (auto *U = dyn_cast<UnaryOperator>(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<ArraySubscriptExpr>(E)) {
+ markCanNotBeConst(A->getBase(), true);
+ } else if (auto *CLE = dyn_cast<CompoundLiteralExpr>(E)) {
+ markCanNotBeConst(CLE->getInitializer(), true);
+ } else if (auto *ILE = dyn_cast<InitListExpr>(E)) {
+ for (unsigned I = 0U; I < ILE->getNumInits(); ++I)
+ markCanNotBeConst(ILE->getInit(I), true);
+ } else if (Addr) {
+ // Referencing parameter.
+ if (auto *D = dyn_cast<DeclRefExpr>(E)) {
+ if (auto *Par = dyn_cast<ParmVarDecl>(D->getDecl())) {
+ struct ParInfo *PI = getParInfo(Par);
+ if (PI)
+ PI->CanBeConst = false;
+ }
+ }
+ }
+}
+
+} // namespace tidy
+} // namespace clang
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
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
