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

[Jonas Toth via Phabricator via cfe-commits]

JonasToth added a comment.

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?



================
Comment at: clang-tidy/modernize/UseNodiscardCheck.cpp:39
+AST_MATCHER(CXXMethodDecl, isConversionDecl) {
+  // Don't put ``[[nodiscard]]`` in front of a conversion decl
+  // like operator bool().
----------------
I would prefer `isConversionOperator`. Its consistent with `isOverloadedOperator` and uses the right word (`operator bool` is an operator)


================
Comment at: clang-tidy/modernize/UseNodiscardCheck.cpp:106
+                    isVariadic(), hasTemplateReturnType(),
+                    hasParent(cxxRecordDecl(isLambda())),
+                    hasClassMutableFields(),
----------------
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`


================
Comment at: test/clang-tidy/modernize-use-nodiscard.cpp:141
+
+    // Do not add ``[[nodiscard]]`` to paramaeter packs.
+    template <class... Args>
----------------
typo, paramaeter


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

https://reviews.llvm.org/D55433