JonasToth updated this revision to Diff 450358.
JonasToth added a comment.
- add test with typedef
- [docs] improve documentation for misc-const-correctness
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D130793/new/
https://reviews.llvm.org/D130793
Files:
clang-tools-extra/clang-tidy/misc/ConstCorrectnessCheck.cpp
clang-tools-extra/docs/clang-tidy/checks/misc/const-correctness.rst
clang-tools-extra/test/clang-tidy/checkers/misc/const-correctness-pointer-as-values.cpp
clang-tools-extra/test/clang-tidy/checkers/misc/const-correctness-values.cpp
Index: clang-tools-extra/test/clang-tidy/checkers/misc/const-correctness-values.cpp
===================================================================
--- clang-tools-extra/test/clang-tidy/checkers/misc/const-correctness-values.cpp
+++ 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 <typename T>
-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 <typename T>
-struct SmallVector : SmallVectorBase<T> {};
-
-template <class T>
-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<const int *> p_local0;
- // CHECK-MESSAGES: [[@LINE-1]]:3: warning: variable 'p_local0' of type 'SmallVector<const int *>' can be declared 'const'
- // CHECK-FIXES: SmallVector<const int *> const p_local0
- SmallVector<const int *> 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;
};
Index: clang-tools-extra/test/clang-tidy/checkers/misc/const-correctness-pointer-as-values.cpp
===================================================================
--- clang-tools-extra/test/clang-tidy/checkers/misc/const-correctness-pointer-as-values.cpp
+++ 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 <typename T>
+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 <typename T>
+struct SmallVector : SmallVectorBase<T> {};
+
+template <class T>
+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<const int *> p_local0;
+ // CHECK-MESSAGES: [[@LINE-1]]:3: warning: variable 'p_local0' of type 'SmallVector<const int *>' can be declared 'const'
+ // CHECK-FIXES: SmallVector<const int *> const p_local0
+ SmallVector<const int *> 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);
}
Index: clang-tools-extra/docs/clang-tidy/checks/misc/const-correctness.rst
===================================================================
--- clang-tools-extra/docs/clang-tidy/checks/misc/const-correctness.rst
+++ 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 <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 `AUTOSAR C++14 Rule A7-1-1 (6.7.1 Specifiers) <https://www.autosar.org/fileadmin/user_upload/standards/adaptive/17-03/AUTOSAR_RS_CPP14Guidelines.pdf>`_.
-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 <readability-isolate-declaration.html>`_ 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.html>`_
+`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;
-
Index: clang-tools-extra/clang-tidy/misc/ConstCorrectnessCheck.cpp
===================================================================
--- clang-tools-extra/clang-tidy/misc/ConstCorrectnessCheck.cpp
+++ 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 <iostream>
@@ -132,6 +133,12 @@
VC = VariableCategory::Reference;
if (Variable->getType()->isPointerType())
VC = VariableCategory::Pointer;
+ if (Variable->getType()->isArrayType()) {
+ if (const auto *ArrayT = dyn_cast<ArrayType>(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.
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
