[PATCH] D73548: [clang-tidy] Added option for disabling const qualifiers in readability-qualified-auto
Nathan James via Phabricator via cfe-commits
cfe-commits at lists.llvm.org
Tue Jan 28 08:40:17 PST 2020
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<const int *>(Baz2);
+ auto *volatile FooBar = cast<int *>(Baz3);
+
----------------
Quuxplusone wrote:
> Is it worth adding an example of a double pointer?
>
> auto BarN = cast<int **>(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<const int *>(Bar3);
> auto *Foo4 = cast<volatile int *>(Bar4);
>
> Do they become
>
> const auto *volatile Foo3 = cast<const int *>(Bar3);
> volatile auto *Foo4 = cast<volatile int *>(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<const int *>(Bar1);
+ const auto &Foo2 = cast<const int &>(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 &NakedRef = *getIntPtr();
+ auto &NakedConstRef = *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 &ExtendedConstPtr = getCIntPtr();
> // becomes
> auto *const &ExtendedConstPtr = 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
More information about the cfe-commits
mailing list