alexfh added inline comments. ================ Comment at: clang-tidy/readability/NonConstParameterCheck.cpp:44 @@ +43,3 @@ + addParm(Parm); + } else if (const CXXConstructorDecl *Ctor = + Result.Nodes.getNodeAs<CXXConstructorDecl>("Ctor")) { ---------------- `const auto *Ctor`
================ Comment at: docs/clang-tidy/checks/readability-non-const-parameter.rst:7 @@ +6,3 @@ +Finds function pointer 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 ---------------- That now sounds ambiguous (is it about parameters of a pointer to a function type?). I'd say "The check finds function parameters of a pointer type that could be changed to point to a constant type instead." or something similar. ================ Comment at: docs/clang-tidy/checks/readability-non-const-parameter.rst:8 @@ +7,3 @@ +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 ---------------- "prevent unintentional modification of data" ================ Comment at: docs/clang-tidy/checks/readability-non-const-parameter.rst:10 @@ +9,3 @@ + * get additional warnings such as using uninitialized data + * easier for developers to see possible side effects + ---------------- "make it easier for developers ..." ================ Comment at: docs/clang-tidy/checks/readability-non-const-parameter.rst:12 @@ +11,3 @@ + +clang-tidy is not strict about constness, it only warns when the constness will +make the function interface safer. ---------------- s/clang-tidy/This check/ ================ Comment at: test/clang-tidy/readability-non-const-parameter.cpp:244 @@ +243,3 @@ + // CHECK-MESSAGES: :[[@LINE+1]]:21: warning: parameter 'p' can be const + void doStuff(int *p) { + // CHECK-FIXES: {{^}} void doStuff(const int *p) {{{$}} ---------------- Please add a test to ensure that overridden methods are not modified (since you would need to update the base class' method and then all the other overrides. ================ Comment at: test/clang-tidy/readability-non-const-parameter.cpp:251 @@ +250,2 @@ +}; + ---------------- The check can break code taking a pointer to a function it modifies: typedef int (*fn)(int *); int f(int *p) {return *p; } fn fp = &f; It's not always possible to detect, when this happens (for example, a pointer to the function could be taken in a different translation unit), but we should at least keep this case in mind. Please add a test case for this. http://reviews.llvm.org/D15332 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits