[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] D59103: [clang-tidy] New checker bugprone-incomplete-comparison-operator

aaron.ballman added a comment. In D59103#1422775 , @kallehuttunen wrote: > 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 exam

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

aaron.ballman added a comment. I'm definitely worried about the false positive rate for this check -- from a quick look at the diagnostics from LLVM, every one of them is a false positive (though perhaps I've missed something; it was a very quick look). Requiring users to explicitly use a decla

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

Eugene.Zelenko added inline comments. Comment at: clang-tidy/bugprone/IncompleteComparisonOperatorCheck.cpp:197 + + const auto Ops = utils::options::parseStringList(CheckedFunctions); + Finder->addMatcher( Please don't use auto where return type is not obvious.

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

JonasToth added a comment. @lebedev.ri @kallehuttunen what do you think of putting this into context of the proposal (i believe its being standardized with c++20) of `operator==() = default` and the spaceship-operator. This check could serve as a basis to modernize the `operator==` to the defaul

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

MyDeveloperDay added a comment. I definately been burnt by not handling all the cases, but imagine a string class class mystring { const char *ptr; int size; int numallocated; } to compare the strings I wouldn't want to compare all the fields? I think this could lead to a lot of false

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

lebedev.ri added inline comments. Comment at: clang-tidy/bugprone/IncompleteComparisonOperatorCheck.cpp:242-243 +diag(Operator->getBeginLoc(), "incomplete comparison operator"); +Lhs->diagFields(*this, Operator->getBeginLoc()); +Rhs->diagFields(*this, Operator->getBeg

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

lebedev.ri added a comment. 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? Repository: rCTE Clang Tools Extra CHANGES SINCE LAST ACTION https://reviews.llvm.org/D59103/new/ htt

[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