[PATCH] D55433: [clang-tidy] Adding a new modernize use nodiscard checker

[MyDeveloperDay via Phabricator via cfe-commits]

MyDeveloperDay marked an inline comment as done.
MyDeveloperDay added a comment.



In D55433#1349709 <https://reviews.llvm.org/D55433#1349709>, @JonasToth wrote:

> No problem, thats why we test on real projects first, because there is always something hidden in C++ :)
>
> Is there a test for the lambdas?


test/clang-tidy/modernize-use-nodiscard.cpp line 158  (which is how it manifested inside LLVM) as a lambda assigned to an auto



================
Comment at: clang-tidy/modernize/UseNodiscardCheck.cpp:106
+                    isVariadic(), hasTemplateReturnType(),
+                    hasParent(cxxRecordDecl(isLambda())),
+                    hasClassMutableFields(),
----------------
JonasToth wrote:
> what happens for nested lambdas?
> `hasParent` should be avoided if possible, as the `clangd` folks are currently implementing partial traversal to only analyze "the latest change". If you can, please rewrite that without `hasParent`
I'll not deny I wasn't sure how to do this one, and totally stole the idea from another checker

fuchsia/TrailingReturnCheck.cpp:                                hasParent(cxxRecordDecl(isLambda())))))

> hasParent(cxxRecordDecl(isLambda())),

Given that the matcher is on all CXXMethodDecl, I couldn't quite see how I could determine the CXXMethodDecl was in a lambda, without going to the parent  

{F7804868}

without it I'd get

```
const auto nonConstReferenceType = [] {
   return true;
 };

``` 

transformed to


```
const auto nonConstReferenceType = [[nodiscard]] [] {
  return true;
};

```

Which whilst probably true, made for some ugly reading..

not sure about nested lambdas...sounds even harder!









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

https://reviews.llvm.org/D55433