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