JonasToth updated this revision to Diff 142521. JonasToth added a comment. - [Feature] implement array and method access - [Feature] finalize value semantic
Repository: rCTE Clang Tools Extra https://reviews.llvm.org/D45444 Files: clang-tidy/cppcoreguidelines/CMakeLists.txt clang-tidy/cppcoreguidelines/ConstCheck.cpp clang-tidy/cppcoreguidelines/ConstCheck.h clang-tidy/cppcoreguidelines/CppCoreGuidelinesTidyModule.cpp docs/ReleaseNotes.rst docs/clang-tidy/checks/cppcoreguidelines-const.rst docs/clang-tidy/checks/list.rst test/clang-tidy/cppcoreguidelines-const-primtive-builtin.cpp
Index: test/clang-tidy/cppcoreguidelines-const-primtive-builtin.cpp =================================================================== --- /dev/null +++ test/clang-tidy/cppcoreguidelines-const-primtive-builtin.cpp @@ -0,0 +1,387 @@ +// RUN: %check_clang_tidy %s cppcoreguidelines-const %t + +// ------- Provide test samples for primitive builtins --------- +// - every 'p_*' variable is a 'potential_const_*' variable +// - every 'np_*' variable is a 'non_potential_const_*' variable + +bool global; +char np_global = 0; // globals can't be known to be const + +namespace foo { +int scoped; +float np_scoped = 1; // namespace variables are like globals +} // namespace foo + +void some_function(double, wchar_t); + +void some_function(double np_arg0, wchar_t np_arg1) { + int p_local0 = 2; + // CHECK-MESSAGES: [[@LINE-1]]:3: warning: variable 'p_local0' of type 'int' can be declared const + + int np_local0; + const int np_local1 = 42; + + unsigned int np_local2 = 3; + np_local2 <<= 4; + + int np_local3 = 4; + ++np_local3; + int np_local4 = 4; + np_local4++; + + int np_local5 = 4; + --np_local5; + int np_local6 = 4; + np_local6--; +} + +void some_lambda_environment_capture_all_by_reference(double np_arg0) { + int np_local0 = 0; + int p_local0 = 1; + // CHECK-MESSAGES: [[@LINE-1]]:3: warning: variable 'p_local0' of type 'int' can be declared const + + int np_local2; + const int np_local3 = 2; + + // Capturing all variables by reference prohibits making them const. + [&]() { ++np_local0; }; + + int p_local1 = 0; + // CHECK-MESSAGES: [[@LINE-1]]:3: warning: variable 'p_local1' of type 'int' can be declared const +} + +void some_lambda_environment_capture_all_by_value(double np_arg0) { + int np_local0 = 0; + int p_local0 = 1; + // CHECK-MESSAGES: [[@LINE-1]]:3: warning: variable 'p_local0' of type 'int' can be declared const + + int np_local1; + const int np_local2 = 2; + + // Capturing by value has no influence on them. + [=]() { (void)p_local0; }; + + np_local0 += 10; +} + +void function_inout_pointer(int *inout); +void function_in_pointer(const int *in); + +void some_pointer_taking(int *out) { + int np_local0 = 42; + const int *const p0_np_local0 = &np_local0; + int *const p1_np_local0 = &np_local0; + + int np_local1 = 42; + function_inout_pointer(&np_local1); + + // Prevents const. + int np_local2 = 42; + out = &np_local2; // This returns and invalid address, its just about the AST + + int p_local0 = 42; + // CHECK-MESSAGES: [[@LINE-1]]:3: warning: variable 'p_local0' of type 'int' can be declared const + const int *const p0_p_local0 = &p_local0; + + int p_local1 = 42; + // CHECK-MESSAGES: [[@LINE-1]]:3: warning: variable 'p_local1' of type 'int' can be declared const + function_in_pointer(&p_local1); +} + +void function_inout_ref(int &inout); +void function_in_ref(const int &in); + +void some_reference_taking() { + int np_local0 = 42; + const int &r0_np_local0 = np_local0; + int &r1_np_local0 = np_local0; + const int &r2_np_local0 = r1_np_local0; + + int np_local1 = 42; + function_inout_ref(np_local1); + + int p_local0 = 42; + // CHECK-MESSAGES: [[@LINE-1]]:3: warning: variable 'p_local0' of type 'int' can be declared const + const int &r0_p_local0 = p_local0; + + int p_local1 = 42; + // CHECK-MESSAGES: [[@LINE-1]]:3: warning: variable 'p_local1' of type 'int' can be declared const + function_in_ref(p_local1); +} + +double *non_const_pointer_return() { + double p_local0 = 0.0; + // CHECK-MESSAGES: [[@LINE-1]]:3: warning: variable 'p_local0' of type 'double' can be declared const + double np_local0 = 24.4; + + return &np_local0; +} + +const double *const_pointer_return() { + double p_local0 = 0.0; + // CHECK-MESSAGES: [[@LINE-1]]:3: warning: variable 'p_local0' of type 'double' can be declared const + double p_local1 = 24.4; + // CHECK-MESSAGES: [[@LINE-1]]:3: warning: variable 'p_local1' of type 'double' can be declared const + return &p_local1; +} + +double &non_const_ref_return() { + double p_local0 = 0.0; + // CHECK-MESSAGES: [[@LINE-1]]:3: warning: variable 'p_local0' of type 'double' can be declared const + double np_local0 = 42.42; + return np_local0; +} + +const double &const_ref_return() { + double p_local0 = 0.0; + // CHECK-MESSAGES: [[@LINE-1]]:3: warning: variable 'p_local0' of type 'double' can be declared const + double p_local1 = 24.4; + // CHECK-MESSAGES: [[@LINE-1]]:3: warning: variable 'p_local1' of type 'double' can be declared const + return p_local1; +} + +double *&return_non_const_pointer_ref() { + double *np_local0 = nullptr; + return np_local0; +} + +void overloaded_arguments(const int &in); +void overloaded_arguments(int &inout); +void overloaded_arguments(const int *in); +void overloaded_arguments(int *inout); + +void function_calling() { + int np_local0 = 42; + overloaded_arguments(np_local0); + + const int np_local1 = 42; + overloaded_arguments(np_local1); + + int np_local2 = 42; + overloaded_arguments(&np_local2); + + const int np_local3 = 42; + overloaded_arguments(&np_local3); +} + +template <typename T> +void define_locals(T np_arg0, T &np_arg1, int np_arg2) { + T np_local0 = 0; + np_local0 += np_arg0 * np_arg1; + + T np_local1 = 42; + np_local0 += np_local1; + + // Used as argument to an overloaded function with const and non-const. + T np_local2 = 42; + overloaded_arguments(np_local2); + + int np_local4 = 42; + // non-template values are ok still. + int p_local0 = 42; + // CHECK-MESSAGES: [[@LINE-1]]:3: warning: variable 'p_local0' of type 'int' can be declared const + np_local4 += p_local0; +} + +void template_instantiation() { + const int np_local0 = 42; + int np_local1 = 42; + + define_locals(np_local0, np_local1, np_local0); + define_locals(np_local1, np_local1, np_local1); +} + +struct ConstNonConstClass { + ConstNonConstClass(); + ConstNonConstClass(double &np_local0); + double nonConstMethod() {} + double constMethod() const {} + double modifyingMethod(double &np_arg0) const; +}; + +void direct_method_calls() { + ConstNonConstClass np_local0; + + np_local0.constMethod(); + np_local0.nonConstMethod(); + + ConstNonConstClass p_local0; + // CHECK-MESSAGES: [[@LINE-1]]:3: warning: variable 'p_local0' of type 'ConstNonConstClass' can be declared const + p_local0.constMethod(); + + ConstNonConstClass p_local1; + // CHECK-MESSAGES: [[@LINE-1]]:3: warning: variable 'p_local1' of type 'ConstNonConstClass' can be declared const + double np_local1; + p_local1.modifyingMethod(np_local1); + + double np_local2; + ConstNonConstClass p_local2(np_local2); + // CHECK-MESSAGES: [[@LINE-1]]:3: warning: variable 'p_local2' of type 'ConstNonConstClass' can be declared const +} + +struct OperatorsAsConstAsPossible { + OperatorsAsConstAsPossible &operator+=(const OperatorsAsConstAsPossible &rhs); + OperatorsAsConstAsPossible operator+(const OperatorsAsConstAsPossible &rhs) const; +}; + +struct NonConstOperators { +}; +NonConstOperators operator+(NonConstOperators &lhs, NonConstOperators &rhs); +NonConstOperators operator-(NonConstOperators lhs, NonConstOperators rhs); + +void internal_operator_calls() { + OperatorsAsConstAsPossible np_local0; + OperatorsAsConstAsPossible np_local1; + OperatorsAsConstAsPossible p_local0; + // CHECK-MESSAGES: [[@LINE-1]]:3: warning: variable 'p_local0' of type 'OperatorsAsConstAsPossible' can be declared const + OperatorsAsConstAsPossible p_local1; + // CHECK-MESSAGES: [[@LINE-1]]:3: warning: variable 'p_local1' of type 'OperatorsAsConstAsPossible' can be declared const + + np_local0 += p_local0; + np_local1 = p_local0 + p_local1; + + NonConstOperators np_local2; + NonConstOperators np_local3; + NonConstOperators np_local4; + + np_local2 = np_local3 + np_local4; + + NonConstOperators p_local2; + // CHECK-MESSAGES: [[@LINE-1]]:3: warning: variable 'p_local2' of type 'NonConstOperators' can be declared const + NonConstOperators p_local3 = p_local2 - p_local2; + // CHECK-MESSAGES: [[@LINE-1]]:3: warning: variable 'p_local3' of type 'NonConstOperators' can be declared const +} + +struct MyVector { + double *begin(); + const double *begin() const; + + double *end(); + const double *end() const; + + double &operator[](int index); + double operator[](int index) const; + + double values[100]; +}; + +void vector_usage() { + double np_local0[10]; + np_local0[5] = 42.; + + MyVector np_local1; + np_local1[5] = 42.; + + double p_local0[10] = {0., 1., 2., 3., 4., 5., 6., 7., 8., 9.}; + // CHECK-MESSAGES: [[@LINE-1]]:3: warning: variable 'p_local0' of type 'double [10]' can be declared const + double p_local1 = p_local0[5]; + // CHECK-MESSAGES: [[@LINE-1]]:3: warning: variable 'p_local1' of type 'double' can be declared const + + // The following subscript calls suprisingly choose the non-const operator + // version. + MyVector np_local2; + double p_local2 = np_local2[42]; + // CHECK-MESSAGES: [[@LINE-1]]:3: warning: variable 'p_local2' of type 'double' can be declared const + + MyVector np_local3; + const double np_local4 = np_local3[42]; + + // This subscript results in const overloaded operator. + const MyVector np_local5{}; + double p_local3 = np_local5[42]; + // CHECK-MESSAGES: [[@LINE-1]]:3: warning: variable 'p_local3' of type 'double' can be declared const +} + +void const_handle(const double &np_local0); +void const_handle(const double *np_local0); + +void non_const_handle(double &np_local0); +void non_const_handle(double *np_local0); + +void handle_from_array() { + // Non-const handle from non-const array forbids declaring the array as const + double np_local0[10] = {0., 1., 2., 3., 4., 5., 6., 7., 8., 9.}; + double *p_local0 = &np_local0[1]; + // CHECK-MESSAGES: [[@LINE-1]]:3: warning: variable 'p_local0' of type 'double *' can be declared const + + double np_local1[10] = {0., 1., 2., 3., 4., 5., 6., 7., 8., 9.}; + double &non_const_ref = np_local1[1]; + + double np_local2[10] = {0., 1., 2., 3., 4., 5., 6., 7., 8., 9.}; + double *np_local3; + np_local3 = &np_local2[5]; + + double np_local4[10] = {0., 1., 2., 3., 4., 5., 6., 7., 8., 9.}; + non_const_handle(np_local4[2]); + double np_local5[10] = {0., 1., 2., 3., 4., 5., 6., 7., 8., 9.}; + non_const_handle(&np_local5[2]); + + // Constant handles are ok + double p_local1[10] = {0., 1., 2., 3., 4., 5., 6., 7., 8., 9.}; + // CHECK-MESSAGES: [[@LINE-1]]:3: warning: variable 'p_local1' of type 'double [10]' can be declared const + const double *p_local2 = &p_local1[2]; + // CHECK-MESSAGES: [[@LINE-1]]:3: warning: variable 'p_local2' of type 'const double *' can be declared const + + double p_local3[10] = {0., 1., 2., 3., 4., 5., 6., 7., 8., 9.}; + // CHECK-MESSAGES: [[@LINE-1]]:3: warning: variable 'p_local3' of type 'double [10]' can be declared const + const double &const_ref = p_local3[2]; + + double p_local4[10] = {0., 1., 2., 3., 4., 5., 6., 7., 8., 9.}; + // CHECK-MESSAGES: [[@LINE-1]]:3: warning: variable 'p_local4' of type 'double [10]' can be declared const + const double *const_ptr; + const_ptr = &p_local4[2]; + + double p_local5[10] = {0., 1., 2., 3., 4., 5., 6., 7., 8., 9.}; + // CHECK-MESSAGES: [[@LINE-1]]:3: warning: variable 'p_local5' of type 'double [10]' can be declared const + const_handle(p_local5[2]); + double p_local6[10] = {0., 1., 2., 3., 4., 5., 6., 7., 8., 9.}; + // CHECK-MESSAGES: [[@LINE-1]]:3: warning: variable 'p_local6' of type 'double [10]' can be declared const + const_handle(&p_local6[2]); +} + +void range_for() { + int np_local0[2] = {1, 2}; + for (int &non_const_ref : np_local0) { + } + + int np_local1[2] = {1, 2}; + for (auto &non_const_ref : np_local1) { + } + + int np_local2[2] = {1, 2}; + for (auto &&non_const_ref : np_local2) { + } + + int *np_local3[2] = {&np_local0[0], &np_local0[1]}; + for (int *non_const_ptr : np_local3) { + // CHECK-MESSAGES: [[@LINE-1]]:8: warning: variable 'non_const_ptr' of type 'int *' can be declared const + } + + int *np_local4[2] = {&np_local0[0], &np_local0[1]}; + for (auto *non_const_ptr : np_local4) { + // CHECK-MESSAGES: [[@LINE-1]]:8: warning: variable 'non_const_ptr' of type 'int *' can be declared const + } + + int p_local0[2] = {1, 2}; + // CHECK-MESSAGES: [[@LINE-1]]:3: warning: variable 'p_local0' of type 'int [2]' can be declared const + for (int value : p_local0) { + // CHECK-MESSAGES: [[@LINE-1]]:8: warning: variable 'value' of type 'int' can be declared const + } + + int p_local1[2] = {1, 2}; + // CHECK-MESSAGES: [[@LINE-1]]:3: warning: variable 'p_local1' of type 'int [2]' can be declared const + for (const int &const_ref : p_local1) { + } + + int *p_local2[2] = {&np_local0[0], &np_local0[1]}; + // CHECK-MESSAGES: [[@LINE-1]]:3: warning: variable 'p_local2' of type 'int *[2]' can be declared const + for (const int *con_ptr : p_local2) { + // CHECK-MESSAGES: [[@LINE-1]]:8: warning: variable 'con_ptr' of type 'const int *' can be declared const + } + + int *p_local3[2] = {nullptr, nullptr}; + // CHECK-MESSAGES: [[@LINE-1]]:3: warning: variable 'p_local3' of type 'int *[2]' can be declared const + for (const auto *con_ptr : p_local3) { + // CHECK-MESSAGES: [[@LINE-1]]:8: warning: variable 'con_ptr' of type 'const int *' can be declared const + } +} Index: docs/clang-tidy/checks/list.rst =================================================================== --- docs/clang-tidy/checks/list.rst +++ docs/clang-tidy/checks/list.rst @@ -76,6 +76,7 @@ cert-oop11-cpp (redirects to performance-move-constructor-init) <cert-oop11-cpp> cppcoreguidelines-avoid-goto cppcoreguidelines-c-copy-assignment-signature (redirects to misc-unconventional-assign-operator) <cppcoreguidelines-c-copy-assignment-signature> + cppcoreguidelines-const cppcoreguidelines-interfaces-global-init cppcoreguidelines-no-malloc cppcoreguidelines-owning-memory Index: docs/clang-tidy/checks/cppcoreguidelines-const.rst =================================================================== --- /dev/null +++ docs/clang-tidy/checks/cppcoreguidelines-const.rst @@ -0,0 +1,6 @@ +.. title:: clang-tidy - cppcoreguidelines-const + +cppcoreguidelines-const +======================= + +FIXME: Describe what patterns does the check detect and why. Give examples. Index: docs/ReleaseNotes.rst =================================================================== --- docs/ReleaseNotes.rst +++ docs/ReleaseNotes.rst @@ -98,6 +98,11 @@ The usage of ``goto`` for control flow is error prone and should be replaced with looping constructs. Every backward jump is rejected. Forward jumps are only allowed in nested loops. + +- New :doc:`cppcoreguidelines-const + <clang-tidy/checks/cppcoreguidelines-const>` check + + FIXME: add release notes. - New alias :doc:`fuchsia-header-anon-namespaces <clang-tidy/checks/fuchsia-header-anon-namespaces>` to :doc:`google-build-namespaces Index: clang-tidy/cppcoreguidelines/CppCoreGuidelinesTidyModule.cpp =================================================================== --- clang-tidy/cppcoreguidelines/CppCoreGuidelinesTidyModule.cpp +++ clang-tidy/cppcoreguidelines/CppCoreGuidelinesTidyModule.cpp @@ -12,6 +12,7 @@ #include "../ClangTidyModuleRegistry.h" #include "../misc/UnconventionalAssignOperatorCheck.h" #include "AvoidGotoCheck.h" +#include "ConstCheck.h" #include "InterfacesGlobalInitCheck.h" #include "NoMallocCheck.h" #include "OwningMemoryCheck.h" @@ -38,6 +39,8 @@ void addCheckFactories(ClangTidyCheckFactories &CheckFactories) override { CheckFactories.registerCheck<AvoidGotoCheck>( "cppcoreguidelines-avoid-goto"); + CheckFactories.registerCheck<ConstCheck>( + "cppcoreguidelines-const"); CheckFactories.registerCheck<InterfacesGlobalInitCheck>( "cppcoreguidelines-interfaces-global-init"); CheckFactories.registerCheck<NoMallocCheck>("cppcoreguidelines-no-malloc"); Index: clang-tidy/cppcoreguidelines/ConstCheck.h =================================================================== --- /dev/null +++ clang-tidy/cppcoreguidelines/ConstCheck.h @@ -0,0 +1,68 @@ +//===--- ConstCheck.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_CPPCOREGUIDELINES_CONSTCHECK_H +#define LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_CPPCOREGUIDELINES_CONSTCHECK_H + +#include "../ClangTidy.h" +#include <iostream> +#include <unordered_map> + +namespace clang { +namespace tidy { +namespace cppcoreguidelines { + +/// This check warns for every variable, that could be declared as const, but +/// isn't. +/// +/// For the user-facing documentation see: +/// http://clang.llvm.org/extra/clang-tidy/checks/cppcoreguidelines-const.html +class ConstCheck : public ClangTidyCheck { +public: + ConstCheck(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: + void variableRegistering(ast_matchers::MatchFinder *Finder); + void handleRegistration(const ast_matchers::MatchFinder::MatchResult &Result); + + void valueTypeMatchers(ast_matchers::MatchFinder *Finder); + void checkValueType(const ast_matchers::MatchFinder::MatchResult &Result); + void invalidateRefCaptured(const LambdaExpr *Lambda); + + template <typename ASTElement> + bool notConstVal(const ast_matchers::MatchFinder::MatchResult &Result, + StringRef matcher_bind) { + if (Result.Nodes.getNodeAs<ASTElement>(matcher_bind)) { + // std::cout << "Prohibting through " << std::string(matcher_bind) + // << std::endl; + const auto *Variable = Result.Nodes.getNodeAs<VarDecl>("value-decl"); + ValueCanBeConst[Variable] = false; + return true; + } + return false; + } + + void handleTypeMatchers(ast_matchers::MatchFinder *Finder); + void checkHandleType(const ast_matchers::MatchFinder::MatchResult &Result); + + void diagnosePotentialConst(); + + llvm::DenseMap<const VarDecl *, bool> ValueCanBeConst; + llvm::DenseMap<const VarDecl *, bool> HandleCanBeConst; +}; + +} // namespace cppcoreguidelines +} // namespace tidy +} // namespace clang + +#endif // LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_CPPCOREGUIDELINES_CONSTCHECK_H Index: clang-tidy/cppcoreguidelines/ConstCheck.cpp =================================================================== --- /dev/null +++ clang-tidy/cppcoreguidelines/ConstCheck.cpp @@ -0,0 +1,392 @@ +//===--- ConstCheck.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 "ConstCheck.h" +#include "clang/AST/ASTContext.h" +#include "clang/ASTMatchers/ASTMatchFinder.h" + +#include <iostream> + +using namespace clang::ast_matchers; + +namespace clang { +namespace tidy { +namespace cppcoreguidelines { + +/* + * General Thoughts + * ================ + * + * For now: Only local variables are considered. Globals/namespace variables, + * paramters and class members are not analyzed. + * Parameters have a check already: readability-non-const-parameter + * + * + * Handle = either a pointer or reference + * Value = everything else (Type variable_name;) + * + * Value Semantic + * ============== + * - it is neither global nor namespace level + CHECK + * - it never gets assigned to after initialization + CHECK + * -> every uninitialized variable can not be const + CHECK + * - no non-const handle is created with it + CHECK + * - no non-const pointer from it + CHECK + * - no non-const pointer argument + CHECK + * - no non-const reference from it + CHECK + * - no non-const reference argument + CHECK + * - no non-const capture by reference in a lambda + CHECK + * - it is not returned as non-const handle from a function + CHECK + * - it address is not assigned to an out pointer parameter + CHECK + * + * primitive Builtins + * ---------------- + * - it is not modified with an operator (++i,i++,--i,i--) + CHECK + * - it is not modified with an operator-assignment + CHECK + * + * objects + * ------- + * - there is no call to a non-const method + CHECK + * - there is no call to an non-const overloaded operator + CHECK + * - there is no non-const iterator created from this type + CHECK + * (std::begin and friends) + * + * arrays + * ------ + * - there is no non-const operator[] access + CHECK + * - there is no non-const handle creation of one of the elements + CHECK + * - there is no non-const iterator created from this type + CHECK + * (std::begin and friends) + * + * templated variables + * ------------------- + * - one can not reason about templated variables, because every sensible + * operation is overloadable and different instantiations will result + * in types with different const-properties. + * - Example: operator+(T& lhs, T& rhs) -> modification might occur for this + * type + * -> this fordbids `val = val1 + val2` val1 and val2 to be const + * - only concepts give possibility to infer constness of templated variables + * + * Handle Semantic + * =============== + * - modification of the pointee prohibit constness + * + * pointers + * -------- + * - match both for value and handle semantic + * + * references + * ---------- + * - only handle semantic applies + * - references to templated types suffer from the same problems as templated + * values + * + * forwarding reference + * -------------------- + * - same as references? + * + * Implementation strategy + * ======================= + * + * - Register every declared local variable/constant with value semantic. + * (pointers, values) + * Store if they can be made const. + * (const int i = 10 : no, + * int *const = &i : no, + * int i = 10 : yes, -> const int i = 10 + * int *p_i = &i : yes, -> int *const p_i = &i) + * - Register every declared local variable/constant with handle semantic. + * (pointers, references) + * Store if they can be made const, meaning if they can be a const target + * (const int *cp_i = &i : no, + * const int &cr_i = i : no, + * int *p_i = &i : yes, -> const int *p_i = &i + * int &r_i = i : yes, -> const int &r_i = i) + * - Keep 2 dictionaries for values and handles + * + * - Match operations/events that forbid values to be const -> mark then 'no' + * - Match operations/events that forbid handles to be const -> mark then 'no' + * + * - once the translation unit is finished, determine what can be const, by + * just iterating over all keys and check if they map to 'true'. + * - values that can be const -> emit warning for their type and name + * - handles that can be const -> emit warning for the pointee type and name + * - ignore the rest + * + * Open Questions + * ============== + * + * - type conversions: + * - one can overload the type conversion operation and modify a value of a + * class -> implications? + * - what about the 'mutable' keyword -> not considered now, because it applies + * only to class members + */ + +void ConstCheck::registerMatchers(MatchFinder *Finder) { + // Ensure that all intersting variables are registered in our mapping. + variableRegistering(Finder); + valueTypeMatchers(Finder); +} + +void ConstCheck::check(const MatchFinder::MatchResult &Result) { + if (!getLangOpts().CPlusPlus) + return; + + handleRegistration(Result); + checkValueType(Result); + // checkHandleType(Result); +} + +// The decision which variable might be made const can only be made at the +// end of each translation unit. +void ConstCheck::onEndOfTranslationUnit() { diagnosePotentialConst(); } + +void ConstCheck::variableRegistering(MatchFinder *Finder) { + const auto HandleType = + anyOf(hasType(referenceType()), hasType(pointerType())); + const auto ValueType = unless(hasType(referenceType())); + const auto ConstType = hasType(isConstQualified()); + const auto TemplateType = anyOf(hasType(templateTypeParmType()), + hasType(substTemplateTypeParmType())); + const auto LocalVariable = + anyOf(hasAncestor(functionDecl(hasBody(compoundStmt()))), + hasAncestor(lambdaExpr())); + + // Match local variables, that could be const. + // Example: `int i = 10`, `int i` (will be used if program is correct) + Finder->addMatcher(varDecl(allOf(LocalVariable, hasInitializer(anything()), + unless(ConstType), unless(TemplateType), + unless(isImplicit()), ValueType)) + .bind("new-local-value"), + this); + + // Match local handle types, that are not const. + // Example: `int &ri`, `int * pi`. + Finder->addMatcher(varDecl(allOf(LocalVariable, HandleType, + unless(isImplicit()), unless(ConstType))) + .bind("new-local-handle"), + this); +} + +void ConstCheck::handleRegistration(const MatchFinder::MatchResult &Result) { + // Local variables can be declared as consts. + if (const auto *Variable = + Result.Nodes.getNodeAs<VarDecl>("new-local-value")) { + // std::cout << "Added new local value" << std::endl; + if (ValueCanBeConst.find(Variable) == ValueCanBeConst.end()) { + ValueCanBeConst[Variable] = true; + return; + } + } + + if (const auto *Variable = + Result.Nodes.getNodeAs<VarDecl>("new-local-handle")) { + // std::cout << "Added new local handle" << std::endl; + if (HandleCanBeConst.find(Variable) == HandleCanBeConst.end()) { + HandleCanBeConst[Variable] = true; + return; + } + } +} + +void ConstCheck::valueTypeMatchers(MatchFinder *Finder) { + // Matchers for the basic differntiation groups of types. + const auto IsHandleType = + anyOf(hasType(referenceType()), hasType(pointerType())); + const auto IsValueType = unless(hasType(referenceType())); + + // Matchers for non-const handles. + const auto IsRefenceToNonConstType = + hasType(references(qualType(unless(isConstQualified())))); + const auto IsPointerToNonConstType = + hasType(pointerType(pointee(unless(isConstQualified())))); + const auto IsNonConstHandleType = + anyOf(IsRefenceToNonConstType, IsPointerToNonConstType); + + // Match value, array and pointer access. + // Pointer do have value and reference semantic. + const auto ValueDeclRef = declRefExpr( + allOf(hasDeclaration(varDecl(unless(isImplicit())).bind("value-decl")), + IsValueType)); + const auto ArrayAccess = + arraySubscriptExpr(hasBase(ignoringImpCasts(ValueDeclRef))); + const auto IsValueDeclRefExpr = anyOf(ArrayAccess, ValueDeclRef); + + // Classical assignment of any form (=, +=, <<=, ...) modifies the LHS + // and prohibts it from being const. + Finder->addMatcher( + binaryOperator(allOf(isAssignmentOperator(), hasLHS(IsValueDeclRefExpr))) + .bind("value-assignment"), + this); + + // Usage of the '++' or '--' operator modifies a variable. + Finder->addMatcher( + unaryOperator(allOf(anyOf(hasOperatorName("++"), hasOperatorName("--")), + hasUnaryOperand(IsValueDeclRefExpr))) + .bind("value-unary-modification"), + this); + + // Check the address operator. + Finder->addMatcher( + unaryOperator(allOf(hasOperatorName("&"), + // Checking for the ImplicitCastExpr is enough, + // because a pointer can get casted only in the 'add + // const' direction implicitly. + unless(hasParent(implicitCastExpr())), + hasUnaryOperand(IsValueDeclRefExpr))) + .bind("value-address-to-non-const"), + this); + + // Check creation of references to this value. + Finder->addMatcher( + varDecl(allOf(IsNonConstHandleType, hasInitializer(IsValueDeclRefExpr), + unless(isImplicit()))) + .bind("value-non-const-reference"), + this); + + // Check function calls that bind by reference. + Finder->addMatcher( + callExpr(forEachArgumentWithParam( + IsValueDeclRefExpr, parmVarDecl(IsNonConstHandleType) + .bind("value-non-const-ref-call-param"))), + this); + + // Check return values that reference a value. + Finder->addMatcher( + functionDecl( + allOf(hasDescendant(returnStmt(hasReturnValue(IsValueDeclRefExpr))), + returns(qualType( + references(qualType(unless(isConstQualified()))))))) + .bind("returns-non-const-ref"), + this); + + // Check for direct method calls, that modify its object by declaration. + Finder->addMatcher( + cxxMemberCallExpr(allOf(on(IsValueDeclRefExpr), + unless(callee(cxxMethodDecl(isConst()))))) + .bind("non-const-member-call"), + this); + + // Check for operator calls, that are non-const. E.g. operator= + Finder->addMatcher( + cxxOperatorCallExpr(allOf(hasArgument(0, IsValueDeclRefExpr), + unless(callee(cxxMethodDecl(isConst()))))) + .bind("non-const-operator-call"), + this); + + // Check for range-for loops that declare non-const handles as loop variable. + Finder->addMatcher( + cxxForRangeStmt(allOf(hasLoopVariable(IsNonConstHandleType), + hasRangeInit(IsValueDeclRefExpr))) + .bind("non-const-range-for"), + this); + + // Lambda expression can capture variables by reference which invalidates + // the captured variables. Lambdas capture only the variables they actually + // use! + Finder->addMatcher(lambdaExpr().bind("value-lambda"), this); +} + +void ConstCheck::checkValueType(const MatchFinder::MatchResult &Res) { + bool Prohibited = false; + + // Assignment of any form prohibits the LHS to be const. + Prohibited = notConstVal<BinaryOperator>(Res, "value-assignment"); + // Usage of the '++' or '--' operator modifies a value. + Prohibited |= notConstVal<UnaryOperator>(Res, "value-unary-modification"); + // The address of the values has been taken and did not result in a + // pointer to const. + Prohibited |= notConstVal<UnaryOperator>(Res, "value-address-to-non-const"); + // A reference is initialized with a value. + Prohibited |= notConstVal<VarDecl>(Res, "value-non-const-reference"); + // A call where a parameter is a non-const reference. + Prohibited |= notConstVal<ParmVarDecl>(Res, "value-non-const-ref-call-param"); + // A function returning a non-const reference prohibts its return value + // to be const. + Prohibited |= notConstVal<FunctionDecl>(Res, "returns-non-const-ref"); + // Calling a member function, that is not declared as const prohibts + // constness of the value. + Prohibited |= notConstVal<CXXMemberCallExpr>(Res, "non-const-member-call"); + // Calling an overloaded operator that is not declared as const probits + // constness similar to member calls. + Prohibited |= + notConstVal<CXXOperatorCallExpr>(Res, "non-const-operator-call"); + // Range-For can loop in a modifying way over the range. This is equivalent + // to taking a reference/pointer to one of the elements of the range. + Prohibited |= notConstVal<CXXForRangeStmt>(Res, "non-const-range-for"); + + // If the matchable targets did result in prohibiton, we can stop. + // Otherwise analysis that does not function with the same pattern must be + // applied. + if (Prohibited) + return; + + // Analysis of the lambda is more difficult. + // Offloaded into a separate function. + if (const auto *Lambda = Res.Nodes.getNodeAs<LambdaExpr>("value-lambda")) { + invalidateRefCaptured(Lambda); + return; + } +} + +void ConstCheck::invalidateRefCaptured(const LambdaExpr *Lambda) { + for (const auto capture : Lambda->captures()) { + if (capture.capturesVariable() && + capture.getCaptureKind() == LambdaCaptureKind::LCK_ByRef) { + ValueCanBeConst[capture.getCapturedVar()] = false; + } + } +} + +void ConstCheck::diagnosePotentialConst() { + // Diagnose all potential values. + for (const auto it : ValueCanBeConst) { + bool VarCanBeConst = it.second; + + if (VarCanBeConst) { + const VarDecl *Variable = it.first; + + diag(Variable->getLocStart(), + "variable %0 of type %1 can be declared const") + << Variable << Variable->getType(); + } + } + + /* + for (const auto it : HandleCanBeConst) { + bool HandleCanBeConst = it.second; + + // Example: `int& ri` could be `const int& ri`. + if (HandleCanBeConst) { + const VarDecl *Variable = it.first; + + // Differentiate between pointers and references. + QualType HandleType = Variable->getType(); + if (HandleType->isReferenceType()) { + + diag(Variable->getLocStart(), + "reference type %0 of variable %1 can be declared const") + << HandleType << Variable; + } else if (HandleType->isPointerType()) { + + diag(Variable->getLocStart(), + "pointee type %0 of variable %1 can be declared const") + << HandleType << Variable; + } else + llvm_unreachable("Expected handle type"); + } + } + */ +} + +} // namespace cppcoreguidelines +} // namespace tidy +} // namespace clang Index: clang-tidy/cppcoreguidelines/CMakeLists.txt =================================================================== --- clang-tidy/cppcoreguidelines/CMakeLists.txt +++ clang-tidy/cppcoreguidelines/CMakeLists.txt @@ -2,6 +2,7 @@ add_clang_library(clangTidyCppCoreGuidelinesModule AvoidGotoCheck.cpp + ConstCheck.cpp CppCoreGuidelinesTidyModule.cpp InterfacesGlobalInitCheck.cpp NoMallocCheck.cpp
_______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits