[PATCH] D124446: [clang-tidy] Add the misc-discarded-return-value check

[Whisperity via Phabricator via cfe-commits]

whisperity added inline comments.


================
Comment at: clang-tools-extra/clang-tidy/misc/DiscardedReturnValueCheck.cpp:96-99
+AST_MATCHER_P(StaticAssertDecl, hasAssertExpr,
+              ast_matchers::internal::Matcher<Expr>, InnerMatcher) {
+  return InnerMatcher.matches(*Node.getAssertExpr(), Finder, Builder);
+}
----------------
This is superfluous. The error message of a `static_assert` is always a string literal that has to be //right there// (i.e. a `constexpr const char *` function is NOT accepted). So if you are matching and ignoring the `ImplicitCastExpr`s, the following //SHOULD// match the static assert just fine:

```lang=cpp
staticAssertDecl(has(callExpr()))
```

http://godbolt.org/z/oeWfh3Mjo


================
Comment at: clang-tools-extra/clang-tidy/misc/DiscardedReturnValueCheck.cpp:106-126
+// FIXME: Move ASTMatcher library.
+// Note: Taken from UnusedUsingDeclsCheck.
+AST_POLYMORPHIC_MATCHER_P(
+    forEachTemplateArgument,
+    AST_POLYMORPHIC_SUPPORTED_TYPES(ClassTemplateSpecializationDecl,
+                                    TemplateSpecializationType, FunctionDecl),
+    clang::ast_matchers::internal::Matcher<TemplateArgument>, InnerMatcher) {
----------------
martong wrote:
> Could you please have this in an independent parent patch?
D125383


================
Comment at: clang-tools-extra/test/clang-tidy/checkers/misc-discarded-return-value-50p.cpp:631-635
+  // FIXME: Because call<Lambda> is a template, calls to it, even with different
+  // lambdas are calculated together, but the resulting diagnostic message is
+  // wrong. The culprit seems to be the USR generated for
+  // 'call<(lambda in lambdaCallerTest)>' being
+  // "c:misc-discarded-return-value-50p.cpp at F@call<#&$@F at lambdaCallerTest#@Sa>#S0_#"
----------------
(⏰ Stale comment leaked back in time from the later cross-TU implementation. This patch does not involve USRs yet.)


================
Comment at: clang-tools-extra/test/clang-tidy/checkers/misc-discarded-return-value-50p.cpp:645-646
+
+  // Here, call<...> has a different key:
+  // "c:misc-discarded-return-value-50p.cpp at F@call<#&$@F at lambdaCallerTest2#@Sa>#S0_#"
+}
----------------
[⏰ Ditto]


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D124446