[PATCH] D41655: [clang-tidy] New check bugprone-unused-return-value

2018-03-19 Thread Alexander Kornienko via Phabricator via cfe-commits
This revision was automatically updated to reflect the committed changes. Closed by commit rL327833: [clang-tidy] New check bugprone-unused-return-value (authored by alexfh, committed by ). Herald added subscribers: llvm-commits, klimek. Changed prior to commit:

[PATCH] D41655: [clang-tidy] New check bugprone-unused-return-value

2018-03-15 Thread Kalle Huttunen via Phabricator via cfe-commits
khuttun updated this revision to Diff 138595. khuttun added a comment. Rebased https://reviews.llvm.org/D41655 Files: clang-tidy/bugprone/BugproneTidyModule.cpp clang-tidy/bugprone/CMakeLists.txt clang-tidy/bugprone/UnusedReturnValueCheck.cpp

[PATCH] D41655: [clang-tidy] New check bugprone-unused-return-value

2018-03-15 Thread Alexander Kornienko via Phabricator via cfe-commits
alexfh added a comment. In https://reviews.llvm.org/D41655#1037552, @khuttun wrote: > In https://reviews.llvm.org/D41655#1037234, @alexfh wrote: > > > Do you need help committing the patch? > > > Yes please, I don't have commit access to the repo. The patch doesn't apply cleanly. Please rebase

[PATCH] D41655: [clang-tidy] New check bugprone-unused-return-value

2018-03-14 Thread Kalle Huttunen via Phabricator via cfe-commits
khuttun added a comment. In https://reviews.llvm.org/D41655#1037234, @alexfh wrote: > Do you need help committing the patch? Yes please, I don't have commit access to the repo. I think the next step for improving this checker could be to make it work with class template member functions.

[PATCH] D41655: [clang-tidy] New check bugprone-unused-return-value

2018-03-14 Thread Alexander Kornienko via Phabricator via cfe-commits
alexfh added a comment. Do you need help committing the patch? https://reviews.llvm.org/D41655 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D41655: [clang-tidy] New check bugprone-unused-return-value

2018-03-14 Thread Alexander Kornienko via Phabricator via cfe-commits
alexfh accepted this revision. alexfh added a comment. LG. Thank you! BTW, there's an old related patch https://reviews.llvm.org/D17043, which mostly focuses on member functions of STL containers. It might be useful as a reference (there's a pretty extensive list of member functions and

[PATCH] D41655: [clang-tidy] New check bugprone-unused-return-value

2018-03-12 Thread Aaron Ballman via Phabricator via cfe-commits
aaron.ballman accepted this revision. aaron.ballman added a comment. This revision is now accepted and ready to land. I think this check LGTM. https://reviews.llvm.org/D41655 ___ cfe-commits mailing list cfe-commits@lists.llvm.org

[PATCH] D41655: [clang-tidy] New check bugprone-unused-return-value

2018-03-11 Thread Kalle Huttunen via Phabricator via cfe-commits
khuttun added a comment. Should I close this review? https://reviews.llvm.org/D41655 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D41655: [clang-tidy] New check bugprone-unused-return-value

2018-02-17 Thread Kalle Huttunen via Phabricator via cfe-commits
khuttun updated this revision to Diff 134802. khuttun added a comment. I changed the checker to use hasAnyName. Checking for `unique_ptr::release()` and all the `empty()` functions is removed. The checker doesn't report any warnings from LLVM + clang codebases now.

[PATCH] D41655: [clang-tidy] New check bugprone-unused-return-value

2018-02-03 Thread Kalle Huttunen via Phabricator via cfe-commits
khuttun added a comment. > From what I can tell of these reports, they almost all boil down to ignoring > the call to `release()` because the pointer is no longer owned by the > `unique_ptr`. This is a pretty reasonable code pattern, but it also seems > reasonable to expect users to cast the

[PATCH] D41655: [clang-tidy] New check bugprone-unused-return-value

2018-02-02 Thread Aaron Ballman via Phabricator via cfe-commits
aaron.ballman added a comment. In https://reviews.llvm.org/D41655#980672, @khuttun wrote: > The checker reports 7 warnings on LLVM + Clang code bases, all on > std::unique_ptr::release: > > lib/Bitcode/Reader/BitReader.cpp:114:3 > > - release() called on moved-from unique_ptr > - no harm, just

