[PATCH] D73548: [clang-tidy] Added option for disabling const qualifiers in readability-qualified-auto
aaron.ballman added inline comments. Comment at: clang-tools-extra/clang-tidy/readability/QualifiedAutoCheck.cpp:262 + << (Var->getType().isLocalVolatileQualified() ? "volatile " : "") + << Var->getName() << FixItHint::CreateInsertion(InsertPos, "const "); +} njames93 wrote: > aaron.ballman wrote: > > Not specific to this patch, so feel free to address in another one -- I > > think you should be passing `Var` instead of `Var->getName()` -- the > > diagnostics engine does magic to format the name properly for any > > `NamedDecl`, including proper quoting. (Similar applies elsewhere.) > I couldn't do that as it actually messes the quotes up: `'auto > &'TdNakedCRef''` Ah, good point. I was thinking about variable template specializations, but I'm not certain there's an actual issue with them -- it seems like we really want just the base identifier here. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D73548/new/ https://reviews.llvm.org/D73548 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D73548: [clang-tidy] Added option for disabling const qualifiers in readability-qualified-auto
This revision was automatically updated to reflect the committed changes.
njames93 marked an inline comment as done.
Closed by commit rG8a68c40a1bf2: [clang-tidy] Added option for disabling const
qualifiers in readability… (authored by njames93).
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D73548/new/
https://reviews.llvm.org/D73548
Files:
clang-tools-extra/clang-tidy/llvm/LLVMTidyModule.cpp
clang-tools-extra/clang-tidy/readability/QualifiedAutoCheck.cpp
clang-tools-extra/clang-tidy/readability/QualifiedAutoCheck.h
clang-tools-extra/docs/ReleaseNotes.rst
clang-tools-extra/docs/clang-tidy/checks/readability-qualified-auto.rst
clang-tools-extra/test/clang-tidy/checkers/llvm-qualified-auto.cpp
Index: clang-tools-extra/test/clang-tidy/checkers/llvm-qualified-auto.cpp
===
--- /dev/null
+++ clang-tools-extra/test/clang-tidy/checkers/llvm-qualified-auto.cpp
@@ -0,0 +1,21 @@
+// RUN: %check_clang_tidy %s llvm-qualified-auto %t
+
+// This check just ensures by default the llvm alias doesn't add const
+// qualifiers to decls, so no need to copy the entire test file from
+// readability-qualified-auto.
+
+int *getIntPtr();
+const int *getCIntPtr();
+
+void foo() {
+ auto NakedPtr = getIntPtr();
+ // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: 'auto NakedPtr' can be declared as 'auto *NakedPtr'
+ // CHECK-FIXES: {{^}} auto *NakedPtr = getIntPtr();
+ auto NakedConstPtr = getCIntPtr();
+ // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: 'auto NakedConstPtr' can be declared as 'const auto *NakedConstPtr'
+ // CHECK-FIXES: {{^}} const auto *NakedConstPtr = getCIntPtr();
+ auto *Ptr = getIntPtr();
+ auto *ConstPtr = getCIntPtr();
+ auto = *getIntPtr();
+ auto = *getCIntPtr();
+}
Index: clang-tools-extra/docs/clang-tidy/checks/readability-qualified-auto.rst
===
--- clang-tools-extra/docs/clang-tidy/checks/readability-qualified-auto.rst
+++ clang-tools-extra/docs/clang-tidy/checks/readability-qualified-auto.rst
@@ -3,23 +3,16 @@
readability-qualified-auto
==
-Adds pointer and ``const`` qualifications to ``auto``-typed variables that are deduced
-to pointers and ``const`` pointers.
+Adds pointer qualifications to ``auto``-typed variables that are deduced to
+pointers.
-`LLVM Coding Standards `_ advises to
-make it obvious if a ``auto`` typed variable is a pointer, constant pointer or
-constant reference. This check will transform ``auto`` to ``auto *`` when the
-type is deduced to be a pointer, as well as adding ``const`` when applicable to
-``auto`` pointers or references
+`LLVM Coding Standards `_
+advises to make it obvious if a ``auto`` typed variable is a pointer. This
+check will transform ``auto`` to ``auto *`` when the type is deduced to be a
+pointer.
.. code-block:: c++
- for (auto : MutatableContainer) {
-change(Data);
- }
- for (auto : ConstantContainer) {
-observe(Data);
- }
for (auto Data : MutatablePtrContainer) {
change(*Data);
}
@@ -31,12 +24,6 @@
.. code-block:: c++
- for (auto : MutatableContainer) {
-change(Data);
- }
- for (const auto : ConstantContainer) {
-observe(Data);
- }
for (auto *Data : MutatablePtrContainer) {
change(*Data);
}
@@ -44,21 +31,54 @@
observe(*Data);
}
-Note const volatile qualified types will retain their const and volatile qualifiers.
+Note ``const`` ``volatile`` qualified types will retain their ``const`` and
+``volatile`` qualifiers. Pointers to pointers will not be fully qualified.
.. code-block:: c++
- const auto Foo = cast(Baz1);
- const auto Bar = cast(Baz2);
- volatile auto FooBar = cast(Baz3);
+ const auto Foo = cast(Baz1);
+ const auto Bar = cast(Baz2);
+ volatile auto FooBar = cast(Baz3);
+ auto BarFoo = cast(Baz4);
Would be transformed into:
.. code-block:: c++
- auto *const Foo = cast(Baz1);
- const auto *const Bar = cast(Baz2);
- auto *volatile FooBar = cast(Baz3);
+ auto *const Foo = cast(Baz1);
+ const auto *const Bar = cast(Baz2);
+ auto *volatile FooBar = cast(Baz3);
+ auto *BarFoo = cast(Baz4);
+
+Options
+---
+
+.. option:: AddConstToQualified
+
+ When set to `1` the check will add const qualifiers variables defined as
+ ``auto *`` or ``auto &`` when applicable.
+ Default value is '1'.
+
+.. code-block:: c++
+
+ auto Foo1 = cast(Bar1);
+ auto *Foo2 = cast(Bar2);
+ auto = cast(Bar3);
+
+ If AddConstToQualified is set to `0`, it will be transformed into:
+
+.. code-block:: c++
+
+ const auto *Foo1 = cast(Bar1);
+ auto *Foo2 = cast(Bar2);
+ auto = cast(Bar3);
+
+ Otherwise it will be transformed into:
+
+.. code-block:: c++
+
+ const auto *Foo1 = cast(Bar1);
+ const auto *Foo2
[PATCH] D73548: [clang-tidy] Added option for disabling const qualifiers in readability-qualified-auto
njames93 marked 2 inline comments as done. njames93 added inline comments. Comment at: clang-tools-extra/clang-tidy/readability/QualifiedAutoCheck.cpp:262 + << (Var->getType().isLocalVolatileQualified() ? "volatile " : "") + << Var->getName() << FixItHint::CreateInsertion(InsertPos, "const "); +} aaron.ballman wrote: > Not specific to this patch, so feel free to address in another one -- I think > you should be passing `Var` instead of `Var->getName()` -- the diagnostics > engine does magic to format the name properly for any `NamedDecl`, including > proper quoting. (Similar applies elsewhere.) I couldn't do that as it actually messes the quotes up: `'auto &'TdNakedCRef''` Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D73548/new/ https://reviews.llvm.org/D73548 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D73548: [clang-tidy] Added option for disabling const qualifiers in readability-qualified-auto
aaron.ballman accepted this revision. aaron.ballman added a comment. This revision is now accepted and ready to land. LGTM! Comment at: clang-tools-extra/clang-tidy/readability/QualifiedAutoCheck.cpp:262 + << (Var->getType().isLocalVolatileQualified() ? "volatile " : "") + << Var->getName() << FixItHint::CreateInsertion(InsertPos, "const "); +} Not specific to this patch, so feel free to address in another one -- I think you should be passing `Var` instead of `Var->getName()` -- the diagnostics engine does magic to format the name properly for any `NamedDecl`, including proper quoting. (Similar applies elsewhere.) Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D73548/new/ https://reviews.llvm.org/D73548 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D73548: [clang-tidy] Added option for disabling const qualifiers in readability-qualified-auto
merge_guards_bot added a comment.
{icon check-circle color=green} Unit tests: pass. 62383 tests passed, 0 failed
and 839 were skipped.
{icon check-circle color=green} clang-tidy: pass.
{icon check-circle color=green} clang-format: pass.
Build artifacts
:
diff.json
,
clang-tidy.txt
,
clang-format.patch
,
CMakeCache.txt
,
console-log.txt
,
test-results.xml
//Pre-merge checks is in beta. Report issue
.
Please join beta .//
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D73548/new/
https://reviews.llvm.org/D73548
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D73548: [clang-tidy] Added option for disabling const qualifiers in readability-qualified-auto
njames93 updated this revision to Diff 241924.
njames93 added a comment.
- Small reformat
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D73548/new/
https://reviews.llvm.org/D73548
Files:
clang-tools-extra/clang-tidy/llvm/LLVMTidyModule.cpp
clang-tools-extra/clang-tidy/readability/QualifiedAutoCheck.cpp
clang-tools-extra/clang-tidy/readability/QualifiedAutoCheck.h
clang-tools-extra/docs/ReleaseNotes.rst
clang-tools-extra/docs/clang-tidy/checks/readability-qualified-auto.rst
clang-tools-extra/test/clang-tidy/checkers/llvm-qualified-auto.cpp
Index: clang-tools-extra/test/clang-tidy/checkers/llvm-qualified-auto.cpp
===
--- /dev/null
+++ clang-tools-extra/test/clang-tidy/checkers/llvm-qualified-auto.cpp
@@ -0,0 +1,21 @@
+// RUN: %check_clang_tidy %s llvm-qualified-auto %t
+
+// This check just ensures by default the llvm alias doesn't add const
+// qualifiers to decls, so no need to copy the entire test file from
+// readability-qualified-auto.
+
+int *getIntPtr();
+const int *getCIntPtr();
+
+void foo() {
+ auto NakedPtr = getIntPtr();
+ // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: 'auto NakedPtr' can be declared as 'auto *NakedPtr'
+ // CHECK-FIXES: {{^}} auto *NakedPtr = getIntPtr();
+ auto NakedConstPtr = getCIntPtr();
+ // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: 'auto NakedConstPtr' can be declared as 'const auto *NakedConstPtr'
+ // CHECK-FIXES: {{^}} const auto *NakedConstPtr = getCIntPtr();
+ auto *Ptr = getIntPtr();
+ auto *ConstPtr = getCIntPtr();
+ auto = *getIntPtr();
+ auto = *getCIntPtr();
+}
Index: clang-tools-extra/docs/clang-tidy/checks/readability-qualified-auto.rst
===
--- clang-tools-extra/docs/clang-tidy/checks/readability-qualified-auto.rst
+++ clang-tools-extra/docs/clang-tidy/checks/readability-qualified-auto.rst
@@ -3,23 +3,16 @@
readability-qualified-auto
==
-Adds pointer and ``const`` qualifications to ``auto``-typed variables that are deduced
-to pointers and ``const`` pointers.
+Adds pointer qualifications to ``auto``-typed variables that are deduced to
+pointers.
-`LLVM Coding Standards `_ advises to
-make it obvious if a ``auto`` typed variable is a pointer, constant pointer or
-constant reference. This check will transform ``auto`` to ``auto *`` when the
-type is deduced to be a pointer, as well as adding ``const`` when applicable to
-``auto`` pointers or references
+`LLVM Coding Standards `_
+advises to make it obvious if a ``auto`` typed variable is a pointer. This
+check will transform ``auto`` to ``auto *`` when the type is deduced to be a
+pointer.
.. code-block:: c++
- for (auto : MutatableContainer) {
-change(Data);
- }
- for (auto : ConstantContainer) {
-observe(Data);
- }
for (auto Data : MutatablePtrContainer) {
change(*Data);
}
@@ -31,12 +24,6 @@
.. code-block:: c++
- for (auto : MutatableContainer) {
-change(Data);
- }
- for (const auto : ConstantContainer) {
-observe(Data);
- }
for (auto *Data : MutatablePtrContainer) {
change(*Data);
}
@@ -44,21 +31,54 @@
observe(*Data);
}
-Note const volatile qualified types will retain their const and volatile qualifiers.
+Note ``const`` ``volatile`` qualified types will retain their ``const`` and
+``volatile`` qualifiers. Pointers to pointers will not be fully qualified.
.. code-block:: c++
- const auto Foo = cast(Baz1);
- const auto Bar = cast(Baz2);
- volatile auto FooBar = cast(Baz3);
+ const auto Foo = cast(Baz1);
+ const auto Bar = cast(Baz2);
+ volatile auto FooBar = cast(Baz3);
+ auto BarFoo = cast(Baz4);
Would be transformed into:
.. code-block:: c++
- auto *const Foo = cast(Baz1);
- const auto *const Bar = cast(Baz2);
- auto *volatile FooBar = cast(Baz3);
+ auto *const Foo = cast(Baz1);
+ const auto *const Bar = cast(Baz2);
+ auto *volatile FooBar = cast(Baz3);
+ auto *BarFoo = cast(Baz4);
+
+Options
+---
+
+.. option:: AddConstToQualified
+
+ When set to `1` the check will add const qualifiers variables defined as
+ ``auto *`` or ``auto &`` when applicable.
+ Default value is '1'.
+
+.. code-block:: c++
+
+ auto Foo1 = cast(Bar1);
+ auto *Foo2 = cast(Bar2);
+ auto = cast(Bar3);
+
+ If AddConstToQualified is set to `0`, it will be transformed into:
+
+.. code-block:: c++
+
+ const auto *Foo1 = cast(Bar1);
+ auto *Foo2 = cast(Bar2);
+ auto = cast(Bar3);
+
+ Otherwise it will be transformed into:
+
+.. code-block:: c++
+
+ const auto *Foo1 = cast(Bar1);
+ const auto *Foo2 = cast(Bar2);
+ const auto = cast(Bar3);
-This check helps to enforce this `LLVM Coding Standards recommendation
[PATCH] D73548: [clang-tidy] Added option for disabling const qualifiers in readability-qualified-auto
merge_guards_bot added a comment.
{icon times-circle color=red} Unit tests: fail. 62275 tests passed, 1 failed
and 827 were skipped.
failed: Clang.CodeGenOpenCL/amdgpu-features.cl
{icon check-circle color=green} clang-tidy: pass.
{icon times-circle color=red} clang-format: fail. Please format your changes
with clang-format by running `git-clang-format HEAD^` or applying this patch
.
Build artifacts
:
diff.json
,
clang-tidy.txt
,
clang-format.patch
,
CMakeCache.txt
,
console-log.txt
,
test-results.xml
//Pre-merge checks is in beta. Report issue
.
Please join beta .//
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D73548/new/
https://reviews.llvm.org/D73548
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D73548: [clang-tidy] Added option for disabling const qualifiers in readability-qualified-auto
njames93 updated this revision to Diff 241374.
njames93 added a comment.
- Streamline fixits
- Add documentation about double pointers, maybe a follow up patch to fix
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D73548/new/
https://reviews.llvm.org/D73548
Files:
clang-tools-extra/clang-tidy/llvm/LLVMTidyModule.cpp
clang-tools-extra/clang-tidy/readability/QualifiedAutoCheck.cpp
clang-tools-extra/clang-tidy/readability/QualifiedAutoCheck.h
clang-tools-extra/docs/ReleaseNotes.rst
clang-tools-extra/docs/clang-tidy/checks/readability-qualified-auto.rst
clang-tools-extra/test/clang-tidy/checkers/llvm-qualified-auto.cpp
Index: clang-tools-extra/test/clang-tidy/checkers/llvm-qualified-auto.cpp
===
--- /dev/null
+++ clang-tools-extra/test/clang-tidy/checkers/llvm-qualified-auto.cpp
@@ -0,0 +1,21 @@
+// RUN: %check_clang_tidy %s llvm-qualified-auto %t
+
+// This check just ensures by default the llvm alias doesn't add const
+// qualifiers to decls, so no need to copy the entire test file from
+// readability-qualified-auto.
+
+int *getIntPtr();
+const int *getCIntPtr();
+
+void foo() {
+ auto NakedPtr = getIntPtr();
+ // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: 'auto NakedPtr' can be declared as 'auto *NakedPtr'
+ // CHECK-FIXES: {{^}} auto *NakedPtr = getIntPtr();
+ auto NakedConstPtr = getCIntPtr();
+ // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: 'auto NakedConstPtr' can be declared as 'const auto *NakedConstPtr'
+ // CHECK-FIXES: {{^}} const auto *NakedConstPtr = getCIntPtr();
+ auto *Ptr = getIntPtr();
+ auto *ConstPtr = getCIntPtr();
+ auto = *getIntPtr();
+ auto = *getCIntPtr();
+}
Index: clang-tools-extra/docs/clang-tidy/checks/readability-qualified-auto.rst
===
--- clang-tools-extra/docs/clang-tidy/checks/readability-qualified-auto.rst
+++ clang-tools-extra/docs/clang-tidy/checks/readability-qualified-auto.rst
@@ -3,23 +3,16 @@
readability-qualified-auto
==
-Adds pointer and ``const`` qualifications to ``auto``-typed variables that are deduced
-to pointers and ``const`` pointers.
+Adds pointer qualifications to ``auto``-typed variables that are deduced to
+pointers.
-`LLVM Coding Standards `_ advises to
-make it obvious if a ``auto`` typed variable is a pointer, constant pointer or
-constant reference. This check will transform ``auto`` to ``auto *`` when the
-type is deduced to be a pointer, as well as adding ``const`` when applicable to
-``auto`` pointers or references
+`LLVM Coding Standards `_
+advises to make it obvious if a ``auto`` typed variable is a pointer. This
+check will transform ``auto`` to ``auto *`` when the type is deduced to be a
+pointer.
.. code-block:: c++
- for (auto : MutatableContainer) {
-change(Data);
- }
- for (auto : ConstantContainer) {
-observe(Data);
- }
for (auto Data : MutatablePtrContainer) {
change(*Data);
}
@@ -31,12 +24,6 @@
.. code-block:: c++
- for (auto : MutatableContainer) {
-change(Data);
- }
- for (const auto : ConstantContainer) {
-observe(Data);
- }
for (auto *Data : MutatablePtrContainer) {
change(*Data);
}
@@ -44,21 +31,54 @@
observe(*Data);
}
-Note const volatile qualified types will retain their const and volatile qualifiers.
+Note ``const`` ``volatile`` qualified types will retain their ``const`` and
+``volatile`` qualifiers. Pointers to pointers will not be fully qualified.
.. code-block:: c++
- const auto Foo = cast(Baz1);
- const auto Bar = cast(Baz2);
- volatile auto FooBar = cast(Baz3);
+ const auto Foo = cast(Baz1);
+ const auto Bar = cast(Baz2);
+ volatile auto FooBar = cast(Baz3);
+ auto BarFoo = cast(Baz4);
Would be transformed into:
.. code-block:: c++
- auto *const Foo = cast(Baz1);
- const auto *const Bar = cast(Baz2);
- auto *volatile FooBar = cast(Baz3);
+ auto *const Foo = cast(Baz1);
+ const auto *const Bar = cast(Baz2);
+ auto *volatile FooBar = cast(Baz3);
+ auto *BarFoo = cast(Baz4);
+
+Options
+---
+
+.. option:: AddConstToQualified
+
+ When set to `1` the check will add const qualifiers variables defined as
+ ``auto *`` or ``auto &`` when applicable.
+ Default value is '1'.
+
+.. code-block:: c++
+
+ auto Foo1 = cast(Bar1);
+ auto *Foo2 = cast(Bar2);
+ auto = cast(Bar3);
+
+ If AddConstToQualified is set to `0`, it will be transformed into:
+
+.. code-block:: c++
+
+ const auto *Foo1 = cast(Bar1);
+ auto *Foo2 = cast(Bar2);
+ auto = cast(Bar3);
+
+ Otherwise it will be transformed into:
+
+.. code-block:: c++
+
+ const auto *Foo1 = cast(Bar1);
+ const auto *Foo2 = cast(Bar2);
+ const auto = cast(Bar3);
-This check helps to enforce this
[PATCH] D73548: [clang-tidy] Added option for disabling const qualifiers in readability-qualified-auto
aaron.ballman added inline comments. Comment at: clang-tools-extra/docs/clang-tidy/checks/readability-qualified-auto.rst:41 + auto *const Bar = cast(Baz2); + auto *volatile FooBar = cast(Baz3); + njames93 wrote: > aaron.ballman wrote: > > njames93 wrote: > > > Quuxplusone wrote: > > > > Is it worth adding an example of a double pointer? > > > > > > > > auto BarN = cast(FooN); > > > > > > > > Does that become `auto*` or `auto**` (and why)? My wild guess is that > > > > it becomes `auto*` (and because nobody cares about double pointers), > > > > but I could be wrong. > > > Double pointers are just resolved to `auto *`. > > They resolve to `auto *` today but I could see a real argument that they > > should resolve to `auto **` instead based on the same logic of: don't make > > the reader infer pointer/references and qualifiers. > One change per patch... I wasn't suggesting a change to this patch, I was checking to see if there's sentiment that we missed this case in the first place. Sorry for not making that more clear. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D73548/new/ https://reviews.llvm.org/D73548 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D73548: [clang-tidy] Added option for disabling const qualifiers in readability-qualified-auto
merge_guards_bot added a comment.
{icon times-circle color=red} Unit tests: fail. 62275 tests passed, 1 failed
and 827 were skipped.
failed: Clang.CodeGenOpenCL/amdgpu-features.cl
{icon check-circle color=green} clang-tidy: pass.
{icon check-circle color=green} clang-format: pass.
Build artifacts
:
diff.json
,
clang-tidy.txt
,
clang-format.patch
,
CMakeCache.txt
,
console-log.txt
,
test-results.xml
//Pre-merge checks is in beta. Report issue
.
Please join beta .//
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D73548/new/
https://reviews.llvm.org/D73548
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D73548: [clang-tidy] Added option for disabling const qualifiers in readability-qualified-auto
njames93 updated this revision to Diff 241082.
njames93 added a comment.
- Always add const to `auto` typed variable.
This update adds const to variables just typed with `auto`, but wont enforce
checking on `auto *` or `auto &` unless `AddConstToQualified` is set.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D73548/new/
https://reviews.llvm.org/D73548
Files:
clang-tools-extra/clang-tidy/llvm/LLVMTidyModule.cpp
clang-tools-extra/clang-tidy/readability/QualifiedAutoCheck.cpp
clang-tools-extra/clang-tidy/readability/QualifiedAutoCheck.h
clang-tools-extra/docs/ReleaseNotes.rst
clang-tools-extra/docs/clang-tidy/checks/readability-qualified-auto.rst
clang-tools-extra/test/clang-tidy/checkers/llvm-qualified-auto.cpp
Index: clang-tools-extra/test/clang-tidy/checkers/llvm-qualified-auto.cpp
===
--- /dev/null
+++ clang-tools-extra/test/clang-tidy/checkers/llvm-qualified-auto.cpp
@@ -0,0 +1,21 @@
+// RUN: %check_clang_tidy %s llvm-qualified-auto %t
+
+// This check just ensures by default the llvm alias doesn't add const
+// qualifiers to decls, so no need to copy the entire test file from
+// readability-qualified-auto.
+
+int *getIntPtr();
+const int *getCIntPtr();
+
+void foo() {
+ auto NakedPtr = getIntPtr();
+ // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: 'auto NakedPtr' can be declared as 'auto *NakedPtr'
+ // CHECK-FIXES: {{^}} auto *NakedPtr = getIntPtr();
+ auto NakedConstPtr = getCIntPtr();
+ // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: 'auto NakedConstPtr' can be declared as 'const auto *NakedConstPtr'
+ // CHECK-FIXES: {{^}} const auto *NakedConstPtr = getCIntPtr();
+ auto *Ptr = getIntPtr();
+ auto *ConstPtr = getCIntPtr();
+ auto = *getIntPtr();
+ auto = *getCIntPtr();
+}
Index: clang-tools-extra/docs/clang-tidy/checks/readability-qualified-auto.rst
===
--- clang-tools-extra/docs/clang-tidy/checks/readability-qualified-auto.rst
+++ clang-tools-extra/docs/clang-tidy/checks/readability-qualified-auto.rst
@@ -3,23 +3,16 @@
readability-qualified-auto
==
-Adds pointer and ``const`` qualifications to ``auto``-typed variables that are deduced
-to pointers and ``const`` pointers.
+Adds pointer qualifications to ``auto``-typed variables that are deduced to
+pointers.
-`LLVM Coding Standards `_ advises to
-make it obvious if a ``auto`` typed variable is a pointer, constant pointer or
-constant reference. This check will transform ``auto`` to ``auto *`` when the
-type is deduced to be a pointer, as well as adding ``const`` when applicable to
-``auto`` pointers or references
+`LLVM Coding Standards `_
+advises to make it obvious if a ``auto`` typed variable is a pointer. This
+check will transform ``auto`` to ``auto *`` when the type is deduced to be a
+pointer.
.. code-block:: c++
- for (auto : MutatableContainer) {
-change(Data);
- }
- for (auto : ConstantContainer) {
-observe(Data);
- }
for (auto Data : MutatablePtrContainer) {
change(*Data);
}
@@ -31,12 +24,6 @@
.. code-block:: c++
- for (auto : MutatableContainer) {
-change(Data);
- }
- for (const auto : ConstantContainer) {
-observe(Data);
- }
for (auto *Data : MutatablePtrContainer) {
change(*Data);
}
@@ -48,17 +35,47 @@
.. code-block:: c++
- const auto Foo = cast(Baz1);
- const auto Bar = cast(Baz2);
- volatile auto FooBar = cast(Baz3);
+ const auto Foo = cast(Baz1);
+ const auto Bar = cast(Baz2);
+ volatile auto FooBar = cast(Baz3);
Would be transformed into:
.. code-block:: c++
- auto *const Foo = cast(Baz1);
- const auto *const Bar = cast(Baz2);
- auto *volatile FooBar = cast(Baz3);
+ auto *const Foo = cast(Baz1);
+ const auto *const Bar = cast(Baz2);
+ auto *volatile FooBar = cast(Baz3);
+
+Options
+---
+
+.. option:: AddConstToQualified
+
+ When set to `1` the check will add const qualifiers variables defined as
+ ``auto *`` or ``auto &`` when applicable.
+ Default value is '1'.
+
+.. code-block:: c++
+
+ auto Foo1 = cast(Bar1);
+ auto *Foo2 = cast(Bar2);
+ auto = cast(Bar3);
+
+ If AddConstToQualified is set to `0`, it will be transformed into:
+
+.. code-block:: c++
+
+ const auto *Foo1 = cast(Bar1);
+ auto *Foo2 = cast(Bar2);
+ auto = cast(Bar3);
+
+ Otherwise it will be transformed into:
+
+.. code-block:: c++
+
+ const auto *Foo1 = cast(Bar1);
+ const auto *Foo2 = cast(Bar2);
+ const auto = cast(Bar3);
-This check helps to enforce this `LLVM Coding Standards recommendation
-`_.
+ Note in the LLVM alias, the default value is `0`.
Index: clang-tools-extra/docs/ReleaseNotes.rst
[PATCH] D73548: [clang-tidy] Added option for disabling const qualifiers in readability-qualified-auto
njames93 marked 4 inline comments as done. njames93 added inline comments. Comment at: clang-tools-extra/docs/clang-tidy/checks/readability-qualified-auto.rst:41 + auto *const Bar = cast(Baz2); + auto *volatile FooBar = cast(Baz3); + aaron.ballman wrote: > njames93 wrote: > > Quuxplusone wrote: > > > Is it worth adding an example of a double pointer? > > > > > > auto BarN = cast(FooN); > > > > > > Does that become `auto*` or `auto**` (and why)? My wild guess is that it > > > becomes `auto*` (and because nobody cares about double pointers), but I > > > could be wrong. > > Double pointers are just resolved to `auto *`. > They resolve to `auto *` today but I could see a real argument that they > should resolve to `auto **` instead based on the same logic of: don't make > the reader infer pointer/references and qualifiers. One change per patch... Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D73548/new/ https://reviews.llvm.org/D73548 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D73548: [clang-tidy] Added option for disabling const qualifiers in readability-qualified-auto
aaron.ballman added inline comments. Comment at: clang-tools-extra/docs/clang-tidy/checks/readability-qualified-auto.rst:41 + auto *const Bar = cast(Baz2); + auto *volatile FooBar = cast(Baz3); + njames93 wrote: > Quuxplusone wrote: > > Is it worth adding an example of a double pointer? > > > > auto BarN = cast(FooN); > > > > Does that become `auto*` or `auto**` (and why)? My wild guess is that it > > becomes `auto*` (and because nobody cares about double pointers), but I > > could be wrong. > Double pointers are just resolved to `auto *`. They resolve to `auto *` today but I could see a real argument that they should resolve to `auto **` instead based on the same logic of: don't make the reader infer pointer/references and qualifiers. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D73548/new/ https://reviews.llvm.org/D73548 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D73548: [clang-tidy] Added option for disabling const qualifiers in readability-qualified-auto
njames93 marked 7 inline comments as done. njames93 added inline comments. Comment at: clang-tools-extra/docs/clang-tidy/checks/readability-qualified-auto.rst:41 + auto *const Bar = cast(Baz2); + auto *volatile FooBar = cast(Baz3); + Quuxplusone wrote: > Is it worth adding an example of a double pointer? > > auto BarN = cast(FooN); > > Does that become `auto*` or `auto**` (and why)? My wild guess is that it > becomes `auto*` (and because nobody cares about double pointers), but I could > be wrong. Double pointers are just resolved to `auto *`. Comment at: clang-tools-extra/docs/clang-tidy/checks/readability-qualified-auto.rst:64 + + Otherwise it will get be transformed into: + Quuxplusone wrote: > s/Will be/will be/ > s/will get be/will be/ > > In the preceding section you give an example with `volatile`. How about here? > What happens with > > auto *volatile Foo3 = cast(Bar3); > auto *Foo4 = cast(Bar4); > > Do they become > > const auto *volatile Foo3 = cast(Bar3); > volatile auto *Foo4 = cast(Bar4); > > as I would expect? Good spot on the Will/be/get/(whatever I tried to type) the check keeps local volatile qualifiers, but it wont add volatile pointer (or ref) qualifiers even if the deduced type is a volatile pointer. In the first patch we came to the conclusion not to add them. Comment at: clang-tools-extra/docs/clang-tidy/checks/readability-qualified-auto.rst:69 + const auto *Foo1 = cast(Bar1); + const auto = cast(Bar2); + Quuxplusone wrote: > How does this option interact with > > const int range[10]; > for (auto&& elt : range) > > Does it (incorrectly IMHO) make that `const auto&& elt`, or (correctly IMHO) > `const auto& elt`, or (also correctly) leave it as [`auto&&`, which Always > Works](https://quuxplusone.github.io/blog/2018/12/15/autorefref-always-works/)? RValueReferences are ignored Comment at: clang-tools-extra/test/clang-tidy/checkers/llvm-qualified-auto.cpp:20 + auto = *getIntPtr(); + auto = *getCIntPtr(); +} Quuxplusone wrote: > It is worth adding one test case to show that the LLVM alias does not > gratuitously //remove// `const` from declarations: > > auto const = getCIntPtr(); > // becomes > auto *const = getCIntPtr(); That test case looks malformed as the function getCIntPtr() doesn't return a reference. as for removing const etc those cases are all in the `readability-qualified-auto.cpp` test file Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D73548/new/ https://reviews.llvm.org/D73548 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D73548: [clang-tidy] Added option for disabling const qualifiers in readability-qualified-auto
Quuxplusone added inline comments. Comment at: clang-tools-extra/docs/clang-tidy/checks/readability-qualified-auto.rst:41 + auto *const Bar = cast(Baz2); + auto *volatile FooBar = cast(Baz3); + Is it worth adding an example of a double pointer? auto BarN = cast(FooN); Does that become `auto*` or `auto**` (and why)? My wild guess is that it becomes `auto*` (and because nobody cares about double pointers), but I could be wrong. Comment at: clang-tools-extra/docs/clang-tidy/checks/readability-qualified-auto.rst:64 + + Otherwise it will get be transformed into: + s/Will be/will be/ s/will get be/will be/ In the preceding section you give an example with `volatile`. How about here? What happens with auto *volatile Foo3 = cast(Bar3); auto *Foo4 = cast(Bar4); Do they become const auto *volatile Foo3 = cast(Bar3); volatile auto *Foo4 = cast(Bar4); as I would expect? Comment at: clang-tools-extra/docs/clang-tidy/checks/readability-qualified-auto.rst:69 + const auto *Foo1 = cast(Bar1); + const auto = cast(Bar2); + How does this option interact with const int range[10]; for (auto&& elt : range) Does it (incorrectly IMHO) make that `const auto&& elt`, or (correctly IMHO) `const auto& elt`, or (also correctly) leave it as [`auto&&`, which Always Works](https://quuxplusone.github.io/blog/2018/12/15/autorefref-always-works/)? Comment at: clang-tools-extra/test/clang-tidy/checkers/llvm-qualified-auto.cpp:20 + auto = *getIntPtr(); + auto = *getCIntPtr(); +} It is worth adding one test case to show that the LLVM alias does not gratuitously //remove// `const` from declarations: auto const = getCIntPtr(); // becomes auto *const = getCIntPtr(); Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D73548/new/ https://reviews.llvm.org/D73548 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D73548: [clang-tidy] Added option for disabling const qualifiers in readability-qualified-auto
merge_guards_bot added a comment.
{icon check-circle color=green} Unit tests: pass. 62260 tests passed, 0 failed
and 827 were skipped.
{icon check-circle color=green} clang-tidy: pass.
{icon check-circle color=green} clang-format: pass.
Build artifacts
:
diff.json
,
clang-tidy.txt
,
clang-format.patch
,
CMakeCache.txt
,
console-log.txt
,
test-results.xml
//Pre-merge checks is in beta. Report issue
.
Please join beta .//
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D73548/new/
https://reviews.llvm.org/D73548
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D73548: [clang-tidy] Added option for disabling const qualifiers in readability-qualified-auto
njames93 marked an inline comment as done. njames93 added inline comments. Comment at: clang-tools-extra/clang-tidy/readability/QualifiedAutoCheck.cpp:209 + llvm::StringRef PtrConst = + (AddConstQualifier && isPointerConst(Var->getType())) ? "const " : ""; llvm::StringRef LocalConst = IsLocalConst ? "const " : ""; If a variables is typed as `auto x = cast(y)`, should the behaviour be `const auto*x = ...` even if AddConstQualifier is turned off. Thereby making the AddConstQualifier check only there to ensure that variables already typed as `auto *` and `auto &` get const qualifier if necessary Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D73548/new/ https://reviews.llvm.org/D73548 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D73548: [clang-tidy] Added option for disabling const qualifiers in readability-qualified-auto
lebedev.ri added a comment. SGTM, but please wait for the reviewers. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D73548/new/ https://reviews.llvm.org/D73548 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D73548: [clang-tidy] Added option for disabling const qualifiers in readability-qualified-auto
njames93 created this revision.
Herald added subscribers: cfe-commits, xazax.hun.
Herald added a project: clang.
Adds an option called `AddConstQualifier` to readability-qualified-auto to
toggle adding const to the auto typed pointers and references. By default its
enabled but in the LLVM module its disabled.
Repository:
rG LLVM Github Monorepo
https://reviews.llvm.org/D73548
Files:
clang-tools-extra/clang-tidy/llvm/LLVMTidyModule.cpp
clang-tools-extra/clang-tidy/readability/QualifiedAutoCheck.cpp
clang-tools-extra/clang-tidy/readability/QualifiedAutoCheck.h
clang-tools-extra/docs/ReleaseNotes.rst
clang-tools-extra/docs/clang-tidy/checks/readability-qualified-auto.rst
clang-tools-extra/test/clang-tidy/checkers/llvm-qualified-auto.cpp
Index: clang-tools-extra/test/clang-tidy/checkers/llvm-qualified-auto.cpp
===
--- /dev/null
+++ clang-tools-extra/test/clang-tidy/checkers/llvm-qualified-auto.cpp
@@ -0,0 +1,21 @@
+// RUN: %check_clang_tidy %s llvm-qualified-auto %t
+
+// This check just ensures by default the llvm alias doesn't add const
+// qualifiers to decls, so no need to copy the entire test file from
+// readability-qualified-auto.
+
+int *getIntPtr();
+const int *getCIntPtr();
+
+void foo() {
+ auto NakedPtr = getIntPtr();
+ // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: 'auto NakedPtr' can be declared as 'auto *NakedPtr'
+ // CHECK-FIXES: {{^}} auto *NakedPtr = getIntPtr();
+ auto NakedConstPtr = getCIntPtr();
+ // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: 'auto NakedConstPtr' can be declared as 'auto *NakedConstPtr'
+ // CHECK-FIXES: {{^}} auto *NakedConstPtr = getCIntPtr();
+ auto *Ptr = getIntPtr();
+ auto *ConstPtr = getCIntPtr();
+ auto = *getIntPtr();
+ auto = *getCIntPtr();
+}
Index: clang-tools-extra/docs/clang-tidy/checks/readability-qualified-auto.rst
===
--- clang-tools-extra/docs/clang-tidy/checks/readability-qualified-auto.rst
+++ clang-tools-extra/docs/clang-tidy/checks/readability-qualified-auto.rst
@@ -3,62 +3,72 @@
readability-qualified-auto
==
-Adds pointer and ``const`` qualifications to ``auto``-typed variables that are deduced
-to pointers and ``const`` pointers.
+Adds pointer qualifications to ``auto``-typed variables that are deduced to
+pointers.
`LLVM Coding Standards `_ advises to
-make it obvious if a ``auto`` typed variable is a pointer, constant pointer or
-constant reference. This check will transform ``auto`` to ``auto *`` when the
-type is deduced to be a pointer, as well as adding ``const`` when applicable to
-``auto`` pointers or references
+make it obvious if a ``auto`` typed variable is a pointer. This check will
+transform ``auto`` to ``auto *`` when the type is deduced to be a pointer.
.. code-block:: c++
- for (auto : MutatableContainer) {
-change(Data);
- }
- for (auto : ConstantContainer) {
-observe(Data);
- }
for (auto Data : MutatablePtrContainer) {
change(*Data);
}
- for (auto Data : ConstantPtrContainer) {
-observe(*Data);
- }
Would be transformed into:
.. code-block:: c++
- for (auto : MutatableContainer) {
-change(Data);
- }
- for (const auto : ConstantContainer) {
-observe(Data);
- }
for (auto *Data : MutatablePtrContainer) {
change(*Data);
}
- for (const auto *Data : ConstantPtrContainer) {
-observe(*Data);
- }
Note const volatile qualified types will retain their const and volatile qualifiers.
.. code-block:: c++
- const auto Foo = cast(Baz1);
- const auto Bar = cast(Baz2);
- volatile auto FooBar = cast(Baz3);
+ const auto Foo = cast(Baz1);
+ const auto Bar = cast(Baz2);
+ volatile auto FooBar = cast(Baz3);
Would be transformed into:
.. code-block:: c++
- auto *const Foo = cast(Baz1);
- const auto *const Bar = cast(Baz2);
- auto *volatile FooBar = cast(Baz3);
+ auto *const Foo = cast(Baz1);
+ auto *const Bar = cast(Baz2);
+ auto *volatile FooBar = cast(Baz3);
+
+Options
+---
+
+.. option:: AddConstQualifier
+
+ When set to `1` the check will add const qualifiers to auto typed pointers
+ or references.
+ Default value is '1'.
+
+.. code-block:: c++
+
+ auto Foo1 = cast(Bar1);
+ auto = cast(Bar2);
+
+ If AddConstQualifier is set to `0`, Will be transformed into:
+
+.. code-block:: c++
+
+ auto *Foo1 = cast(Bar1);
+ auto = cast(Bar2);
+
+ Otherwise it will get be transformed into:
+
+.. code-block:: c++
+
+ const auto *Foo1 = cast(Bar1);
+ const auto = cast(Bar2);
+
+ Note in the LLVM alias, the default value is `0`.
This check helps to enforce this `LLVM Coding Standards recommendation
`_.
Index: clang-tools-extra/docs/ReleaseNotes.rst
