[clang-tidy] Added readability-qualified-auto check

[njames93]

Summary

Adds a check that detects any auto variables that are deduced to a pointer or a const pointer then adds in the const and asterisk according. Will also check auto L value references that could be written as const. This relates to the coding standard https://llvm.org/docs/CodingStandards.html#beware-unnecessary-copies-with-auto

[njames93]

side note when creating this, the add_new_check.py file hasn't been updated in relation to this commit https://github.com/llvm/llvm-project/commit/d2c9c9157b0528436d3b9f5e22c872f0ee6509a2. This results in a malformed rst file. In this patch I have just manually set it up correctly, but a fix will need to be made for the add_new_check.py

[Eugene.Zelenko]

May be check belong to LLVM module?

clang-tools-extra/clang-tidy/readability/QualifiedAutoCheck.cpp

70

Unnecessary empty line.

clang-tools-extra/docs/clang-tidy/checks/readability-qualified-auto.rst

7

Please make first statement same as Release Notes.

14

Please separate with empty line.

26

Unnecessary empty line.

31

Please separate with empty line.

44

Unnecessary empty line.

[Eugene.Zelenko]

clang-tools-extra/clang-tidy/readability/QualifiedAutoCheck.cpp

47

Please separate with empty line.

[lebedev.ri]

Oh, thank you for working on this one, i've always missed this particular check.

Given that there's finally progress on const-correctness check,
should this only handle adding */&, and leave const alone?

In D72217#1804628, @Eugene.Zelenko wrote:

May be check belong to LLVM module?

I wouldn't say this is really llvm-specific.

[Mordante]

Nice to see this patch! I intended to look at a similar patch, but that's no longer required.

One question does it also handle cases like std::vector<int **> PtrPtrVec; properly?

clang-tools-extra/test/clang-tidy/checkers/readability-qualified-auto.cpp

20

Wouldn't it be nicer to write 'auto NakedPtr' can be declared as an 'auto *NakedPtr' ?
(Likewise for the other warnings.)

24

I looks like the fix adds an extra space character after the * in auto * NakedCPtr

28

I looks like the fix adds an extra space character, giving two spaces in const ConstPtr

[njames93]

I'll address those issues tonight. As for the ** case, I'll. Hadn't even thought of that, I'll try and sort that out

[sylvestre.ledru]

In D72217#1804533, @njames93 wrote:

side note when creating this, the add_new_check.py file hasn't been updated in relation to this commit https://github.com/llvm/llvm-project/commit/d2c9c9157b0528436d3b9f5e22c872f0ee6509a2. This results in a malformed rst file. In this patch I have just manually set it up correctly, but a fix will need to be made for the add_new_check.py

My bad, sorry, I will have a look!

[JonasToth]

+1 for the check from me as well :)

clang-tools-extra/clang-tidy/readability/QualifiedAutoCheck.cpp

20

Please make the comments full sentences with punctuation.

This if has only one statement, so please ellide the braces - by coding standard.

49

thats dangerous. Your source location could be invalid or a macroID, I would rather make these llvm::Optional-

62

the outer const might be debatable. some code-bases don't want the * const, i think neither does LLVM. maybe that could be configurable?

72

please make that comment a full sentence with punctuation and ellide the braces.

80

braces.

88

braces.

97

full sentence.

105

braces.

clang-tools-extra/docs/clang-tidy/checks/readability-qualified-auto.rst

46

I think this justifies an alias into the llvm-module we have.

clang-tools-extra/test/clang-tidy/checkers/readability-qualified-auto.cpp

54

please add some tests with typedefs and macro-interference in the definitions.

Typedefs i am interested in: (names are up to you, just to get the idea)

using MyPtr = int *;
using MyRef = int&;
using CMyPtr = const int *;
using CCMyPtr = const int * const;
using CMyRef = const int&;

I believe resolves the typedefs and becomes the reference/pointer, but that needs some test.

The transformation should not happen if the code comes from a macro ID

int* get_ptr() { return new int(42); }
#define AUTO auto
AUTO my_variable = get_pointer();

Transformation should not happen in this case, because the macro might be used in different places and macros are complicated.

101

What about

auto my_function -> int * { /* foo */ }

other contexts for auto need to be considered (lambdas, others?)

[njames93]

fixed some of the code guidelines issues. Will tackle some of the more pressing issues like ensuring correctness

[njames93]

