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

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. https://reviews.llvm.org/D41

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

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

[PATCH] D41056: [clang-tidy] New check misc-uniqueptr-release-unused-retval

khuttun abandoned this revision. khuttun added a comment. Closing this as more general check is being reviewed here: https://reviews.llvm.org/D41655 Repository: rL LLVM https://reviews.llvm.org/D41056 ___ cfe-commits mailing list cfe-commits@lis

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

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

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 clang-tidy/bugprone/UnusedReturn

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

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()) +<< Matched->getSour

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

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 clang-tidy/bugprone/UnusedReturn

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

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 we

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

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 clang-tidy/bugprone/UnusedReturn

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

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 tha

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

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: > > > > aar

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

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 clang-tidy/bugprone/UnusedReturnValueChe

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

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

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. That

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

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 clang-tidy/bugprone/UnusedReturnValueCheck.h

[PATCH] D59103: [clang-tidy] New checker bugprone-incomplete-comparison-operator

kallehuttunen created this revision. kallehuttunen added reviewers: alexfh, hokein, JonasToth, aaron.ballman. kallehuttunen added a project: clang-tools-extra. Herald added subscribers: jdoerfert, xazax.hun, mgorny. Herald added a project: clang. The checker detects comparison operators that don't

[PATCH] D59103: [clang-tidy] New checker bugprone-incomplete-comparison-operator

kallehuttunen added a comment. The checker gives quite many warnings on LLVM code base. For example, running it for lib/Transforms/Scalar: /home/kalle/llvm/lib/Transforms/Scalar/GVN.cpp:119:3: warning: incomplete comparison operator [bugprone-incomplete-comparison-operator] bool operator=

[PATCH] D59103: [clang-tidy] New checker bugprone-incomplete-comparison-operator

kallehuttunen added a comment. In D59103#1421875 , @lebedev.ri wrote: > Thank you for working on this! > > This looks questionable to me. > Is this based on some coding standard? > Are there any numbers on the false-positive rate of theck? It's not bas

[PATCH] D59103: [clang-tidy] New checker bugprone-incomplete-comparison-operator

kallehuttunen added a comment. I found this checker to be useful in the code base I initially developed it for, but the usage of comparison operators there is pretty much limited to comparing simple aggregate types. It's true that this checker can produce lots of false positives, maybe too much

[PATCH] D59103: [clang-tidy] New checker bugprone-incomplete-comparison-operator

kallehuttunen added a comment. Another idea that came to my mind would be to enable this check only for annotated types. So warning for missing field access would be only given for types that have for example `[[clang::annotate("value type")]]` annotation. Possibly other kinds of checks could b

[PATCH] D59103: [clang-tidy] New checker bugprone-incomplete-comparison-operator

kallehuttunen added a comment. In D59103#1423307 , @aaron.ballman wrote: > In D59103#1422775 , @kallehuttunen > wrote: > > > Another idea that came to my mind would be to enable this check only for > > annotated

[PATCH] D45891: [clang-tidy] Improve bugprone-unused-return-value check

khuttun created this revision. khuttun added a reviewer: alexfh. Herald added subscribers: cfe-commits, xazax.hun. Add support for checking class template member functions. Also add the following functions to be checked by default: - std::unique_ptr::release - std::basic_string::empty - std::vec

[PATCH] D45891: [clang-tidy] Improve bugprone-unused-return-value check

khuttun added a comment. Could someone help getting this merged? I don't have commit access to the repo. Repository: rCTE Clang Tools Extra https://reviews.llvm.org/D45891 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.

[PATCH] D45891: [clang-tidy] Improve bugprone-unused-return-value check

khuttun added a comment. Thank you! Repository: rCTE Clang Tools Extra https://reviews.llvm.org/D45891 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D46317: [clang-tidy] New check bugprone-map-subscript-operator-lookup

khuttun created this revision. khuttun added a reviewer: alexfh. Herald added subscribers: xazax.hun, mgorny. Warns on std::map and std::unordered_map lookups done with operator[]. Inserting values with operator[] is still allowed. Repository: rCTE Clang Tools Extra https://reviews.llvm.org/

[PATCH] D46317: [clang-tidy] New check bugprone-map-subscript-operator-lookup

khuttun added a comment. This checker is inspired by Louis Brandy's excellent CppCon 2017 talk (https://www.youtube.com/watch?v=lkgszkPnV8g) where he describes the side-effects of std::map::operator[] as being one of the most common source of bugs in Facebook's C++ code. In the talk he mentions

[PATCH] D46317: [clang-tidy] New check bugprone-map-subscript-operator-lookup

khuttun abandoned this revision. khuttun added a comment. Abandoning this. The false positive rate would be too high for this checker. Repository: rCTE Clang Tools Extra https://reviews.llvm.org/D46317 ___ cfe-commits mailing list cfe-commits@lis

