mikecrowe marked an inline comment as done.
mikecrowe added inline comments.


================
Comment at: clang-tools-extra/clang-tidy/modernize/UseStdPrintCheck.cpp:73-95
+static clang::ast_matchers::StatementMatcher
+unusedReturnValue(clang::ast_matchers::StatementMatcher MatchedCallExpr) {
+  auto UnusedInCompoundStmt =
+      compoundStmt(forEach(MatchedCallExpr),
+                   // The checker can't currently differentiate between the
+                   // return statement and other statements inside GNU 
statement
+                   // expressions, so disable the checker inside them to avoid
----------------
PiotrZSL wrote:
> NOTE: Personally I do not thing that this is right way. Instead of using 
> "inclusion" matcher, better would be to use elimination.
> like:
> ```callExpr(unless(hasParent(anyOf(varDecl(), callExpr(), ifStmt(), ...```.
> 
> But if it's working fine, then it could be for now, so lets leave it. Simply 
> with this it may not find all cases.
I'm happy to try doing it a different way. I just took this code from the 
`bugprone-unused-return-value` check. I had a go at using:
```C++
    Finder->addMatcher(
      callExpr(unless(hasParent(anyOf(varDecl(), callExpr(), ifStmt(), 
whileStmt(), doStmt(), forStmt(), cxxForRangeStmt(), switchCase()))), 
argumentCountAtLeast(1),
```
but there's no overload of `hasParent` that will accept the return type of 
`anyOf`:
```
/home/mac/git/llvm-project/clang-tools-extra/clang-tidy/modernize/UseStdPrintCheck.cpp:100:23:
 error: no matching function for call to object of type 'const 
internal::ArgumentAdaptingMatcherFunc<clang::ast_matchers::internal::HasParentMatcher,
 internal::TypeList<Decl, NestedNameSpecifierLoc, Stmt, TypeLoc, Attr>, 
internal::TypeList<Decl, NestedNameSpecifierLoc, Stmt, TypeLoc, Attr>>'
      callExpr(unless(hasParent(anyOf(varDecl(), callExpr(), ifStmt(), 
whileStmt(), doStmt(), forStmt(), cxxForRangeStmt(), switchCase()))), 
argumentCountAtLeast(1),
                      ^~~~~~~~~
/home/mac/git/llvm-project/clang/include/clang/ASTMatchers/ASTMatchersInternal.h:1491:3:
 note: candidate template ignored: could not match 'Matcher' against 
'VariadicOperatorMatcher'
  operator()(const Matcher<T> &InnerMatcher) const {
  ^
/home/mac/git/llvm-project/clang/include/clang/ASTMatchers/ASTMatchersInternal.h:1498:3:
 note: candidate template ignored: could not match 'MapAnyOfHelper' against 
'VariadicOperatorMatcher'
  operator()(const MapAnyOfHelper<T...> &InnerMatcher) const {
```
(Reducing the set of matchers inside the `anyOf` didn't help.)


================
Comment at: 
clang-tools-extra/test/clang-tidy/checkers/modernize/use-std-print-absl.cpp:63
+  // CHECK-MESSAGES-NOT: [[@LINE-1]]:10: warning: use 'std::println' instead 
of 'PrintF' [modernize-use-std-print]
+  // CHECK-FIXES-NOT: std::println("return value {}", i);
+}
----------------
PiotrZSL wrote:
> NOTE: I don't think that those FIXES-NOT are needed.
OK. I'll leave only the ones that are for deficiencies that might one day be 
fixed.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D153860

_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Reply via email to