I have tried to make the code more robust against macro decls. Also target the actual type specifier rather than everything before the name when doing the replacements. This should leave any other qualifiers or attributes in tact. The test cases have been updated to add checks for using and typedefs alias. There are also tests for function pointers and lambdas, with the current behaviour to add pointer qual for function pointer but not for lambdas(as they aren't function pointers under the hood). Not sure I want to implement support for pointers to pointers as that's not going to help readability(Are you a pointer to an array of arrays, array of pointers, pointer to array or pointer to pointer). As for the alias whats the correct procedure for creating an alias

[njames93]

What do you think about volatile qualifiers. Personally I don't think its important to qualify an volatile on a pointer that is volatile, but i should adhere to the decl being declared as volatile

volatile auto X = getPointer();
//transform to
auto *X volatile = getPointer();

auto X = getVolatilePointer();
//transform to
auto *X = getVolatilePointer();

clang-tools-extra/clang-tidy/readability/QualifiedAutoCheck.cpp

62

The outer const is only there if the vardecl was already declared as const

const auto X = getPointer();
auto *const X = getPointer();

Those 2 decls are functionally identical, both mean that the pointer X can't change but X is pointing to data that can change

[JonasToth]

clang-tools-extra/clang-tidy/readability/QualifiedAutoCheck.cpp

62

I see. That is ok then, but might be surprising to some users, as they might think the pointee itself is const (which it isn't). I think thats something worth to write in the documentation.

clang-tools-extra/test/clang-tidy/checkers/readability-qualified-auto.cpp

184

How does that beatiful code-construct get handled?

auto * function_ptr = +[](int) {};

The lambda is actually converted to a function pointer through the +-operator. Not sure this happens anywhere, but it can happen.
So the case:

auto unnotice_ptr = +[](int) {};

should be transformed.

[njames93]

Would you say this behaviour is better?

void baz() {
  auto MyFunctionPtr = getPtrFunction();
  // CHECK-MESSAGES-NOT: :[[@LINE-1]]:3: warning: 'auto MyFunctionPtr' can be declared as 'auto *MyFunctionPtr'
  // CHECK-FIXES-NOT: {{^}}  auto *MyFunctionPtr = getPtrFunction();
  auto MyFunctionVal = getValFunction();
  // CHECK-MESSAGES-NOT: :[[@LINE-1]]:3: warning: 'auto MyFunctionVal' can be declared as 'auto *MyFunctionVal'
  // CHECK-FIXES-NOT: {{^}}  auto *MyFunctionVal = getValFunction();

  auto LambdaTest = [] { return 0; };
  // CHECK-MESSAGES-NOT: :[[@LINE-1]]:3: warning: 'auto LambdaTest' can be declared as 'auto *LambdaTest'
  // CHECK-FIXES-NOT: {{^}}  auto *LambdaTest = [] { return 0; };#

  auto LambdaTest2 = +[] { return 0; };
  // CHECK-MESSAGES-NOT: :[[@LINE-1]]:3: warning: 'auto LambdaTest2' can be declared as 'auto *LambdaTest2'
  // CHECK-FIXES-NOT: {{^}}  auto *LambdaTest2 = +[] { return 0; };
}

clang-tools-extra/test/clang-tidy/checkers/readability-qualified-auto.cpp

184

It gets handled the same as retty (*)(params), I should probably make function pointers in general not be converted

[njames93]

adhere to coding guidelines, added check for local volatile qualifiers, disregard pointer addition on function pointer types

[njames93]

Added docs for const and volatile qualifiers

[JonasToth]

clang-tools-extra/test/clang-tidy/checkers/readability-qualified-auto.cpp

184

Ok.
There exist function references, too int (&my_reference) (int); ( i think that was the syntax).
They should then follow this as well (+ user-facing documentation).

[njames93]

clang-tools-extra/test/clang-tidy/checkers/readability-qualified-auto.cpp

184

You are right that there is a syntax like that, but if you try to bind one to an auto, it will decay to a pointer. If you bind one to an auto reference, then it will be just like the reference syntax above. In either case this won't trigger any replacements

[njames93]

Added an alias into the llvm module, LMK if I have done that incorrectly

[Eugene.Zelenko]

By the word, will be interesting to see results of this check run on LLVM code. Probably results should be split on modules.

clang-tools-extra/docs/ReleaseNotes.rst

145

Please move alias entry into aliases section. See previous release for proper order.

clang-tools-extra/docs/clang-tidy/checks/llvm-qualified-auto.rst

7

Please make length same as title.

[njames93]

In D72217#1809212, @Eugene.Zelenko wrote:

By the word, will be interesting to see results of this check run on LLVM code. Probably results should be split on modules.

So ran it on clang and clang-tidy just crashed, gonna debug it see whats happening

clang-tools-extra/docs/ReleaseNotes.rst

145

I looked at the previous release notes and from what i can see, all alias checks are next to each other in alphabetical order, but there is no special placement of the alias checks in the release notes, they just start at an arbitrary position.

[njames93]

In D72217#1809685, @njames93 wrote:

In D72217#1809212, @Eugene.Zelenko wrote:

By the word, will be interesting to see results of this check run on LLVM code. Probably results should be split on modules.

So ran it on clang and clang-tidy just crashed, gonna debug it see whats happening

Crash fixed heres what happened when I ran it on clang/lib

Quite a few occurances of const auto Decl which are renamed as const auto* const, or worse still a few that are redeclared as auto *const

/home/nathan/src/llvm-project/clang/lib/StaticAnalyzer/Checkers/MoveChecker.cpp:623:3: warning: 'const auto IC' can be declared as 'const auto *const IC' [readability-qualified-auto]
  const auto IC = dyn_cast<CXXInstanceCall>(&Call);
  ^~~~~~~~~~~
  const auto *const 
/home/nathan/src/llvm-project/clang/lib/StaticAnalyzer/Checkers/MoveChecker.cpp:636:3: warning: 'const auto MethodDecl' can be declared as 'const auto *const MethodDecl' [readability-qualified-auto]
  const auto MethodDecl = dyn_cast_or_null<CXXMethodDecl>(IC->getDecl());
  ^~~~~~~~~~~
  const auto *const

286 files changed, without looking at header files. One file failed to build which is due to a dependant template. My guess is one call will have returned a naked pointer, and another returned an iterator, maybe I should disregard dependant templates unless it would be possible to deduce its always a pointer

/home/nathan/src/llvm-project/clang/lib/StaticAnalyzer/Frontend/CheckerRegistry.cpp:365:9: error: variable 'It' with type 'auto *' has incompatible initializer of type 'typename std::conditional<std::is_const<vector<CheckerInfo, allocator<CheckerInfo> > >::value, typename vector<CheckerInfo, allocator<CheckerInfo> >::const_iterator, typename vector<CheckerInfo, allocator<CheckerInfo> >::iterator>::type' (aka '__gnu_cxx::__normal_iterator<clang::ento::CheckerRegistry::CheckerInfo *, std::vector<clang::ento::CheckerRegistry::CheckerInfo, std::allocator<clang::ento::CheckerRegistry::CheckerInfo> > >')
  auto *It = binaryFind(Collection, FullName);
        ^    ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
/home/nathan/src/llvm-project/clang/lib/StaticAnalyzer/Frontend/CheckerRegistry.cpp:378:5: note: in instantiation of function template specialization 'insertOptionToCollection<std::vector<clang::ento::CheckerRegistry::CheckerInfo, std::allocator<clang::ento::CheckerRegistry::CheckerInfo> > >' requested here
    insertOptionToCollection(CheckerOptEntry.first, Checkers,
    ^
1 error generated.

[JonasToth]

In D72217#1809721, @njames93 wrote:

In D72217#1809685, @njames93 wrote:

In D72217#1809212, @Eugene.Zelenko wrote:

By the word, will be interesting to see results of this check run on LLVM code. Probably results should be split on modules.

So ran it on clang and clang-tidy just crashed, gonna debug it see whats happening

Crash fixed heres what happened when I ran it on clang/lib

Quite a few occurances of const auto Decl which are renamed as const auto* const, or worse still a few that are redeclared as auto *const

/home/nathan/src/llvm-project/clang/lib/StaticAnalyzer/Checkers/MoveChecker.cpp:623:3: warning: 'const auto IC' can be declared as 'const auto *const IC' [readability-qualified-auto]
  const auto IC = dyn_cast<CXXInstanceCall>(&Call);
  ^~~~~~~~~~~
  const auto *const 
/home/nathan/src/llvm-project/clang/lib/StaticAnalyzer/Checkers/MoveChecker.cpp:636:3: warning: 'const auto MethodDecl' can be declared as 'const auto *const MethodDecl' [readability-qualified-auto]
  const auto MethodDecl = dyn_cast_or_null<CXXMethodDecl>(IC->getDecl());
  ^~~~~~~~~~~
  const auto *const

286 files changed, without looking at header files. One file failed to build which is due to a dependant template. My guess is one call will have returned a naked pointer, and another returned an iterator, maybe I should disregard dependant templates unless it would be possible to deduce its always a pointer

Yes, templates are usually not so nice to handle well :/
One unfortunate thing I am currently looking for is checking if auto->deducedType() is a tempateTypeParmType or substTemplateTypeParmType through matchers, I failed. But in a sense you need to check for that condition for the cases auto -> direct; auto & -> pointee; auto * -> pointee.
From my experience it helps to run it over llvm/lib, too as there is a lot of interesting c++ code in their. My checks broke there more often.

[JonasToth]

In D72217#1807941, @njames93 wrote:

What do you think about volatile qualifiers. Personally I don't think its important to qualify an volatile on a pointer that is volatile, but i should adhere to the decl being declared as volatile

volatile auto X = getPointer();
//transform to
auto *X volatile = getPointer();

auto X = getVolatilePointer();
//transform to
auto *X = getVolatilePointer();

Probably better to stay consistent, especially with volatile not being so obvious.

[JonasToth]

clang-tools-extra/docs/clang-tidy/checks/list.rst

406

That line needs to land above where the other LLVM checks are.

clang-tools-extra/test/clang-tidy/checkers/readability-qualified-auto.cpp

2

please split the tests to one that is c++11 (check_clang_tidy should default to that) and one that has only c++17 features.

204

Please add function references as test anyway, even though there is no new behaviour.

[Eugene.Zelenko]

clang-tools-extra/docs/ReleaseNotes.rst

145

No, aliases are after new checks. See http://releases.llvm.org/9.0.0/tools/clang/tools/extra/docs/ReleaseNotes.html.

[njames93]

In D72217#1809887, @JonasToth wrote:

In D72217#1809721, @njames93 wrote:

In D72217#1809685, @njames93 wrote:

In D72217#1809212, @Eugene.Zelenko wrote:

By the word, will be interesting to see results of this check run on LLVM code. Probably results should be split on modules.

So ran it on clang and clang-tidy just crashed, gonna debug it see whats happening

Crash fixed heres what happened when I ran it on clang/lib

Quite a few occurances of const auto Decl which are renamed as const auto* const, or worse still a few that are redeclared as auto *const

/home/nathan/src/llvm-project/clang/lib/StaticAnalyzer/Checkers/MoveChecker.cpp:623:3: warning: 'const auto IC' can be declared as 'const auto *const IC' [readability-qualified-auto]
  const auto IC = dyn_cast<CXXInstanceCall>(&Call);
  ^~~~~~~~~~~
  const auto *const 
/home/nathan/src/llvm-project/clang/lib/StaticAnalyzer/Checkers/MoveChecker.cpp:636:3: warning: 'const auto MethodDecl' can be declared as 'const auto *const MethodDecl' [readability-qualified-auto]
  const auto MethodDecl = dyn_cast_or_null<CXXMethodDecl>(IC->getDecl());
  ^~~~~~~~~~~
  const auto *const

286 files changed, without looking at header files. One file failed to build which is due to a dependant template. My guess is one call will have returned a naked pointer, and another returned an iterator, maybe I should disregard dependant templates unless it would be possible to deduce its always a pointer

Yes, templates are usually not so nice to handle well :/
One unfortunate thing I am currently looking for is checking if auto->deducedType() is a tempateTypeParmType or substTemplateTypeParmType through matchers, I failed. But in a sense you need to check for that condition for the cases auto -> direct; auto & -> pointee; auto * -> pointee.
From my experience it helps to run it over llvm/lib, too as there is a lot of interesting c++ code in their. My checks broke there more often.

I managed to make some headway with templates, but I think I'll need to add some new ast matchers to get full coverage. However this patch is a little big for more AST matchers as well so that can come at a later date. Right now the behaviour is ignore template instantiations apart from function instantiations where the type is deduced as T*. using that I'm able to run across the whole of llvm and clang lib with no errors in compilation

clang-tools-extra/docs/ReleaseNotes.rst

145

Ahh I see, what threw me is there is an alias in release notes in master that is just randomly thrown in the middle, guess I'll change both of them

clang-tools-extra/docs/clang-tidy/checks/list.rst

406

How come, thought this section was just for alias declarations? or do llvm checks not get listed in the alias section

[Eugene.Zelenko]

clang-tools-extra/docs/ReleaseNotes.rst

145

I think best solution would be to introduce section headers, otherwise people would make similar mistakes during merges in future.

[njames93]

clang-tools-extra/docs/ReleaseNotes.rst

145

Shall I fix the order of everything and add sections then moving forward it should stay consistent?

[Eugene.Zelenko]

clang-tools-extra/docs/ReleaseNotes.rst

145

It'll be very reasonable fix, especially because of upcoming release, but probably it should be done in separate patch.

[njames93]

Those pesky new lines

clang-tools-extra/docs/ReleaseNotes.rst

145

I'll leave this as is then

[njames93]

better template support, still runs on llvm and clang libs just fine

[aaron.ballman]

clang-tools-extra/clang-tidy/readability/QualifiedAutoCheck.cpp

32

We still support restrict in C++ though: https://godbolt.org/z/CT5nw2

Also, one thing that comes up whenever we talk about qualifiers are type attributes like address spaces. I have no idea how those interact with type deduction (simple tests show that it does seem to be deduced: https://godbolt.org/z/LtFAMJ), but I would imagine we would want (at least an option) to be able to ensure those get written out explicitly as well. (If this turns out to be difficult, it could be done in a follow-up patch.)

94

You shouldn't use auto here as the type is not spelled out in the initialization.

95–96

Can you explain why this code is needed? If the pointee is null, there's been some sort of error, I believe.

I am not certain this function is needed, I think QType->getPointeeType().isConstQualified() likely suffices.

102–103

You can skip calling getTypePtr() and just use the -> overload.

104–105

Same observations here as above.

160

auto *

161

Might as well move this closer to where it's actually used.

[Eugene.Zelenko]

clang-tools-extra/clang-tidy/readability/QualifiedAutoCheck.cpp

160

This would be nice test for check.

[njames93]

clang-tools-extra/clang-tidy/readability/QualifiedAutoCheck.cpp

32

I'll add in support for it

95–96

Well it kept crashing without that check so I need to do some more digging on that front

160

I feel like this is what the face-palm emoji is for. I should've pointed that run clang tidy script at clang-tools-extra

[njames93]

Runs on the entire llvm project with no build issues afterwards. also fixed my facepalm

clang-tools-extra/clang-tidy/readability/QualifiedAutoCheck.cpp

95–96

Figured it out, its when the type deduced from auto is a dependant template type, getting the desugarded type is working atm, but I dont know the ast well enough to be sure thats the correct method

160

that wont happen again now :D Idiot proof

[njames93]

clang-tools-extra/clang-tidy/readability/QualifiedAutoCheck.cpp

95–96

I put in an assert check just for sanity reasons. but I have had no issues now I have updated slightly

[njames93]

small nits

[Eugene.Zelenko]

Please rebase from trunk. I sorted Clang-tidy entries in Release Notes and aliases have own subsubsection. Please keep alphabetical order there.

[njames93]

Rebase onto trunk

[aaron.ballman]

LGTM aside from nits

clang-tools-extra/clang-tidy/readability/QualifiedAutoCheck.cpp

84

Can drop the else after return.

100

dyn_cast<> can return null, but you don't check for null here. Should this be using cast<> or should it be checking for null?

170–176

Elide braces. (Same elsewhere in the patch)

clang-tools-extra/docs/clang-tidy/checks/readability-qualified-auto.rst

7

Extra whitespace between variables and that.
auto typed -> auto-typed (with backticks and hyphen)

10

a auto typed -> an auto-typed

[njames93]

reformat/lint

[njames93]

• Update docs to use backticks

clang-tools-extra/clang-tidy/readability/QualifiedAutoCheck.cpp

100

It should be using cast, we already know that its an `AutoType` because the matcher matched on autoType

[aaron.ballman]

Do you need someone to commit this on your behalf? (If you plan to continue contributing, which I hope you do, it may be a good time for you to obtain commit privileges: https://llvm.org/docs/DeveloperPolicy.html#obtaining-commit-access but I am happy to commit on your behalf if you'd prefer.)

[njames93]

In D72217#1819437, @aaron.ballman wrote:

Do you need someone to commit this on your behalf? (If you plan to continue contributing, which I hope you do, it may be a good time for you to obtain commit privileges: https://llvm.org/docs/DeveloperPolicy.html#obtaining-commit-access but I am happy to commit on your behalf if you'd prefer.)

For now I would appreciate it, I have fired off an email though, cheers.

[aaron.ballman]

In D72217#1819678, @njames93 wrote:

In D72217#1819437, @aaron.ballman wrote:

Do you need someone to commit this on your behalf? (If you plan to continue contributing, which I hope you do, it may be a good time for you to obtain commit privileges: https://llvm.org/docs/DeveloperPolicy.html#obtaining-commit-access but I am happy to commit on your behalf if you'd prefer.)

For now I would appreciate it, I have fired off an email though, cheers.

Happy to do so -- can you upload a rebased patch? I am getting merge conflicts:

c:\llvm-project>git apply C:\Users\aballman\Desktop\D72217.diff
C:/Users/aballman/Desktop/D72217.diff:402: trailing whitespace.
  Adds pointer and ``const`` qualifications to ``auto``-typed variables
C:/Users/aballman/Desktop/D72217.diff:468: trailing whitespace.
make it obvious if a ``auto`` typed variable is a pointer, constant pointer or
C:/Users/aballman/Desktop/D72217.diff:469: trailing whitespace.
constant reference. This check will transform ``auto`` to ``auto *`` when the
error: patch failed: clang-tools-extra/docs/clang-tidy/checks/list.rst:397
error: clang-tools-extra/docs/clang-tidy/checks/list.rst: patch does not apply

[njames93]

• Rebase from trunk

[aaron.ballman]

Thank you for the new check, I've commit on your behalf in 36fcbb838c8f293f46bfed78c6ed8c177f1e3485

[merge_guards_bot]

Unit tests: pass. 61851 tests passed, 0 failed and 781 were skipped.

clang-tidy: unknown.

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-format.patch, CMakeCache.txt, console-log.txt, test-results.xml

[sammccall]

This check as configured for LLVM itself is pretty noisy, generating warnings like:

warning: 'auto *CTSD' can be declared as 'const auto *CTSD' [llvm-qualified-auto]

which the LLVM dev guide doesn't have an opinion about.

AFAICS there's no option to disable just the const behavior, and no proposal to change the dev guidelines - is someone working on something here?
Otherwise I'd like to turn this off at least for clang-tools-extra.

[njames93]

In D72217#1844112, @sammccall wrote:

This check as configured for LLVM itself is pretty noisy, generating warnings like:

warning: 'auto *CTSD' can be declared as 'const auto *CTSD' [llvm-qualified-auto]

which the LLVM dev guide doesn't have an opinion about.

AFAICS there's no option to disable just the const behavior, and no proposal to change the dev guidelines - is someone working on something here?
Otherwise I'd like to turn this off at least for clang-tools-extra.

https://llvm.org/docs/CodingStandards.html#beware-unnecessary-copies-with-auto

// Copy pointers, but make it clear that they're pointers.
for (const auto *Ptr : Container) { observe(*Ptr); }
for (auto *Ptr : Container) { Ptr->change(); }

This is the reasoning behind putting the const qualifier in the check. If people feel that this isn't quite enough to justify the const qualifier I'll submit a follow up. The general consensus though is that you should mark auto pointers as const if they don't need to change to express intent

[sammccall]

In D72217#1844204, @njames93 wrote:

https://llvm.org/docs/CodingStandards.html#beware-unnecessary-copies-with-auto

// Copy pointers, but make it clear that they're pointers.
for (const auto *Ptr : Container) { observe(*Ptr); }
for (auto *Ptr : Container) { Ptr->change(); }

This is the reasoning behind putting the const qualifier in the check. If people feel that this isn't quite enough to justify the const qualifier I'll submit a follow up. The general consensus though is that you should mark auto pointers as const if they don't need to change to express intent

The text/rule there is explicitly about avoiding/clarifying copies - the examples indeed use 'const' but AFAICT the "don't copy" reasoning only applies to including *&.

FWIW I think const here is often noise, particularly in AST-walking code where you're traversing an edge from an X* to a Y* - the latter will be const if the former is, and I care at API boundaries but not in between. (It's usually a meaningless distinction - e.g. we're just reading but it's a non-const pointer because RecursiveASTVisitor isn't const-friendly).

So while spelling const is often helpful, we shouldn't (and don't) require it, and the current config of this check is too intrusive.

[njames93]

In D72217#1844262, @sammccall wrote:

The text/rule there is explicitly about avoiding/clarifying copies - the examples indeed use 'const' but AFAICT the "don't copy" reasoning only applies to including *&.

FWIW I think const here is often noise, particularly in AST-walking code where you're traversing an edge from an X* to a Y* - the latter will be const if the former is, and I care at API boundaries but not in between. (It's usually a meaningless distinction - e.g. we're just reading but it's a non-const pointer because RecursiveASTVisitor isn't const-friendly).

So while spelling const is often helpful, we shouldn't (and don't) require it, and the current config of this check is too intrusive.

I have always been a little unsure, a few of the patches I have submitted reviewers have said to add const to auto * or turn auto into const auto * which gave the impression its a guideline. Could add an option to the check to forgo the const qualifier checks though.

[aaron.ballman]

In D72217#1844262, @sammccall wrote:

In D72217#1844204, @njames93 wrote:

https://llvm.org/docs/CodingStandards.html#beware-unnecessary-copies-with-auto

// Copy pointers, but make it clear that they're pointers.
for (const auto *Ptr : Container) { observe(*Ptr); }
for (auto *Ptr : Container) { Ptr->change(); }

This is the reasoning behind putting the const qualifier in the check. If people feel that this isn't quite enough to justify the const qualifier I'll submit a follow up. The general consensus though is that you should mark auto pointers as const if they don't need to change to express intent

The text/rule there is explicitly about avoiding/clarifying copies - the examples indeed use 'const' but AFAICT the "don't copy" reasoning only applies to including *&.

FWIW I think const here is often noise, particularly in AST-walking code where you're traversing an edge from an X* to a Y* - the latter will be const if the former is, and I care at API boundaries but not in between. (It's usually a meaningless distinction - e.g. we're just reading but it's a non-const pointer because RecursiveASTVisitor isn't const-friendly).

So while spelling const is often helpful, we shouldn't (and don't) require it, and the current config of this check is too intrusive.

Oddly enough, there were several long-time reviewers (including myself) who firmly believed we did document this requirement at one point in time, but we couldn't find evidence of it when we started digging harder. We've been giving this review feedback consistently for many years, so "we don't require it" may not be entirely accurate. We may not have written down a requirement for it, but I've *definitely* heard that we should not be deducing qualifiers and I agree with that sentiment. It makes the code harder to understand as a reviewer because there's no way to tell whether code is mutating the pointer/reference or not without further digging (and it changes where code dispatches to, so it's important information for readers of the code to have).

I'm fine making this into an option, but my preference be that the option is on by default for the readability checker. For the LLVM checker, maybe we default the option to off and argue about changing the coding standard.

[njames93]

https://reviews.llvm.org/D73548
Next step is to fire off an email to llvm-dev about adding it to the coding standard :)

clang-tools-extra/clang-tidy/llvm/LLVMTidyModule.cpp

 //===--- LLVMTidyModule.cpp - clang-tidy ----------------------------------===//
 //
 // Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
 // See https://llvm.org/LICENSE.txt for license information.
 // SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
 //
 //===----------------------------------------------------------------------===//
 
 #include "../ClangTidy.h"
 #include "../ClangTidyModule.h"
 #include "../ClangTidyModuleRegistry.h"
 #include "../readability/NamespaceCommentCheck.h"
+#include "../readability/QualifiedAutoCheck.h"
 #include "HeaderGuardCheck.h"
 #include "IncludeOrderCheck.h"
 #include "PreferIsaOrDynCastInConditionalsCheck.h"
 #include "PreferRegisterOverUnsignedCheck.h"
 #include "TwineLocalCheck.h"
 
 namespace clang {
 namespace tidy {
 namespace llvm_check {
 
 class LLVMModule : public ClangTidyModule {
 public:
   void addCheckFactories(ClangTidyCheckFactories &CheckFactories) override {
     CheckFactories.registerCheck<LLVMHeaderGuardCheck>("llvm-header-guard");
     CheckFactories.registerCheck<IncludeOrderCheck>("llvm-include-order");
     CheckFactories.registerCheck<readability::NamespaceCommentCheck>(
         "llvm-namespace-comment");
     CheckFactories.registerCheck<PreferIsaOrDynCastInConditionalsCheck>(
         "llvm-prefer-isa-or-dyn-cast-in-conditionals");
     CheckFactories.registerCheck<PreferRegisterOverUnsignedCheck>(
         "llvm-prefer-register-over-unsigned");
+    CheckFactories.registerCheck<readability::QualifiedAutoCheck>(
+        "llvm-qualified-auto");
     CheckFactories.registerCheck<TwineLocalCheck>("llvm-twine-local");
   }
 };
 
 // Register the LLVMTidyModule using this statically initialized variable.
 static ClangTidyModuleRegistry::Add<LLVMModule> X("llvm-module",
                                                   "Adds LLVM lint checks.");
 
 } // namespace llvm_check
 
 // This anchor is used to force the linker to link in the generated object file
 // and thus register the LLVMModule.
 volatile int LLVMModuleAnchorSource = 0;
 
 } // namespace tidy
 } // namespace clang

clang-tools-extra/clang-tidy/readability/CMakeLists.txt

add_clang_library(clangTidyReadabilityModule
   IsolateDeclarationCheck.cpp
   MagicNumbersCheck.cpp
   MakeMemberFunctionConstCheck.cpp
   MisleadingIndentationCheck.cpp
   MisplacedArrayIndexCheck.cpp
   NamedParameterCheck.cpp
   NamespaceCommentCheck.cpp
   NonConstParameterCheck.cpp
+  QualifiedAutoCheck.cpp
   ReadabilityTidyModule.cpp
   RedundantAccessSpecifiersCheck.cpp
   RedundantControlFlowCheck.cpp
   RedundantDeclarationCheck.cpp
   RedundantFunctionPtrDereferenceCheck.cpp
   RedundantMemberInitCheck.cpp
   RedundantPreprocessorCheck.cpp
   RedundantSmartptrGetCheck.cpp

clang-tools-extra/clang-tidy/readability/QualifiedAutoCheck.h

+//===--- QualifiedAutoCheck.h - clang-tidy ----------------------*- C++ -*-===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===----------------------------------------------------------------------===//
+
+#ifndef LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_READABILITY_QUALIFIEDAUTOCHECK_H
+#define LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_READABILITY_QUALIFIEDAUTOCHECK_H
+
+#include "../ClangTidyCheck.h"
+
+namespace clang {
+namespace tidy {
+namespace readability {
+
+/// Finds variables declared as auto that could be declared as:
+/// 'auto*' or 'const auto *' and reference variables declared as:
+/// 'const auto &'.
+///
+/// For the user-facing documentation see:
+/// http://clang.llvm.org/extra/clang-tidy/checks/readability-qualified-auto.html
+class QualifiedAutoCheck : public ClangTidyCheck {
+public:
+  QualifiedAutoCheck(StringRef Name, ClangTidyContext *Context)
+      : ClangTidyCheck(Name, Context) {}
+  void registerMatchers(ast_matchers::MatchFinder *Finder) override;
+  void check(const ast_matchers::MatchFinder::MatchResult &Result) override;
+};
+
+} // namespace readability
+} // namespace tidy
+} // namespace clang
+
+#endif // LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_READABILITY_QUALIFIEDAUTOCHECK_H

clang-tools-extra/clang-tidy/readability/QualifiedAutoCheck.cpp

+//===--- QualifiedAutoCheck.cpp - clang-tidy ------------------------------===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===----------------------------------------------------------------------===//
+
+#include "QualifiedAutoCheck.h"
+#include "../utils/LexerUtils.h"
+#include "clang/ASTMatchers/ASTMatchers.h"
+#include "llvm/ADT/Optional.h"
+#include "llvm/ADT/SmallVector.h"
+
+using namespace clang::ast_matchers;
+
+namespace clang {
+namespace tidy {
+namespace readability {
+
+namespace {
+
+// FIXME move to ASTMatchers
+AST_MATCHER_P(QualType, hasUnqualifiedType,
+              ast_matchers::internal::Matcher<QualType>, InnerMatcher) {
+  return InnerMatcher.matches(Node.getUnqualifiedType(), Finder, Builder);
+}
+
+enum class Qualifier { Const, Volatile, Restrict };
+
+llvm::Optional<Token> findQualToken(const VarDecl *Decl, Qualifier Qual,
+                                    const MatchFinder::MatchResult &Result) {
+  // Since either of the locs can be in a macro, use `makeFileCharRange` to be
+  // sure that we have a consistent `CharSourceRange`, located entirely in the
+  // source file.
+
+  assert(Qual == Qualifier::Const || Qual == Qualifier::Volatile ||
+         Qual == Qualifier::Restrict && "Invalid Qualifier");
+
+  SourceLocation BeginLoc = Decl->getQualifierLoc().getBeginLoc();
+  if (BeginLoc.isInvalid())
+    BeginLoc = Decl->getBeginLoc();
+  SourceLocation EndLoc = Decl->getLocation();
+
+  CharSourceRange FileRange = Lexer::makeFileCharRange(
+      CharSourceRange::getCharRange(BeginLoc, EndLoc), *Result.SourceManager,
+      Result.Context->getLangOpts());
+
+  if (FileRange.isInvalid())
+    return llvm::None;
+
+  tok::TokenKind Tok =
+      Qual == Qualifier::Const
+          ? tok::kw_const
+          : Qual == Qualifier::Volatile ? tok::kw_volatile : tok::kw_restrict;
+
+  return utils::lexer::getQualifyingToken(Tok, FileRange, *Result.Context,
+                                          *Result.SourceManager);
+}
+
+llvm::Optional<SourceRange>
+getTypeSpecifierLocation(const VarDecl *Var,
+                         const MatchFinder::MatchResult &Result) {
+  SourceRange TypeSpecifier(
+      Var->getTypeSpecStartLoc(),
+      Var->getTypeSpecEndLoc().getLocWithOffset(Lexer::MeasureTokenLength(
+          Var->getTypeSpecEndLoc(), *Result.SourceManager,
+          Result.Context->getLangOpts())));
+
+  if (TypeSpecifier.getBegin().isMacroID() ||
+      TypeSpecifier.getEnd().isMacroID())
+    return llvm::None;
+  return TypeSpecifier;
+}
+
+llvm::Optional<SourceRange> mergeReplacementRange(SourceRange &TypeSpecifier,
+                                                  const Token &ConstToken) {
+  if (TypeSpecifier.getBegin().getLocWithOffset(-1) == ConstToken.getEndLoc()) {
+    TypeSpecifier.setBegin(ConstToken.getLocation());
+    return llvm::None;
+  }
+  if (TypeSpecifier.getEnd().getLocWithOffset(1) == ConstToken.getLocation()) {
+    TypeSpecifier.setEnd(ConstToken.getEndLoc());
+    return llvm::None;
+  }
+  return SourceRange(ConstToken.getLocation(), ConstToken.getEndLoc());
+}
+
+bool isPointerConst(QualType QType) {
+  QualType Pointee = QType->getPointeeType();
+  assert(!Pointee.isNull() && "can't have a null Pointee");
+  return Pointee.isConstQualified();
+}
+
+bool isAutoPointerConst(QualType QType) {
+  QualType Pointee =
+      cast<AutoType>(QType->getPointeeType().getTypePtr())->desugar();
+  assert(!Pointee.isNull() && "can't have a null Pointee");
+  return Pointee.isConstQualified();
+}
+
+} // namespace
+
+void QualifiedAutoCheck::registerMatchers(MatchFinder *Finder) {
+  if (!getLangOpts().CPlusPlus11)
+    return; // Auto deduction not used in 'C or C++03 and earlier', so don't
+            // register Matchers.
+  auto ExplicitSingleVarDecl =
+      [](const ast_matchers::internal::Matcher<VarDecl> &InnerMatcher,
+         llvm::StringRef ID) {
+        return declStmt(
+            unless(isInTemplateInstantiation()),
+            hasSingleDecl(
+                varDecl(unless(isImplicit()), InnerMatcher).bind(ID)));
+      };
+  auto ExplicitSingleVarDeclInTemplate =
+      [](const ast_matchers::internal::Matcher<VarDecl> &InnerMatcher,
+         llvm::StringRef ID) {
+        return declStmt(
+            isInTemplateInstantiation(),
+            hasSingleDecl(
+                varDecl(unless(isImplicit()), InnerMatcher).bind(ID)));
+      };
+
+  auto IsBoundToType = refersToType(equalsBoundNode("type"));
+
+  Finder->addMatcher(
+      ExplicitSingleVarDecl(hasType(autoType(hasDeducedType(
+                                pointerType(pointee(unless(functionType())))))),
+                            "auto"),
+      this);
+
+  Finder->addMatcher(
+      ExplicitSingleVarDeclInTemplate(
+          allOf(hasType(autoType(hasDeducedType(pointerType(
+                    pointee(hasUnqualifiedType(qualType().bind("type")),
+                            unless(functionType())))))),
+                anyOf(hasAncestor(
+                          functionDecl(hasAnyTemplateArgument(IsBoundToType))),
+                      hasAncestor(classTemplateSpecializationDecl(
+                          hasAnyTemplateArgument(IsBoundToType))))),
+          "auto"),
+      this);
+  Finder->addMatcher(ExplicitSingleVarDecl(
+                         hasType(pointerType(pointee(autoType()))), "auto_ptr"),
+                     this);
+  Finder->addMatcher(
+      ExplicitSingleVarDecl(hasType(lValueReferenceType(pointee(autoType()))),
+                            "auto_ref"),
+      this);
+}
+
+void QualifiedAutoCheck::check(const MatchFinder::MatchResult &Result) {
+  if (const auto *Var = Result.Nodes.getNodeAs<VarDecl>("auto")) {
+    SourceRange TypeSpecifier;
+    if (llvm::Optional<SourceRange> TypeSpec =
+            getTypeSpecifierLocation(Var, Result)) {
+      TypeSpecifier = *TypeSpec;
+    } else
+      return;
+
+    llvm::SmallVector<SourceRange, 4> RemoveQualifiersRange;
+    auto CheckQualifier = [&](bool IsPresent, Qualifier Qual) {
+      if (IsPresent) {
+        llvm::Optional<Token> Token = findQualToken(Var, Qual, Result);
+        if (!Token || Token->getLocation().isMacroID())
+          return true; // Disregard this VarDecl.
+        if (llvm::Optional<SourceRange> Result =
+                mergeReplacementRange(TypeSpecifier, *Token))
+          RemoveQualifiersRange.push_back(*Result);
+      }
+      return false;
+    };
+
+    bool IsLocalConst = Var->getType().isLocalConstQualified();
+    bool IsLocalVolatile = Var->getType().isLocalVolatileQualified();
+    bool IsLocalRestrict = Var->getType().isLocalRestrictQualified();
+
+    if (CheckQualifier(IsLocalConst, Qualifier::Const))
+      return;
+    if (CheckQualifier(IsLocalVolatile, Qualifier::Volatile))
+      return;
+    if (CheckQualifier(IsLocalRestrict, Qualifier::Restrict))
+      return;
+
+    // Check for bridging the gap between the asterisk and name.
+    if (Var->getLocation() == TypeSpecifier.getEnd().getLocWithOffset(1))
+      TypeSpecifier.setEnd(TypeSpecifier.getEnd().getLocWithOffset(1));
+
+    CharSourceRange FixItRange = CharSourceRange::getCharRange(TypeSpecifier);
+    if (FixItRange.isInvalid())
+      return;
+
+    SourceLocation FixitLoc = FixItRange.getBegin();
+    for (SourceRange &Range : RemoveQualifiersRange) {
+      if (Range.getBegin() < FixitLoc)
+        FixitLoc = Range.getBegin();
+    }
+
+    std::string ReplStr = [&] {
+      llvm::StringRef PtrConst = isPointerConst(Var->getType()) ? "const " : "";
+      llvm::StringRef LocalConst = IsLocalConst ? "const " : "";
+      llvm::StringRef LocalVol = IsLocalVolatile ? "volatile " : "";
+      llvm::StringRef LocalRestrict = IsLocalRestrict ? "__restrict " : "";
+      return (PtrConst + "auto *" + LocalConst + LocalVol + LocalRestrict)
+          .str();
+    }();
+
+    DiagnosticBuilder Diag =
+        diag(FixitLoc, "'%0%1%2auto %3' can be declared as '%4%3'")
+        << (IsLocalConst ? "const " : "")
+        << (IsLocalVolatile ? "volatile " : "")
+        << (IsLocalRestrict ? "__restrict " : "") << Var->getName() << ReplStr;
+
+    for (SourceRange &Range : RemoveQualifiersRange) {
+      Diag << FixItHint::CreateRemoval(
+          CharSourceRange::getCharRange(Range.getBegin(), Range.getEnd()));
+    }
+
+    Diag << FixItHint::CreateReplacement(FixItRange, ReplStr);
+    return;
+  }
+  if (const auto *Var = Result.Nodes.getNodeAs<VarDecl>("auto_ptr")) {
+    if (!isPointerConst(Var->getType()))
+      return; // Pointer isn't const, no need to add const qualifier.
+    if (!isAutoPointerConst(Var->getType()))
+      return; // Const isnt wrapped in the auto type, so must be declared
+              // explicitly.
+
+    if (Var->getType().isLocalConstQualified()) {
+      llvm::Optional<Token> Token =
+          findQualToken(Var, Qualifier::Const, Result);
+      if (!Token || Token->getLocation().isMacroID())
+        return;
+    }
+    if (Var->getType().isLocalVolatileQualified()) {
+      llvm::Optional<Token> Token =
+          findQualToken(Var, Qualifier::Volatile, Result);
+      if (!Token || Token->getLocation().isMacroID())
+        return;
+    }
+    if (Var->getType().isLocalRestrictQualified()) {
+      llvm::Optional<Token> Token =
+          findQualToken(Var, Qualifier::Restrict, Result);
+      if (!Token || Token->getLocation().isMacroID())
+        return;
+    }
+
+    CharSourceRange FixItRange;
+    if (llvm::Optional<SourceRange> TypeSpec =
+            getTypeSpecifierLocation(Var, Result)) {
+      FixItRange = CharSourceRange::getCharRange(*TypeSpec);
+      if (FixItRange.isInvalid())
+        return;
+    } else
+      return;
+
+    DiagnosticBuilder Diag =
+        diag(FixItRange.getBegin(),
+             "'auto *%0%1%2' can be declared as 'const auto *%0%1%2'")
+        << (Var->getType().isLocalConstQualified() ? "const " : "")
+        << (Var->getType().isLocalVolatileQualified() ? "volatile " : "")
+        << Var->getName();
+    Diag << FixItHint::CreateReplacement(FixItRange, "const auto *");
+    return;
+  }
+  if (const auto *Var = Result.Nodes.getNodeAs<VarDecl>("auto_ref")) {
+    if (!isPointerConst(Var->getType()))
+      return; // Pointer isn't const, no need to add const qualifier.
+    if (!isAutoPointerConst(Var->getType()))
+      // Const isnt wrapped in the auto type, so must be declared explicitly.
+      return;
+
+    CharSourceRange FixItRange;
+    if (llvm::Optional<SourceRange> TypeSpec =
+            getTypeSpecifierLocation(Var, Result)) {
+      FixItRange = CharSourceRange::getCharRange(*TypeSpec);
+      if (FixItRange.isInvalid())
+        return;
+    } else
+      return;
+
+    DiagnosticBuilder Diag =
+        diag(FixItRange.getBegin(),
+             "'auto &%0' can be declared as 'const auto &%0'")
+        << Var->getName();
+    Diag << FixItHint::CreateReplacement(FixItRange, "const auto &");
+    return;
+  }
+}
+
+} // namespace readability
+} // namespace tidy
+} // namespace clang

clang-tools-extra/clang-tidy/readability/ReadabilityTidyModule.cpp

 #include "InconsistentDeclarationParameterNameCheck.h"
 #include "IsolateDeclarationCheck.h"
 #include "MagicNumbersCheck.h"
 #include "MakeMemberFunctionConstCheck.h"
 #include "MisleadingIndentationCheck.h"
 #include "MisplacedArrayIndexCheck.h"
 #include "NamedParameterCheck.h"
 #include "NonConstParameterCheck.h"
+#include "QualifiedAutoCheck.h"
 #include "RedundantAccessSpecifiersCheck.h"
 #include "RedundantControlFlowCheck.h"
 #include "RedundantDeclarationCheck.h"
 #include "RedundantFunctionPtrDereferenceCheck.h"
 #include "RedundantMemberInitCheck.h"
 #include "RedundantPreprocessorCheck.h"
 #include "RedundantSmartptrGetCheck.h"
 #include "RedundantStringCStrCheck.h"
void addCheckFactories(ClangTidyCheckFactories &CheckFactories) override {
     CheckFactories.registerCheck<MagicNumbersCheck>(
         "readability-magic-numbers");
     CheckFactories.registerCheck<MakeMemberFunctionConstCheck>(
         "readability-make-member-function-const");
     CheckFactories.registerCheck<MisleadingIndentationCheck>(
         "readability-misleading-indentation");
     CheckFactories.registerCheck<MisplacedArrayIndexCheck>(
         "readability-misplaced-array-index");
+    CheckFactories.registerCheck<QualifiedAutoCheck>(
+        "readability-qualified-auto");
     CheckFactories.registerCheck<RedundantAccessSpecifiersCheck>(
         "readability-redundant-access-specifiers");
     CheckFactories.registerCheck<RedundantFunctionPtrDereferenceCheck>(
         "readability-redundant-function-ptr-dereference");
     CheckFactories.registerCheck<RedundantMemberInitCheck>(
         "readability-redundant-member-init");
     CheckFactories.registerCheck<RedundantPreprocessorCheck>(
         "readability-redundant-preprocessor");

clang-tools-extra/docs/ReleaseNotes.rst

- New :doc:`linuxkernel-must-use-errs
   ``linux/err.h``.
 
 - New :doc:`llvm-prefer-register-over-unsigned
   <clang-tidy/checks/llvm-prefer-register-over-unsigned>` check.
 
   Finds historical use of ``unsigned`` to hold vregs and physregs and rewrites
   them to use ``Register``
 
 - New :doc:`objc-missing-hash
   <clang-tidy/checks/objc-missing-hash>` check.
 
   Finds Objective-C implementations that implement ``-isEqual:`` without also
   appropriately implementing ``-hash``.
 
 - New :doc:`performance-no-automatic-move
   <clang-tidy/checks/performance-no-automatic-move>` check.
 
   Finds local variables that cannot be automatically moved due to constness.
 
 - New :doc:`performance-trivially-destructible
   <clang-tidy/checks/performance-trivially-destructible>` check.
 
   Finds types that could be made trivially-destructible by removing out-of-line
   defaulted destructor declarations.
 
 - New :doc:`readability-make-member-function-const
   <clang-tidy/checks/readability-make-member-function-const>` check.
 
   Finds non-static member functions that can be made ``const``
   because the functions don't use ``this`` in a non-const way.
 
+- New :doc:`readability-qualified-auto
+  <clang-tidy/checks/readability-qualified-auto>` check.
+
+  Adds pointer and ``const`` qualifications to ``auto``-typed variables 
+  that are deduced to pointers and ``const`` pointers.
+
 - New :doc:`readability-redundant-access-specifiers
   <clang-tidy/checks/readability-redundant-access-specifiers>` check.
 
   Finds classes, structs, and unions that contain redundant member
   access specifiers.
 
 New aliases
 ^^^^^^^^^^^
 
 - New alias :doc:`cert-pos44-c
   <clang-tidy/checks/cert-pos44-c>` to
   :doc:`bugprone-bad-signal-to-kill-thread
   <clang-tidy/checks/bugprone-bad-signal-to-kill-thread>` was added.
 
+- New alias :doc:`llvm-qualified-auto
+  <clang-tidy/checks/llvm-qualified-auto>` to
+  :doc:`readability-qualified-auto
+  <clang-tidy/checks/readability-qualified-auto>` was added.
+
 Changes in existing checks
 ^^^^^^^^^^^^^^^^^^^^^^^^^^
 
 - Improved :doc:`bugprone-posix-return
   <clang-tidy/checks/bugprone-posix-return>` check.
 
   Now also checks if any calls to ``pthread_*`` functions expect negative
   return values.

clang-tools-extra/docs/clang-tidy/checks/list.rst

.. csv-table::
    `readability-inconsistent-declaration-parameter-name <readability-inconsistent-declaration-parameter-name.html>`_, "Yes"
    `readability-isolate-declaration <readability-isolate-declaration.html>`_, "Yes"
    `readability-magic-numbers <readability-magic-numbers.html>`_,
    `readability-make-member-function-const <readability-make-member-function-const.html>`_, "Yes"
    `readability-misleading-indentation <readability-misleading-indentation.html>`_,
    `readability-misplaced-array-index <readability-misplaced-array-index.html>`_, "Yes"
    `readability-named-parameter <readability-named-parameter.html>`_, "Yes"
    `readability-non-const-parameter <readability-non-const-parameter.html>`_, "Yes"
+   `readability-qualified-auto <readability-qualified-auto.html>`_, "Yes"
    `readability-redundant-access-specifiers <readability-redundant-access-specifiers.html>`_, "Yes"
    `readability-redundant-control-flow <readability-redundant-control-flow.html>`_, "Yes"
    `readability-redundant-declaration <readability-redundant-declaration.html>`_, "Yes"
    `readability-redundant-function-ptr-dereference <readability-redundant-function-ptr-dereference.html>`_, "Yes"
    `readability-redundant-member-init <readability-redundant-member-init.html>`_, "Yes"
    `readability-redundant-preprocessor <readability-redundant-preprocessor.html>`_,
    `readability-redundant-smartptr-get <readability-redundant-smartptr-get.html>`_, "Yes"
    `readability-redundant-string-cstr <readability-redundant-string-cstr.html>`_,
.. csv-table:: Aliases..
    `hicpp-use-auto <hicpp-use-auto.html>`_, `modernize-use-auto <modernize-use-auto.html>`_, "Yes"
    `hicpp-use-emplace <hicpp-use-emplace.html>`_, `modernize-use-emplace <modernize-use-emplace.html>`_, "Yes"
    `hicpp-use-equals-default <hicpp-use-equals-default.html>`_, `modernize-use-equals-default <modernize-use-equals-default.html>`_, "Yes"
    `hicpp-use-equals-delete <hicpp-use-equals-delete.html>`_, `modernize-use-equals-delete <modernize-use-equals-delete.html>`_, "Yes"
    `hicpp-use-noexcept <hicpp-use-noexcept.html>`_, `modernize-use-noexcept <modernize-use-noexcept.html>`_, "Yes"
    `hicpp-use-nullptr <hicpp-use-nullptr.html>`_, `modernize-use-nullptr <modernize-use-nullptr.html>`_, "Yes"
    `hicpp-use-override <hicpp-use-override.html>`_, `modernize-use-override <modernize-use-override.html>`_, "Yes"
    `hicpp-vararg <hicpp-vararg.html>`_, `cppcoreguidelines-pro-type-vararg <cppcoreguidelines-pro-type-vararg.html>`_,
+   `llvm-qualified-auto <llvm-qualified-auto.html>`_, `readability-qualified-auto <readability-qualified-auto.html>`_, "Yes"

clang-tools-extra/docs/clang-tidy/checks/llvm-qualified-auto.rst

+.. title:: clang-tidy - llvm-qualified-auto
+.. meta::
+   :http-equiv=refresh: 5;URL=readability-qualified-auto.html
+
+llvm-qualified-auto
+===================
+
+The llvm-qualified-auto check is an alias, please see
+`readability-qualified-auto <readability-qualified-auto.html>`_
+for more information.

clang-tools-extra/docs/clang-tidy/checks/readability-qualified-auto.rst

+.. title:: clang-tidy - readability-qualified-auto
+
+readability-qualified-auto
+==========================
+
+Adds pointer and ``const`` qualifications to ``auto``-typed variables that are deduced
+to pointers and ``const`` pointers.
+
+`LLVM Coding Standards <https://llvm.org/docs/CodingStandards.html>`_ 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
+
+.. code-block:: c++
+
+  for (auto &Data : MutatableContainer) {
+    change(Data);
+  }
+  for (auto &Data : ConstantContainer) {
+    observe(Data);
+  }
+  for (auto Data : MutatablePtrContainer) {
+    change(*Data);
+  }
+  for (auto Data : ConstantPtrContainer) {
+    observe(*Data);
+  }
+
+Would be transformed into:
+
+.. code-block:: c++
+
+  for (auto &Data : MutatableContainer) {
+    change(Data);
+  }
+  for (const auto &Data : 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<int *>(Baz1);
+  const auto Bar = cast<const int *>(Baz2);
+  volatile auto FooBar = cast<int*>(Baz3);
+
+Would be transformed into:
+
+.. code-block:: c++
+
+  auto *const Foo = cast<int *>(Baz1);
+  const auto *const Bar = cast<const int *>(Baz2);
+  auto *volatile FooBar = cast<int*>(Baz3);
+
+This check helps to enforce this `LLVM Coding Standards recommendation
+<https://llvm.org/docs/CodingStandards.html#beware-unnecessary-copies-with-auto>`_.

clang-tools-extra/test/clang-tidy/checkers/readability-qualified-auto-cxx20.cpp

+// RUN: %check_clang_tidy %s readability-qualified-auto %t -- -- -std=c++20
+namespace std {
+template <typename T>
+class vector { // dummy impl
+  T _data[1];
+
+public:
+  T *begin() { return _data; }
+  const T *begin() const { return _data; }
+  T *end() { return &_data[1]; }
+  const T *end() const { return &_data[1]; }
+  unsigned size() const { return 0; }
+};
+} // namespace std
+
+std::vector<int> *getVec();
+const std::vector<int> *getCVec();
+void foo() {
+  if (auto X = getVec(); X->size() > 0) {
+    // CHECK-MESSAGES: :[[@LINE-1]]:7: warning: 'auto X' can be declared as 'auto *X'
+    // CHECK-FIXES: {{^}}  if (auto *X = getVec(); X->size() > 0) {
+  }
+  switch (auto X = getVec(); X->size()) {
+    // CHECK-MESSAGES: :[[@LINE-1]]:11: warning: 'auto X' can be declared as 'auto *X'
+    // CHECK-FIXES: {{^}}  switch (auto *X = getVec(); X->size()) {
+  default:
+    break;
+  }
+  for (auto X = getVec(); auto Xi : *X) {
+    // CHECK-MESSAGES: :[[@LINE-1]]:8: warning: 'auto X' can be declared as 'auto *X'
+    // CHECK-FIXES: {{^}}  for (auto *X = getVec(); auto Xi : *X) {
+  }
+}
+void bar() {
+  if (auto X = getCVec(); X->size() > 0) {
+    // CHECK-MESSAGES: :[[@LINE-1]]:7: warning: 'auto X' can be declared as 'const auto *X'
+    // CHECK-FIXES: {{^}}  if (const auto *X = getCVec(); X->size() > 0) {
+  }
+  switch (auto X = getCVec(); X->size()) {
+    // CHECK-MESSAGES: :[[@LINE-1]]:11: warning: 'auto X' can be declared as 'const auto *X'
+    // CHECK-FIXES: {{^}}  switch (const auto *X = getCVec(); X->size()) {
+  default:
+    break;
+  }
+  for (auto X = getCVec(); auto Xi : *X) {
+    // CHECK-MESSAGES: :[[@LINE-1]]:8: warning: 'auto X' can be declared as 'const auto *X'
+    // CHECK-FIXES: {{^}}  for (const auto *X = getCVec(); auto Xi : *X) {
+  }
+}

clang-tools-extra/test/clang-tidy/checkers/readability-qualified-auto.cpp

+// RUN: %check_clang_tidy %s readability-qualified-auto %t
+
+namespace typedefs {
+typedef int *MyPtr;
+typedef int &MyRef;
+typedef const int *CMyPtr;
+typedef const int &CMyRef;
+
+MyPtr getPtr();
+MyRef getRef();
+CMyPtr getCPtr();
+CMyRef getCRef();
+
+void foo() {
+  auto TdNakedPtr = getPtr();
+  // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: 'auto TdNakedPtr' can be declared as 'auto *TdNakedPtr'
+  // CHECK-FIXES: {{^}}  auto *TdNakedPtr = getPtr();
+  auto &TdNakedRef = getRef();
+  auto TdNakedRefDeref = getRef();
+  auto TdNakedCPtr = getCPtr();
+  // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: 'auto TdNakedCPtr' can be declared as 'const auto *TdNakedCPtr'
+  // CHECK-FIXES: {{^}}  const auto *TdNakedCPtr = getCPtr();
+  auto &TdNakedCRef = getCRef();
+  // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: 'auto &TdNakedCRef' can be declared as 'const auto &TdNakedCRef'
+  // CHECK-FIXES: {{^}}  const auto &TdNakedCRef = getCRef();
+  auto TdNakedCRefDeref = getCRef();
+}
+
+}; // namespace typedefs
+
+namespace usings {
+using MyPtr = int *;
+using MyRef = int &;
+using CMyPtr = const int *;
+using CMyRef = const int &;
+
+MyPtr getPtr();
+MyRef getRef();
+CMyPtr getCPtr();
+CMyRef getCRef();
+
+void foo() {
+  auto UNakedPtr = getPtr();
+  // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: 'auto UNakedPtr' can be declared as 'auto *UNakedPtr'
+  // CHECK-FIXES: {{^}}  auto *UNakedPtr = getPtr();
+  auto &UNakedRef = getRef();
+  auto UNakedRefDeref = getRef();
+  auto UNakedCPtr = getCPtr();
+  // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: 'auto UNakedCPtr' can be declared as 'const auto *UNakedCPtr'
+  // CHECK-FIXES: {{^}}  const auto *UNakedCPtr = getCPtr();
+  auto &UNakedCRef = getCRef();
+  // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: 'auto &UNakedCRef' can be declared as 'const auto &UNakedCRef'
+  // CHECK-FIXES: {{^}}  const auto &UNakedCRef = getCRef();
+  auto UNakedCRefDeref = getCRef();
+}
+
+}; // namespace usings
+
+int getInt();
+int *getIntPtr();
+const int *getCIntPtr();
+
+void foo() {
+  // make sure check disregards named types
+  int TypedInt = getInt();
+  int *TypedPtr = getIntPtr();
+  const int *TypedConstPtr = getCIntPtr();
+  int &TypedRef = *getIntPtr();
+  const int &TypedConstRef = *getCIntPtr();
+
+  // make sure check disregards auto types that aren't pointers or references
+  auto AutoInt = getInt();
+
+  auto NakedPtr = getIntPtr();
+  // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: 'auto NakedPtr' can be declared as 'auto *NakedPtr'
+  // CHECK-FIXES: {{^}}  auto *NakedPtr = getIntPtr();
+  auto NakedCPtr = getCIntPtr();
+  // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: 'auto NakedCPtr' can be declared as 'const auto *NakedCPtr'
+  // CHECK-FIXES: {{^}}  const auto *NakedCPtr = getCIntPtr();
+
+  const auto ConstPtr = getIntPtr();
+  // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: 'const auto ConstPtr' can be declared as 'auto *const ConstPtr'
+  // CHECK-FIXES: {{^}}  auto *const ConstPtr = getIntPtr();
+  const auto ConstCPtr = getCIntPtr();
+  // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: 'const auto ConstCPtr' can be declared as 'const auto *const ConstCPtr'
+  // CHECK-FIXES: {{^}}  const auto *const ConstCPtr = getCIntPtr();
+
+  volatile auto VolatilePtr = getIntPtr();
+  // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: 'volatile auto VolatilePtr' can be declared as 'auto *volatile VolatilePtr'
+  // CHECK-FIXES: {{^}}  auto *volatile VolatilePtr = getIntPtr();
+  volatile auto VolatileCPtr = getCIntPtr();
+  // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: 'volatile auto VolatileCPtr' can be declared as 'const auto *volatile VolatileCPtr'
+  // CHECK-FIXES: {{^}}  const auto *volatile VolatileCPtr = getCIntPtr();
+
+  auto *QualPtr = getIntPtr();
+  auto *QualCPtr = getCIntPtr();
+  // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: 'auto *QualCPtr' can be declared as 'const auto *QualCPtr'
+  // CHECK-FIXES: {{^}}  const auto *QualCPtr = getCIntPtr();
+  auto *const ConstantQualCPtr = getCIntPtr();
+  // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: 'auto *const ConstantQualCPtr' can be declared as 'const auto *const ConstantQualCPtr'
+  // CHECK-FIXES: {{^}}  const auto *const ConstantQualCPtr = getCIntPtr();
+  auto *volatile VolatileQualCPtr = getCIntPtr();
+  // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: 'auto *volatile VolatileQualCPtr' can be declared as 'const auto *volatile VolatileQualCPtr'
+  // CHECK-FIXES: {{^}}  const auto *volatile VolatileQualCPtr = getCIntPtr();
+  const auto *ConstQualCPtr = getCIntPtr();
+
+  auto &Ref = *getIntPtr();
+  auto &CRef = *getCIntPtr();
+  // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: 'auto &CRef' can be declared as 'const auto &CRef'
+  // CHECK-FIXES: {{^}}  const auto &CRef = *getCIntPtr();
+  const auto &ConstCRef = *getCIntPtr();
+
+  if (auto X = getCIntPtr()) {
+    // CHECK-MESSAGES: :[[@LINE-1]]:7: warning: 'auto X' can be declared as 'const auto *X'
+    // CHECK-FIXES: {{^}}  if (const auto *X = getCIntPtr()) {
+  }
+}
+
+void macroTest() {
+#define _AUTO auto
+#define _CONST const
+  _AUTO AutoMACROPtr = getIntPtr();
+  const _AUTO ConstAutoMacroPtr = getIntPtr();
+  _CONST _AUTO ConstMacroAutoMacroPtr = getIntPtr();
+  _CONST auto ConstMacroAutoPtr = getIntPtr();
+#undef _AUTO
+#undef _CONST
+}
+
+namespace std {
+template <typename T>
+class vector { // dummy impl
+  T _data[1];
+
+public:
+  T *begin() { return _data; }
+  const T *begin() const { return _data; }
+  T *end() { return &_data[1]; }
+  const T *end() const { return &_data[1]; }
+};
+} // namespace std
+
+void change(int &);
+void observe(const int &);
+
+void loopRef(std::vector<int> &Mutate, const std::vector<int> &Constant) {
+  for (auto &Data : Mutate) {
+    change(Data);
+  }
+  for (auto &Data : Constant) {
+    // CHECK-MESSAGES: :[[@LINE-1]]:8: warning: 'auto &Data' can be declared as 'const auto &Data'
+    // CHECK-FIXES: {{^}}  for (const auto &Data : Constant) {
+    observe(Data);
+  }
+}
+
+void loopPtr(const std::vector<int *> &Mutate, const std::vector<const int *> &Constant) {
+  for (auto Data : Mutate) {
+    // CHECK-MESSAGES: :[[@LINE-1]]:8: warning: 'auto Data' can be declared as 'auto *Data'
+    // CHECK-FIXES: {{^}}  for (auto *Data : Mutate) {
+    change(*Data);
+  }
+  for (auto Data : Constant) {
+    // CHECK-MESSAGES: :[[@LINE-1]]:8: warning: 'auto Data' can be declared as 'const auto *Data'
+    // CHECK-FIXES: {{^}}  for (const auto *Data : Constant) {
+    observe(*Data);
+  }
+}
+
+template <typename T>
+void tempLoopPtr(std::vector<T *> &MutateTemplate, std::vector<const T *> &ConstantTemplate) {
+  for (auto Data : MutateTemplate) {
+    // CHECK-MESSAGES: :[[@LINE-1]]:8: warning: 'auto Data' can be declared as 'auto *Data'
+    // CHECK-FIXES: {{^}}  for (auto *Data : MutateTemplate) {
+    change(*Data);
+  }
+  //FixMe
+  for (auto Data : ConstantTemplate) {
+    // CHECK-MESSAGES: :[[@LINE-1]]:8: warning: 'auto Data' can be declared as 'const auto *Data'
+    // CHECK-FIXES: {{^}}  for (const auto *Data : ConstantTemplate) {
+    observe(*Data);
+  }
+}
+
+template <typename T>
+class TemplateLoopPtr {
+public:
+  void operator()(const std::vector<T *> &MClassTemplate, const std::vector<const T *> &CClassTemplate) {
+    for (auto Data : MClassTemplate) {
+      // CHECK-MESSAGES: :[[@LINE-1]]:10: warning: 'auto Data' can be declared as 'auto *Data'
+      // CHECK-FIXES: {{^}}    for (auto *Data : MClassTemplate) {
+      change(*Data);
+    }
+    //FixMe
+    for (auto Data : CClassTemplate) {
+      // CHECK-MESSAGES: :[[@LINE-1]]:10: warning: 'auto Data' can be declared as 'const auto *Data'
+      // CHECK-FIXES: {{^}}    for (const auto *Data : CClassTemplate) {
+      observe(*Data);
+    }
+  }
+};
+
+void bar() {
+  std::vector<int> Vec;
+  std::vector<int *> PtrVec;
+  std::vector<const int *> CPtrVec;
+  loopRef(Vec, Vec);
+  loopPtr(PtrVec, CPtrVec);
+  tempLoopPtr(PtrVec, CPtrVec);
+  TemplateLoopPtr<int>()(PtrVec, CPtrVec);
+}
+
+typedef int *(*functionRetPtr)();
+typedef int (*functionRetVal)();
+
+functionRetPtr getPtrFunction();
+functionRetVal getValFunction();
+
+void baz() {
+  auto MyFunctionPtr = getPtrFunction();
+  // CHECK-MESSAGES-NOT: :[[@LINE-1]]:3: warning: 'auto MyFunctionPtr' can be declared as 'auto *MyFunctionPtr'
+  // CHECK-FIXES-NOT: {{^}}  auto *MyFunctionPtr = getPtrFunction();
+  auto MyFunctionVal = getValFunction();
+  // CHECK-MESSAGES-NOT: :[[@LINE-1]]:3: warning: 'auto MyFunctionVal' can be declared as 'auto *MyFunctionVal'
+  // CHECK-FIXES-NOT: {{^}}  auto *MyFunctionVal = getValFunction();
+
+  auto LambdaTest = [] { return 0; };
+  // CHECK-MESSAGES-NOT: :[[@LINE-1]]:3: warning: 'auto LambdaTest' can be declared as 'auto *LambdaTest'
+  // CHECK-FIXES-NOT: {{^}}  auto *LambdaTest = [] { return 0; };
+
+  auto LambdaTest2 = +[] { return 0; };
+  // CHECK-MESSAGES-NOT: :[[@LINE-1]]:3: warning: 'auto LambdaTest2' can be declared as 'auto *LambdaTest2'
+  // CHECK-FIXES-NOT: {{^}}  auto *LambdaTest2 = +[] { return 0; };
+
+  auto MyFunctionRef = *getPtrFunction();
+  // CHECK-MESSAGES-NOT: :[[@LINE-1]]:3: warning: 'auto MyFunctionRef' can be declared as 'auto *MyFunctionRef'
+  // CHECK-FIXES-NOT: {{^}}  auto *MyFunctionRef = *getPtrFunction();
+
+  auto &MyFunctionRef2 = *getPtrFunction();
+}