JonasToth updated this revision to Diff 153282. JonasToth added a comment. - Merge branch 'master' into check_const
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-pointers-as-values.cpp test/clang-tidy/cppcoreguidelines-const-values.cpp
Index: test/clang-tidy/cppcoreguidelines-const-values.cpp =================================================================== --- /dev/null +++ test/clang-tidy/cppcoreguidelines-const-values.cpp @@ -0,0 +1,557 @@ +// 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 nested_scopes() { + int p_local0 = 2; + // CHECK-MESSAGES: [[@LINE-1]]:3: warning: variable 'p_local0' of type 'int' can be declared const + int np_local0 = 42; + + { + int p_local1 = 42; + // CHECK-MESSAGES: [[@LINE-1]]:5: warning: variable 'p_local1' of type 'int' can be declared const + np_local0 *= 2; + } +} + +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; + const int *const p0_np_local1 = &np_local1; + int *const p1_np_local1 = &np_local1; + *p1_np_local0 = 43; + + int np_local2 = 42; + function_inout_pointer(&np_local2); + + // Prevents const. + int np_local3 = 42; + out = &np_local3; // This returns and invalid address, its just about the AST + + int p_local1 = 42; + // CHECK-MESSAGES: [[@LINE-1]]:3: warning: variable 'p_local1' of type 'int' can be declared const + const int *const p0_p_local1 = &p_local1; + + int p_local2 = 42; + // CHECK-MESSAGES: [[@LINE-1]]:3: warning: variable 'p_local2' of type 'int' can be declared const + function_in_pointer(&p_local2); +} + +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; + r1_np_local0 = 43; + 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; + + double NonConstMember; + const double ConstMember; + + double &NonConstMemberRef; + const double &ConstMemberRef; + + double *NonConstMemberPtr; + const double *ConstMemberPtr; +}; + +void direct_class_access() { + 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 + + ConstNonConstClass np_local3; + np_local3.NonConstMember = 42.; + + ConstNonConstClass np_local4; + np_local4.NonConstMemberRef = 42.; + + ConstNonConstClass p_local3; + // CHECK-MESSAGES: [[@LINE-1]]:3: warning: variable 'p_local3' of type 'ConstNonConstClass' can be declared const + const double val0 = p_local3.NonConstMember; + const double val1 = p_local3.NonConstMemberRef; + const double val2 = *p_local3.NonConstMemberPtr; + + ConstNonConstClass p_local4; + // CHECK-MESSAGES: [[@LINE-1]]:3: warning: variable 'p_local4' of type 'ConstNonConstClass' can be declared const + *np_local4.NonConstMemberPtr = 42.; +} + +void class_access_array() { + ConstNonConstClass np_local0[2]; + np_local0[0].constMethod(); + np_local0[1].constMethod(); + np_local0[1].nonConstMethod(); + + ConstNonConstClass p_local0[2]; + // CHECK-MESSAGES: [[@LINE-1]]:3: warning: variable 'p_local0' of type 'ConstNonConstClass [2]' can be declared const + p_local0[0].constMethod(); + np_local0[1].constMethod(); +} + +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]; // Could be `double *const`, but warning deactivated by default + + double np_local1[10] = {0., 1., 2., 3., 4., 5., 6., 7., 8., 9.}; + double &non_const_ref = np_local1[1]; + non_const_ref = 42.; + + 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]; // Could be `const double *const`, but warning deactivated by default + + 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) { + non_const_ref = 42; + } + + int np_local1[2] = {1, 2}; + for (auto &non_const_ref : np_local1) { + non_const_ref = 43; + } + + int np_local2[2] = {1, 2}; + for (auto &&non_const_ref : np_local2) { + non_const_ref = 44; + } + + // FIXME the warning message is suboptimal. It could be defined as + // `int *const np_local3[2]` because the pointers are not reseated. + // But this is not easily deducable from the warning. + int *np_local3[2] = {&np_local0[0], &np_local0[1]}; + // CHECK-MESSAGES: [[@LINE-1]]:3: warning: variable 'np_local3' of type 'int *[2]' can be declared const + for (int *non_const_ptr : np_local3) { + *non_const_ptr = 45; + } + + // FIXME same as above, but silenced + int *const np_local4[2] = {&np_local0[0], &np_local0[1]}; + for (auto *non_const_ptr : np_local4) { + *non_const_ptr = 46; + } + + 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) { + } + + 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) { + } +} + +inline void *operator new(decltype(sizeof(void *)), void *p) { return p; } + +struct Value { +}; +void placement_new() { + Value Mem; + Value *V = new (&Mem) Value; +} + +struct ModifyingConversion { + operator int() { return 15; } +}; +struct NonModifyingConversion { + operator int() const { return 15; } +}; +void conversion_operators() { + ModifyingConversion np_local0; + NonModifyingConversion p_local0; + // CHECK-MESSAGES: [[@LINE-1]]:3: warning: variable 'p_local0' of type 'NonModifyingConversion' can be declared const + + int np_local1 = np_local0; + np_local1 = p_local0; +} + +void casts() { + decltype(sizeof(void *)) p_local0 = 42; + // CHECK-MESSAGES: [[@LINE-1]]:3: warning: variable 'p_local0' of type 'decltype(sizeof(void *))' (aka 'unsigned long') can be declared const + auto np_local0 = reinterpret_cast<void *>(p_local0); + np_local0 = nullptr; + + int p_local1 = 43; + // CHECK-MESSAGES: [[@LINE-1]]:3: warning: variable 'p_local1' of type 'int' can be declared const + short p_local2 = static_cast<short>(p_local1); + // CHECK-MESSAGES: [[@LINE-1]]:3: warning: variable 'p_local2' of type 'short' can be declared const + + int np_local1 = p_local2; + int &np_local2 = static_cast<int &>(np_local1); + np_local2 = 5; +} + +void ternary_operator() { + int np_local0 = 1, np_local1 = 2; + int &np_local2 = true ? np_local0 : np_local1; + np_local2 = 2; + + int p_local0 = 3, np_local3 = 5; + // CHECK-MESSAGES: [[@LINE-1]]:3: warning: variable 'p_local0' of type 'int' can be declared const + const int &np_local4 = true ? p_local0 : ++np_local3; + + int np_local5[3] = {1, 2, 3}; + int &np_local6 = np_local5[1] < np_local5[2] ? np_local5[0] : np_local5[2]; + np_local6 = 42; + + int np_local7[3] = {1, 2, 3}; + int *np_local8 = np_local7[1] < np_local7[2] ? &np_local7[0] : &np_local7[2]; + *np_local8 = 42; +} + +// taken from http://www.cplusplus.com/reference/type_traits/integral_constant/ +template <typename T, T v> +struct integral_constant { + static constexpr T value = v; + using value_type = T; + using type = integral_constant<T, v>; + constexpr operator T() { return v; } +}; + +template <typename T> +struct is_integral : integral_constant<bool, false> {}; +template <> +struct is_integral<int> : integral_constant<bool, true> {}; + +template <typename T> +struct not_integral : integral_constant<bool, false> {}; +template <> +struct not_integral<double> : integral_constant<bool, true> {}; + +// taken from http://www.cplusplus.com/reference/type_traits/enable_if/ +template <bool Cond, typename T = void> +struct enable_if {}; + +template <typename T> +struct enable_if<true, T> { using type = T; }; + +template <typename T> +struct TMPClass { + T alwaysConst() const { return T{}; } + + template <typename T2 = T, typename = typename enable_if<is_integral<T2>::value>::type> + T sometimesConst() const { return T{}; } + + template <typename T2 = T, typename = typename enable_if<not_integral<T2>::value>::type> + T sometimesConst() { return T{}; } +}; + +void meta_type() { + TMPClass<int> p_local0; + // CHECK-MESSAGES: [[@LINE-1]]:3: warning: variable 'p_local0' of type 'TMPClass<int>' can be declared const + p_local0.alwaysConst(); + p_local0.sometimesConst(); + + TMPClass<double> p_local1; + // CHECK-MESSAGES: [[@LINE-1]]:3: warning: variable 'p_local1' of type 'TMPClass<double>' can be declared const + p_local1.alwaysConst(); + + TMPClass<double> np_local0; + np_local0.alwaysConst(); + np_local0.sometimesConst(); +} Index: test/clang-tidy/cppcoreguidelines-const-pointers-as-values.cpp =================================================================== --- /dev/null +++ test/clang-tidy/cppcoreguidelines-const-pointers-as-values.cpp @@ -0,0 +1,11 @@ +// RUN: %check_clang_tidy %s cppcoreguidelines-const %t \ +// RUN: -config='{CheckOptions: \ +// RUN: [{key: "cppcoreguidelines-const.AnalyzeValues", value: 1},\ +// RUN: {key: "cppcoreguidelines-const.WarnPointersAsValues", value: 1}]}' \ +// RUN: -- + +void potential_const_pointer() { + 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 +} Index: docs/clang-tidy/checks/list.rst =================================================================== --- docs/clang-tidy/checks/list.rst +++ docs/clang-tidy/checks/list.rst @@ -77,6 +77,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-narrowing-conversions cppcoreguidelines-no-malloc Index: docs/clang-tidy/checks/cppcoreguidelines-const.rst =================================================================== --- /dev/null +++ docs/clang-tidy/checks/cppcoreguidelines-const.rst @@ -0,0 +1,68 @@ +.. title:: clang-tidy - cppcoreguidelines-const + +cppcoreguidelines-const +======================= + +This check implements detection of local variables that could be declared as +``const``, but are not. Declaring variables as ``const`` is required by many +coding guidelines for example +`CppCoreGuidelines ES.25 <https://github.com/isocpp/CppCoreGuidelines/blob/master/CppCoreGuidelines.md#es25-declare-an-object-const-or-constexpr-unless-you-want-to-modify-its-value-later-on>`_ +and `High Integrity C++ 7.1.2 <http://www.codingstandard.com/rule/7-1-2-use-const-whenever-possible/>`_. + +Please note that this analysis is type based only. Variables that are not modified +but non-const handles might escape out of the scope are not diagnosed as potential +``const``. + +.. code-block:: c++ + + // Declare a variable, which is not ``const`` ... + int i = 42; + // but use it as read-only. This means that `i` can be declared ``const``. + int result = i * i; + +The check analyzes values, pointers and references (if configured that way). +For better understanding some code samples: + +.. code-block:: c++ + + // Normal values like built-ins or objects. + int potential_const_int = 42; + int copy_of_value = potential_const_int; + + MyClass could_be_const; + could_be_const.const_qualified_method(); + + // References can be declared const as well. + int &reference_value = potential_const_int; + int another_copy = reference_value; + + // Similar behaviour for pointers. + int *pointer_variable = &potential_const_int; + int last_copy = *pointer_variable; + + +Options +------- + +.. option:: AnalyzeValues (default = 1) + + Enable or disable the analysis of ordinary value variables, like ``int i = 42;`` + +.. option:: AnalyzeReferences (default = 1) + + Enable or disable the analysis of reference variables, like ``int &ref = i;`` + +.. option:: WarnPointersAsValues (default = 0) + + This option enables the suggestion for ``const`` of the pointer itself. + Pointer values have two possibilities to be ``const``, the pointer itself + and the value pointing to. + + .. code-block:: c++ + + const int value = 42; + const int * const pointer_variable = &value; + + // The following operations are forbidden for `pointer_variable`. + // *pointer_variable = 44; + // pointer_variable = nullptr; Index: docs/ReleaseNotes.rst =================================================================== --- docs/ReleaseNotes.rst +++ docs/ReleaseNotes.rst @@ -102,6 +102,11 @@ 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 + + Suggest adding ``const`` to unmodified local variables. + - New :doc:`cppcoreguidelines-narrowing-conversions <clang-tidy/checks/cppcoreguidelines-narrowing-conversions>` check 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 "NarrowingConversionsCheck.h" #include "NoMallocCheck.h" @@ -39,6 +40,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<NarrowingConversionsCheck>( Index: clang-tidy/cppcoreguidelines/ConstCheck.h =================================================================== --- /dev/null +++ clang-tidy/cppcoreguidelines/ConstCheck.h @@ -0,0 +1,57 @@ +//===--- 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 "../utils/ExprMutationAnalyzer.h" + +namespace clang { +namespace tidy { + +namespace utils { +class ExprMutationAnalyzer; +} + +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), + AnalyzeValues(Options.get("AnalyzeValues", 1)), + AnalyzeReferences(Options.get("AnalyzeReferences", 1)), + WarnPointersAsValues(Options.get("WarnPointersAsValues", 0)) {} + + void storeOptions(ClangTidyOptions::OptionMap &Opts) override; + void registerMatchers(ast_matchers::MatchFinder *Finder) override; + void check(const ast_matchers::MatchFinder::MatchResult &Result) override; + +private: + void registerScope(const CompoundStmt *Scope, ASTContext *Context); + + using MutationAnalyzer = std::unique_ptr<utils::ExprMutationAnalyzer>; + llvm::DenseMap<const CompoundStmt *, MutationAnalyzer> ScopesCache; + + const bool AnalyzeValues; + const bool AnalyzeReferences; + const bool WarnPointersAsValues; +}; + +} // 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,207 @@ +//===--- 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 "../utils/ASTUtils.h" +// #include "../utils/DeclRefExprUtils.h" +#include "clang/AST/ASTContext.h" +#include "clang/ASTMatchers/ASTMatchFinder.h" + +#include <iostream> + +using namespace clang::ast_matchers; + +namespace clang { +namespace tidy { +namespace cppcoreguidelines { + +/* + * NOTE: This massive comment will be removed before committing. It serves as + * notebook on what to keep an eye on for now. + * + * 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 non-const access to a member + * - 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 + * - Handles follow the typ of the pointee + * + * - no assignment to the target of the handle + * + * 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 + * + */ + +namespace { +AST_MATCHER(VarDecl, isLocal) { return Node.isLocalVarDecl(); } +} // namespace + +void ConstCheck::storeOptions(ClangTidyOptions::OptionMap &Opts) { + Options.store(Opts, "AnalyzeValues", AnalyzeValues); + Options.store(Opts, "AnalyzeReferences", AnalyzeReferences); + Options.store(Opts, "WarnPointersAsValues", WarnPointersAsValues); +} + +void ConstCheck::registerMatchers(MatchFinder *Finder) { + if (!getLangOpts().CPlusPlus) + return; + + const auto ConstType = hasType(isConstQualified()); + const auto ConstReference = hasType(references(isConstQualified())); + const auto TemplateType = anyOf(hasType(templateTypeParmType()), + hasType(substTemplateTypeParmType())); + + // Match local variables, that could be const. + // Example: `int i = 10`, `int i` (will be used if program is correct) + const auto LocalValDecl = varDecl(allOf( + isLocal(), hasInitializer(anything()), unless(ConstType), + unless(ConstReference), unless(TemplateType), unless(isImplicit()))); + + // Match the function scope for which the analysis of all local variables + // shall be run. + const auto FunctionScope = + functionDecl(allOf(hasBody(compoundStmt().bind("scope")), + findAll(LocalValDecl.bind("new-local-value")))); + Finder->addMatcher(FunctionScope, this); +} + +void ConstCheck::check(const MatchFinder::MatchResult &Result) { + if (!getLangOpts().CPlusPlus) + return; + + const auto *Scope = Result.Nodes.getNodeAs<CompoundStmt>("scope"); + assert(Scope && "Did not match scope for local variable"); + registerScope(Scope, Result.Context); + + const auto *Variable = Result.Nodes.getNodeAs<VarDecl>("new-local-value"); + assert(Variable && "Did not match local variable definition"); + + // Each variable can only in one category: Value, Pointer, Reference. + // Analysis can be controlled for every category. + if (Variable->getType()->isReferenceType() && !AnalyzeReferences) + return; + + if (Variable->getType()->isPointerType() && !WarnPointersAsValues) + return; + + if (!AnalyzeValues) + return; + + // Offload const-analysis to utility function. + if (ScopesCache[Scope]->isMutated(Variable)) + return; + + // TODO Implement automatic code transformation to add the const. + diag(Variable->getLocStart(), "variable %0 of type %1 can be declared const") + << Variable << Variable->getType(); +} + +void ConstCheck::registerScope(const CompoundStmt *Scope, ASTContext *Context) { + if (ScopesCache.find(Scope) == ScopesCache.end()) { + ScopesCache.insert(std::make_pair( + Scope, llvm::make_unique<utils::ExprMutationAnalyzer>(Scope, Context))); + } +} + +} // 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 NarrowingConversionsCheck.cpp
_______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits