diff --git a/clang-tools-extra/clang-tidy/misc/ConstCorrectnessCheck.cpp b/clang-tools-extra/clang-tidy/misc/ConstCorrectnessCheck.cpp --- a/clang-tools-extra/clang-tidy/misc/ConstCorrectnessCheck.cpp +++ b/clang-tools-extra/clang-tidy/misc/ConstCorrectnessCheck.cpp @@ -11,6 +11,7 @@ #include "clang/AST/ASTContext.h" #include "clang/ASTMatchers/ASTMatchFinder.h" #include "clang/ASTMatchers/ASTMatchers.h" +#include "llvm/Support/Casting.h" #include @@ -132,6 +133,12 @@ VC = VariableCategory::Reference; if (Variable->getType()->isPointerType()) VC = VariableCategory::Pointer; + if (Variable->getType()->isArrayType()) { + if (const auto *ArrayT = dyn_cast(Variable->getType())) { + if (ArrayT->getElementType()->isPointerType()) + VC = VariableCategory::Pointer; + } + } // Each variable can only be in one category: Value, Pointer, Reference. // Analysis can be controlled for every category. diff --git a/clang-tools-extra/docs/clang-tidy/checks/misc/const-correctness.rst b/clang-tools-extra/docs/clang-tidy/checks/misc/const-correctness.rst --- a/clang-tools-extra/docs/clang-tidy/checks/misc/const-correctness.rst +++ b/clang-tools-extra/docs/clang-tidy/checks/misc/const-correctness.rst @@ -4,12 +4,12 @@ ====================== This check implements detection of local variables which could be declared as -``const``, but are not. Declaring variables as ``const`` is required or recommended by many +``const`` but are not. Declaring variables as ``const`` is required or recommended by many coding guidelines, such as: `CppCoreGuidelines ES.25 `_ and `AUTOSAR C++14 Rule A7-1-1 (6.7.1 Specifiers) `_. -Please note that this analysis is type-based only. Variables that are not modified +Please note that this check's analysis is type-based only. Variables that are not modified but used to create a non-const handle that might escape the scope are not diagnosed as potential ``const``. @@ -18,33 +18,37 @@ // 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; + int result = i * i; // Before transformation + int const result = i * i; // After transformation -The check can analyzes values, pointers and references but not (yet) pointees: +The check can analyze values, pointers and references but not (yet) pointees: .. code-block:: c++ // Normal values like built-ins or objects. - int potential_const_int = 42; // 'const int potential_const_int = 42' suggestion. + int potential_const_int = 42; // Before transformation + int const potential_const_int = 42; // After transformation int copy_of_value = potential_const_int; - MyClass could_be_const; // 'const MyClass could_be_const' suggestion; + MyClass could_be_const; // Before transformation + MyClass const could_be_const; // After transformation could_be_const.const_qualified_method(); // References can be declared const as well. - int &reference_value = potential_const_int; // 'const int &reference_value' suggestion. + int &reference_value = potential_const_int; // Before transformation + int const& reference_value = potential_const_int; // After transformation int another_copy = reference_value; // The similar semantics of pointers are not (yet) analyzed. - int *pointer_variable = &potential_const_int; // Not 'const int *pointer_variable' suggestion. + int *pointer_variable = &potential_const_int; // _NO_ 'const int *pointer_variable' suggestion. int last_copy = *pointer_variable; The automatic code transformation is only applied to variables that are declared in single declarations. You may want to prepare your code base with -`readability-isolate-declaration `_ first. +`readability-isolate-declaration <../readability/isolate-declaration.html>`_ first. Note that there is the check -`cppcoreguidelines-avoid-non-const-global-variables `_ +`cppcoreguidelines-avoid-non-const-global-variables <../cppcoreguidelines/avoid-non-const-global-variables.html>`_ to enforce ``const`` correctness on all globals. Known Limitations @@ -52,10 +56,10 @@ The check will not analyze templated variables or variables that are instantiation dependent. Different instantiations can result in different ``const`` correctness properties and in general it -is not possible to find all instantiations of a template. It might be used differently in an -independent translation unit. +is not possible to find all instantiations of a template. The template might be used differently in +an independent translation unit. -Pointees can not be analyzed for constness yet. The following code is shows this limitation. +Pointees can not be analyzed for constness yet. The following code shows this limitation. .. code-block:: c++ @@ -74,15 +78,35 @@ Options ------- -.. option:: AnalyzeValues (default = 1) +.. option:: AnalyzeValues (default = true) Enable or disable the analysis of ordinary value variables, like ``int i = 42;`` -.. option:: AnalyzeReferences (default = 1) + .. code-block:: c++ + + // Warning + int i = 42; + // No warning + int const i = 42; + + // Warning + int a[] = {42, 42, 42}; + // No warning + int const a[] = {42, 42, 42}; + +.. option:: AnalyzeReferences (default = true) Enable or disable the analysis of reference variables, like ``int &ref = i;`` -.. option:: WarnPointersAsValues (default = 0) + .. code-block:: c++ + + int i = 42; + // Warning + int& ref = i; + // No warning + int const& ref = i; + +.. option:: WarnPointersAsValues (default = false) This option enables the suggestion for ``const`` of the pointer itself. Pointer values have two possibilities to be ``const``, the pointer @@ -90,28 +114,36 @@ .. code-block:: c++ - const int value = 42; - const int * const pointer_variable = &value; + int value = 42; - // The following operations are forbidden for `pointer_variable`. - // *pointer_variable = 44; - // pointer_variable = nullptr; + // Warning + const int * pointer_variable = &value; + // No warning + const int *const pointer_variable = &value; -.. option:: TransformValues (default = 1) +.. option:: TransformValues (default = true) - Provides fixit-hints for value types that automatically adds ``const`` if its a single declaration. + Provides fixit-hints for value types that automatically add ``const`` if its a single declaration. .. code-block:: c++ - // Emits a hint for 'value' to become 'const int value = 42;'. + // Before int value = 42; + // After + int const value = 42; + + // Before + int a[] = {42, 42, 42}; + // After + int const a[] = {42, 42, 42}; + // Result is modified later in its life-time. No diagnostic and fixit hint will be emitted. int result = value * 3; result -= 10; -.. option:: TransformReferences (default = 1) +.. option:: TransformReferences (default = true) - Provides fixit-hints for reference types that automatically adds ``const`` if its a single + Provides fixit-hints for reference types that automatically add ``const`` if its a single declaration. .. code-block:: c++ @@ -120,31 +152,45 @@ // it, it can not be transformed (yet). int value = 42; // The reference 'ref_value' is not modified and can be made 'const int &ref_value = value;' + // Before int &ref_value = value; + // After + int const &ref_value = value; // Result is modified later in its life-time. No diagnostic and fixit hint will be emitted. int result = ref_value * 3; result -= 10; -.. option:: TransformPointersAsValues (default = 0) +.. option:: TransformPointersAsValues (default = false) Provides fixit-hints for pointers if their pointee is not changed. This does not analyze if the value-pointed-to is unchanged! - Requires 'WarnPointersAsValues' to be 1. + Requires 'WarnPointersAsValues' to be 'true'. .. code-block:: c++ int value = 42; - // Emits a hint that 'ptr_value' may become 'int *const ptr_value = &value' because its pointee - // is not changed. + + // Before + const int * pointer_variable = &value; + // After + const int *const pointer_variable = &value; + + // Before + const int * a[] = {&value, &value}; + // After + const int *const a[] = {&value, &value}; + + // Before int *ptr_value = &value; + // After + int *const ptr_value = &value; - int result = 100 * (*ptr_value); - // This modification of the pointee is still allowed and not analyzed/diagnosed. + int result = 100 * (*ptr_value); // Does not modify the pointer itself. + // This modification of the pointee is still allowed and not diagnosed. *ptr_value = 0; // The following pointer may not become a 'int *const'. int *changing_pointee = &value; changing_pointee = &result; - diff --git a/clang-tools-extra/test/clang-tidy/checkers/misc/const-correctness-pointer-as-values.cpp b/clang-tools-extra/test/clang-tidy/checkers/misc/const-correctness-pointer-as-values.cpp --- a/clang-tools-extra/test/clang-tidy/checkers/misc/const-correctness-pointer-as-values.cpp +++ b/clang-tools-extra/test/clang-tidy/checkers/misc/const-correctness-pointer-as-values.cpp @@ -10,4 +10,65 @@ double *p_local0 = &np_local0[1]; // CHECK-MESSAGES: [[@LINE-1]]:3: warning: variable 'p_local0' of type 'double *' can be declared 'const' // CHECK-FIXES: double *const p_local0 + + using doublePtr = double*; + using doubleArray = double[15]; + doubleArray np_local1; + doublePtr p_local1 = &np_local1[0]; + // CHECK-MESSAGES: [[@LINE-1]]:3: warning: variable 'p_local1' of type 'doublePtr' (aka 'double *') can be declared 'const' + // CHECK-FIXES: doublePtr const p_local1 +} + +void range_for() { + int np_local0[2] = {1, 2}; + int *p_local0[2] = {&np_local0[0], &np_local0[1]}; + // CHECK-MESSAGES: [[@LINE-1]]:3: warning: variable 'p_local0' of type 'int *[2]' can be declared 'const' + // CHECK-FIXES: int *const p_local0[2] + for (const int *p_local1 : p_local0) { + // CHECK-MESSAGES: [[@LINE-1]]:8: warning: variable 'p_local1' of type 'const int *' can be declared 'const' + // CHECK-FIXES: for (const int *const p_local1 : p_local0) + } + + int *p_local2[2] = {nullptr, nullptr}; + // CHECK-MESSAGES: [[@LINE-1]]:3: warning: variable 'p_local2' of type 'int *[2]' can be declared 'const' + // CHECK-FIXES: int *const p_local2[2] + for (const auto *con_ptr : p_local2) { + } + +} + +template +struct SmallVectorBase { + T data[4]; + void push_back(const T &el) {} + int size() const { return 4; } + T *begin() { return data; } + const T *begin() const { return data; } + T *end() { return data + 4; } + const T *end() const { return data + 4; } +}; + +template +struct SmallVector : SmallVectorBase {}; + +template +void EmitProtocolMethodList(T &&Methods) { + // Note: If the template is uninstantiated the analysis does not figure out, + // that p_local0 could be const. Not sure why, but probably bails because + // some expressions are type-dependent. + SmallVector p_local0; + // CHECK-MESSAGES: [[@LINE-1]]:3: warning: variable 'p_local0' of type 'SmallVector' can be declared 'const' + // CHECK-FIXES: SmallVector const p_local0 + SmallVector np_local0; + for (const auto *I : Methods) { + if (I == nullptr) + np_local0.push_back(I); + } + p_local0.size(); +} +void instantiate() { + int *p_local0[4] = {nullptr, nullptr, nullptr, nullptr}; + // CHECK-MESSAGES: [[@LINE-1]]:3: warning: variable 'p_local0' of type 'int *[4]' can be declared 'const' + // CHECK-FIXES: int *const p_local0[4] + EmitProtocolMethodList(p_local0); } diff --git a/clang-tools-extra/test/clang-tidy/checkers/misc/const-correctness-values.cpp b/clang-tools-extra/test/clang-tidy/checkers/misc/const-correctness-values.cpp --- a/clang-tools-extra/test/clang-tidy/checkers/misc/const-correctness-values.cpp +++ b/clang-tools-extra/test/clang-tidy/checkers/misc/const-correctness-values.cpp @@ -526,18 +526,11 @@ // CHECK-FIXES: int const p_local1[2] 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' - // CHECK-FIXES: int *const p_local2[2] - 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' - // CHECK-FIXES: int *const p_local3[2] - for (const auto *con_ptr : p_local3) { - } +void arrays_of_pointers_are_ignored() { + int *np_local0[2] = {nullptr, nullptr}; + // CHECK-NOT-FIXES: int * const np_local0[2] } inline void *operator new(decltype(sizeof(void *)), void *p) { return p; } @@ -908,41 +901,6 @@ sizeof(int[++N]); } -template -struct SmallVectorBase { - T data[4]; - void push_back(const T &el) {} - int size() const { return 4; } - T *begin() { return data; } - const T *begin() const { return data; } - T *end() { return data + 4; } - const T *end() const { return data + 4; } -}; - -template -struct SmallVector : SmallVectorBase {}; - -template -void EmitProtocolMethodList(T &&Methods) { - // Note: If the template is uninstantiated the analysis does not figure out, - // that p_local0 could be const. Not sure why, but probably bails because - // some expressions are type-dependent. - SmallVector p_local0; - // CHECK-MESSAGES: [[@LINE-1]]:3: warning: variable 'p_local0' of type 'SmallVector' can be declared 'const' - // CHECK-FIXES: SmallVector const p_local0 - SmallVector np_local0; - for (const auto *I : Methods) { - if (I == nullptr) - np_local0.push_back(I); - } - p_local0.size(); -} -void instantiate() { - int *p_local0[4] = {nullptr, nullptr, nullptr, nullptr}; - // CHECK-MESSAGES: [[@LINE-1]]:3: warning: variable 'p_local0' of type 'int *[4]' can be declared 'const' - // CHECK-FIXES: int *const p_local0[4] - EmitProtocolMethodList(p_local0); -} struct base { int member; };