[PATCH] D41655: [clang-tidy] New check bugprone-unused-return-value

2018-02-02 Thread Alexander Kornienko via Phabricator via cfe-commits
alexfh added inline comments. Comment at: clang-tidy/bugprone/UnusedReturnValueCheck.cpp:45-48 +"^::std::async$|" +"^::std::launder$|" +"^::std::remove$|" +

[PATCH] D41655: [clang-tidy] New check bugprone-unused-return-value

2018-02-02 Thread Alexander Kornienko via Phabricator via cfe-commits
alexfh added inline comments. Comment at: clang-tidy/bugprone/UnusedReturnValueCheck.cpp:36 + Node.printQualifiedName(OS, Policy); + return llvm::Regex(RegExp).match(OS.str()); +} Can we avoid creating the regex on each match? For example, by changing the

[PATCH] D41655: [clang-tidy] New check bugprone-unused-return-value

2018-01-31 Thread Kalle Huttunen via Phabricator via cfe-commits
khuttun added a comment. Herald added a subscriber: hintonda. Any more comments on this? https://reviews.llvm.org/D41655 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D41655: [clang-tidy] New check bugprone-unused-return-value

2018-01-18 Thread Kalle Huttunen via Phabricator via cfe-commits
khuttun added a comment. The checker reports 7 warnings on LLVM + Clang code bases, all on std::unique_ptr::release: lib/Bitcode/Reader/BitReader.cpp:114:3 - release() called on moved-from unique_ptr - no harm, just unnecessary lib/ExecutionEngine/ExecutionEngine.cpp:149:7 - release() called

[PATCH] D41655: [clang-tidy] New check bugprone-unused-return-value

2018-01-18 Thread Kalle Huttunen via Phabricator via cfe-commits
khuttun updated this revision to Diff 130461. khuttun added a comment. - Detect unused return values also inside other kinds of statements than compound statements - Ignore void functions in the checker - Check std::remove, std::remove_if and std::unique by default

[PATCH] D41655: [clang-tidy] New check bugprone-unused-return-value

2018-01-15 Thread Jonas Toth via Phabricator via cfe-commits
JonasToth added inline comments. Comment at: clang-tidy/bugprone/UnusedReturnValueCheck.cpp:47 +"^::std::launder$|" +"^::std::unique_ptr<.*>::release$|" +"^::std::.*::allocate$|"

[PATCH] D41655: [clang-tidy] New check bugprone-unused-return-value

2018-01-13 Thread Aaron Ballman via Phabricator via cfe-commits
aaron.ballman added inline comments. Comment at: test/clang-tidy/bugprone-unused-return-value.cpp:210 + // test that the check is disabled inside GNU statement expressions + ({ std::async(increment, 42); }); + auto StmtExprRetval = ({ std::async(increment, 42); });

[PATCH] D41655: [clang-tidy] New check bugprone-unused-return-value

2018-01-13 Thread Kalle Huttunen via Phabricator via cfe-commits
khuttun added a comment. In https://reviews.llvm.org/D41655#974725, @JonasToth wrote: > High Integrity C++ has the rule `17.5.1 Do not ignore the result of > std::remove, std::remove_if or std::unique`. Do we want to add those to the > preconfigured list? To me these sound like reasonable

[PATCH] D41655: [clang-tidy] New check bugprone-unused-return-value

2018-01-12 Thread Jonas Toth via Phabricator via cfe-commits
JonasToth added inline comments. Comment at: clang-tidy/bugprone/UnusedReturnValueCheck.cpp:47 +"^::std::launder$|" +"^::std::unique_ptr<.*>::release$|" +"^::std::.*::allocate$|"

[PATCH] D41655: [clang-tidy] New check bugprone-unused-return-value

2018-01-12 Thread Jonas Toth via Phabricator via cfe-commits
JonasToth added a comment. High Integrity C++ has the rule `17.5.1 Do not ignore the result of std::remove, std::remove_if or std::unique`. Do we want to add those to the preconfigured list? https://reviews.llvm.org/D41655 ___ cfe-commits mailing

