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

[Sam McCall via Phabricator via cfe-commits]

sammccall added a comment.

In D72217#1844204 <https://reviews.llvm.org/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.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D72217