[PATCH] D72217: [clang-tidy] Added readability-qualified-auto check

[Nathan James via Phabricator via cfe-commits]

njames93 marked 10 inline comments as done.
njames93 added inline comments.


================
Comment at: clang-tools-extra/clang-tidy/readability/QualifiedAutoCheck.cpp:31
+  Const,
+  Volatile // Ignore restrict as not c++ keyword.
+};
----------------
aaron.ballman wrote:
> 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.)
I'll add in support for it


================
Comment at: clang-tools-extra/clang-tidy/readability/QualifiedAutoCheck.cpp:94-95
+  auto Pointee = QType.getTypePtr()->getPointeeType();
+  if (Pointee.isNull())
+    return Pointee.isLocalConstQualified();
+  return Pointee.isConstQualified();
----------------
aaron.ballman wrote:
> 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.
Well it kept crashing without that check so I need to do some more digging on that front 


================
Comment at: clang-tools-extra/clang-tidy/readability/QualifiedAutoCheck.cpp:159
+void QualifiedAutoCheck::check(const MatchFinder::MatchResult &Result) {
+  if (auto Var = Result.Nodes.getNodeAs<VarDecl>("auto")) {
+    bool IsPtrConst = isPointerConst(Var->getType());
----------------
Eugene.Zelenko wrote:
> aaron.ballman wrote:
> > `auto *`
> This would be nice test for check.
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


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D72217/new/

https://reviews.llvm.org/D72217