[PATCH] D41655: [clang-tidy] New check bugprone-unused-return-value

2018-01-11 Thread Aaron Ballman via Phabricator via cfe-commits
aaron.ballman added inline comments. Comment at: test/clang-tidy/bugprone-unused-return-value.cpp:210 + // test that the check is disabled inside GNU statement expressions + ({ std::async(increment, 42); }); + auto StmtExprRetval = ({ std::async(increment, 42); });

[PATCH] D41655: [clang-tidy] New check bugprone-unused-return-value

2018-01-09 Thread Kalle Huttunen via Phabricator via cfe-commits
khuttun updated this revision to Diff 129090. khuttun added a comment. The checker is now disabled inside GNU statement expressions https://reviews.llvm.org/D41655 Files: clang-tidy/bugprone/BugproneTidyModule.cpp clang-tidy/bugprone/CMakeLists.txt

[PATCH] D41655: [clang-tidy] New check bugprone-unused-return-value

2018-01-08 Thread Kalle Huttunen via Phabricator via cfe-commits
khuttun added inline comments. Comment at: test/clang-tidy/bugprone-unused-return-value.cpp:163 + +void noWarning() { + auto AsyncRetval1 = std::async(increment, 42); aaron.ballman wrote: > khuttun wrote: > > aaron.ballman wrote: > > > khuttun wrote: > > > >

[PATCH] D41655: [clang-tidy] New check bugprone-unused-return-value

2018-01-08 Thread Aaron Ballman via Phabricator via cfe-commits
aaron.ballman added inline comments. Comment at: test/clang-tidy/bugprone-unused-return-value.cpp:163 + +void noWarning() { + auto AsyncRetval1 = std::async(increment, 42); khuttun wrote: > aaron.ballman wrote: > > khuttun wrote: > > > aaron.ballman wrote: > >

[PATCH] D41655: [clang-tidy] New check bugprone-unused-return-value

2018-01-08 Thread Kalle Huttunen via Phabricator via cfe-commits
khuttun added inline comments. Comment at: test/clang-tidy/bugprone-unused-return-value.cpp:163 + +void noWarning() { + auto AsyncRetval1 = std::async(increment, 42); aaron.ballman wrote: > khuttun wrote: > > aaron.ballman wrote: > > > Sorry, I just realized

[PATCH] D41655: [clang-tidy] New check bugprone-unused-return-value

2018-01-08 Thread Aaron Ballman via Phabricator via cfe-commits
aaron.ballman added inline comments. Comment at: test/clang-tidy/bugprone-unused-return-value.cpp:163 + +void noWarning() { + auto AsyncRetval1 = std::async(increment, 42); khuttun wrote: > aaron.ballman wrote: > > Sorry, I just realized that we're missing a

[PATCH] D41655: [clang-tidy] New check bugprone-unused-return-value

2018-01-07 Thread Kalle Huttunen via Phabricator via cfe-commits
khuttun updated this revision to Diff 128879. khuttun added a comment. Fix review comments https://reviews.llvm.org/D41655 Files: clang-tidy/bugprone/BugproneTidyModule.cpp clang-tidy/bugprone/CMakeLists.txt clang-tidy/bugprone/UnusedReturnValueCheck.cpp

[PATCH] D41655: [clang-tidy] New check bugprone-unused-return-value

2018-01-07 Thread Kalle Huttunen via Phabricator via cfe-commits
khuttun marked 2 inline comments as done. khuttun added inline comments. Comment at: test/clang-tidy/bugprone-unused-return-value.cpp:163 + +void noWarning() { + auto AsyncRetval1 = std::async(increment, 42); aaron.ballman wrote: > Sorry, I just realized that

[PATCH] D41655: [clang-tidy] New check bugprone-unused-return-value

2018-01-06 Thread Aaron Ballman via Phabricator via cfe-commits
aaron.ballman added inline comments. Comment at: clang-tidy/bugprone/UnusedReturnValueCheck.cpp:69 + "the value returned by %0 should normally be used") +<< dyn_cast_or_null(Matched->getCalleeDecl()) +<< Matched->getSourceRange(); khuttun

[PATCH] D41655: [clang-tidy] New check bugprone-unused-return-value

2018-01-06 Thread Kalle Huttunen via Phabricator via cfe-commits
khuttun updated this revision to Diff 128844. khuttun added a comment. Fix review comments https://reviews.llvm.org/D41655 Files: clang-tidy/bugprone/BugproneTidyModule.cpp clang-tidy/bugprone/CMakeLists.txt clang-tidy/bugprone/UnusedReturnValueCheck.cpp

[PATCH] D41655: [clang-tidy] New check bugprone-unused-return-value

2018-01-06 Thread Kalle Huttunen via Phabricator via cfe-commits
khuttun marked an inline comment as done. khuttun added inline comments. Comment at: clang-tidy/bugprone/UnusedReturnValueCheck.cpp:69 + "the value returned by %0 should normally be used") +<< dyn_cast_or_null(Matched->getCalleeDecl()) +<<

[PATCH] D41655: [clang-tidy] New check bugprone-unused-return-value

2018-01-04 Thread Aaron Ballman via Phabricator via cfe-commits
aaron.ballman added inline comments. Comment at: clang-tidy/bugprone/UnusedReturnValueCheck.cpp:68 +diag(Matched->getLocStart(), + "the value returned by %0 should normally be used") +<< dyn_cast_or_null(Matched->getCalleeDecl()) "Normally"

[PATCH] D41655: [clang-tidy] New check bugprone-unused-return-value

2018-01-03 Thread Kalle Huttunen via Phabricator via cfe-commits
khuttun updated this revision to Diff 128544. khuttun added a comment. Fix review comments https://reviews.llvm.org/D41655 Files: clang-tidy/bugprone/BugproneTidyModule.cpp clang-tidy/bugprone/CMakeLists.txt clang-tidy/bugprone/UnusedReturnValueCheck.cpp

[PATCH] D41655: [clang-tidy] New check bugprone-unused-return-value

2018-01-02 Thread Kalle Huttunen via Phabricator via cfe-commits
khuttun marked 7 inline comments as done. khuttun added a comment. In https://reviews.llvm.org/D41655#965551, @JonasToth wrote: > I think it would be more user friendly if the configured list can be a list > and the `|` concatenation is done within your code. What do you exactly mean by list?

[PATCH] D41655: [clang-tidy] New check bugprone-unused-return-value

2018-01-01 Thread Aaron Ballman via Phabricator via cfe-commits
aaron.ballman added inline comments. Comment at: clang-tidy/bugprone/UnusedReturnValueCheck.cpp:62-63 +void UnusedReturnValueCheck::check(const MatchFinder::MatchResult ) { + if (const auto Matched = Result.Nodes.getNodeAs("match")) { +diag(Matched->getLocStart(), "unused

[PATCH] D41655: [clang-tidy] New check bugprone-unused-return-value

2018-01-01 Thread Jonas Toth via Phabricator via cfe-commits
JonasToth added a comment. I think it would be more user friendly if the configured list can be a list and the `|` concatenation is done within your code. Comment at: clang-tidy/bugprone/UnusedReturnValueCheck.cpp:29 + llvm::raw_svector_ostream OS(InlinedName); + auto

[PATCH] D41655: [clang-tidy] New check bugprone-unused-return-value

2018-01-01 Thread Eugene Zelenko via Phabricator via cfe-commits
Eugene.Zelenko added inline comments. Comment at: clang-tidy/bugprone/UnusedReturnValueCheck.cpp:13 +#include "clang/ASTMatchers/ASTMatchFinder.h" + +using namespace clang::ast_matchers; Please include cassert, Regex.h, raw_ostream.h, SmallString.h.

[PATCH] D41655: [clang-tidy] New check bugprone-unused-return-value

2018-01-01 Thread Kalle Huttunen via Phabricator via cfe-commits
khuttun created this revision. khuttun added reviewers: alexfh, aaron.ballman. khuttun added a project: clang-tools-extra. Herald added subscribers: xazax.hun, mgorny. Detects function calls where the return value is unused. Checked functions can be configured. https://reviews.llvm.org/D41655