[PATCH] D119470: [clang-tidy] Don't check decltype return types in `readability-const-return-type`

[Evgeny Shulgin via Phabricator via cfe-commits]

Izaron marked 3 inline comments as done.
Izaron added a comment.

> You've submitted some quality patches already, would you be interested in obtaining commit privileges (https://llvm.org/docs/DeveloperPolicy.html#obtaining-commit-access)?

Thanks! I obtained the commit access =) Now I don't have to ask other peoples commit on my behalf.



================
Comment at: clang-tools-extra/clang-tidy/readability/ConstReturnTypeCheck.cpp:56-58
+AST_MATCHER(QualType, isLocalConstQualified) {
+  return Node.isLocalConstQualified();
+}
----------------
aaron.ballman wrote:
> I think we might as well hit all the local qualifiers instead of just `const`, WDYT? e.g., `volatile` et al via `hasLocalQualifiers()`
In this case the checker will complain on this code:
```
const int i = 1;
volatile decltype(i) n() {
  return 12345;
}
```
```
warning: return type 'volatile decltype(i)' (aka 'const volatile int') is 'const'-qualified at the top level, which may reduce code readability without improving const correctness [readability-const-return-type]
```
That wouldn't be the correct outcome.

I added the test for the snippet above.


================
Comment at: clang-tools-extra/clang-tidy/readability/ConstReturnTypeCheck.cpp:104
+  const auto NonLocalConstDecltypeType = qualType(
+      unless(isLocalConstQualified()), anyOf(decltypeType(), autoType()));
   Finder->addMatcher(
----------------
aaron.ballman wrote:
> How about a `TypeOfType`? (The GCC `typeof(foo)` extension)
Thanks! Didn't know about this extension. Now the checker filters this out.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D119470