[PATCH] D46317: [clang-tidy] New check bugprone-map-subscript-operator-lookup

khuttun marked an inline comment as done. khuttun added a comment. In D46317#1988261 , @lebedev.ri wrote: > IMHO we should proceed with this patch. > There's been several patches recently that seem to not care about > false-positive rate, > and in this

[PATCH] D46317: [clang-tidy] New check bugprone-map-subscript-operator-lookup

khuttun updated this revision to Diff 259656. khuttun added a comment. The patch is now updated to be based on the monorepo. I also fixed all the comments from the previous review. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D46317/new/ https://reviews.llvm.org/D46317 Files: clang-

[PATCH] D46317: [clang-tidy] New check bugprone-map-subscript-operator-lookup

khuttun added a comment. I ran the check for LLVM code using configuration `key: bugprone-map-subscript-operator-lookup.MapTypes, value: "::std::map;::std::unordered_map;::llvm::DenseMapBase;::llvm::MapVector;::llvm::StringMap"`. It gave 1212 warnings (see the attachment).F11784386: warnings.tx

[PATCH] D46317: [clang-tidy] New check bugprone-map-subscript-operator-lookup

khuttun updated this revision to Diff 259843. khuttun added a comment. Documented the MapTypes check option. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D46317/new/ https://reviews.llvm.org/D46317 Files: clang-tools-extra/clang-tidy/bugprone/BugproneTidyModule.cpp clang-tools-extr

[PATCH] D46317: [clang-tidy] New check bugprone-map-subscript-operator-lookup

khuttun updated this revision to Diff 261141. khuttun added a comment. Fixed @Eugene.Zelenko's comments CHANGES SINCE LAST ACTION https://reviews.llvm.org/D46317/new/ https://reviews.llvm.org/D46317 Files: clang-tools-extra/clang-tidy/bugprone/BugproneTidyModule.cpp clang-tools-extra/cla

[PATCH] D46317: [clang-tidy] New check bugprone-map-subscript-operator-lookup

khuttun updated this revision to Diff 261142. khuttun added a comment. Accidentally removed the options documentation in the previous patch... Now added it back. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D46317/new/ https://reviews.llvm.org/D46317 Files: clang-tools-extra/clang-t

[PATCH] D46317: [clang-tidy] New check bugprone-map-subscript-operator-lookup

khuttun added a comment. Any comments on this? Is this checker something that could be part of clang-tidy? CHANGES SINCE LAST ACTION https://reviews.llvm.org/D46317/new/ https://reviews.llvm.org/D46317 ___ cfe-commits mailing list cfe-commits@li

[PATCH] D46317: [clang-tidy] New check bugprone-map-subscript-operator-lookup

khuttun added a comment. In D46317#2027071 , @aaron.ballman wrote: > In D46317#2023406 , @khuttun wrote: > > > Any comments on this? Is this checker something that could be part of > > clang-tidy? > > > Thank you

[PATCH] D41056: [clang-tidy] New check misc-uniqueptr-release-unused-retval

khuttun created this revision. Herald added subscribers: xazax.hun, mgorny. Detects calls to std::unique_ptr::release where the return value is unused. https://reviews.llvm.org/D41056 Files: clang-tidy/misc/CMakeLists.txt clang-tidy/misc/MiscTidyModule.cpp clang-tidy/misc/UniqueptrRelease

[PATCH] D41056: [clang-tidy] New check misc-uniqueptr-release-unused-retval

khuttun added a comment. In https://reviews.llvm.org/D41056#950570, @Eugene.Zelenko wrote: > May be //bugprone// is better module then //misc//? Maybe. I can move it if all the reviewers think that it would be better suited there. Repository: rL LLVM https://reviews.llvm.org/D41056 ___

[PATCH] D41056: [clang-tidy] New check misc-uniqueptr-release-unused-retval

khuttun added a comment. In https://reviews.llvm.org/D41056#951145, @aaron.ballman wrote: > In https://reviews.llvm.org/D41056#951083, @alexfh wrote: > > > In https://reviews.llvm.org/D41056#950605, @khuttun wrote: > > > > > In https://reviews.llvm.org/D41056#950570, @Eugene.Zelenko wrote: > > >

[PATCH] D45161: [AST] Add new printing policy to suppress printing template arguments

khuttun created this revision. khuttun added reviewers: sepavloff, alexfh. Herald added a subscriber: cfe-commits. The purpose of this addition is to be able to write AST matchers that match class template member functions by fully qualified name, without the need to explicitly specify the templ

[PATCH] D45161: [AST] Add new printing policy to suppress printing template arguments

khuttun abandoned this revision. khuttun added a comment. Figured out a better way to do this by using `FunctionDecl::getInstantiatedFromMemberFunction` Repository: rC Clang https://reviews.llvm.org/D45161 ___ cfe-commits mailing list cfe-commit

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

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 add

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

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 https://reviews.llvm.org/D416

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

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

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

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 re