[PATCH] D45601: Warn on bool* to bool conversion

hiraditya abandoned this revision. hiraditya added a comment. It appears this warning may not be always useful because there will be false positives. https://reviews.llvm.org/D45601 ___ cfe-commits mailing list cfe-commits@lists.llvm.org

[PATCH] D45601: Warn on bool* to bool conversion

rjmccall added a comment. I agree that this is one of those warnings that just can't be done in any reasonable way. https://reviews.llvm.org/D45601 ___ cfe-commits mailing list cfe-commits@lists.llvm.org

[PATCH] D45601: Warn on bool* to bool conversion

hiraditya added a comment. The bug which motivated this warning is: https://github.com/jemalloc/jemalloc/commit/4df483f0fd76a64e116b1c4f316f8b941078114d#diff-7b26b977303fe92c093a2245b0eaf255 https://reviews.llvm.org/D45601 ___ cfe-commits mailing

[PATCH] D45601: Warn on bool* to bool conversion

aaron.ballman added inline comments. Comment at: lib/Sema/SemaChecking.cpp:2623 +QualType QT = Pointer->getType()->getPointeeType(); +if (!QT.isNull() && QT->isBooleanType()) + // Warn on bool* to bool conversion.

[PATCH] D45601: Warn on bool* to bool conversion

Quuxplusone added inline comments. Comment at: lib/Sema/SemaChecking.cpp:2623 +QualType QT = Pointer->getType()->getPointeeType(); +if (!QT.isNull() && QT->isBooleanType()) + // Warn on bool* to bool conversion.

[PATCH] D45601: Warn on bool* to bool conversion

efriedma added a comment. The thing about the bool*-only version is that bool pointers are rare in C++, so I'm not sure we're gaining much. But if we can't do something more general, there's still some benefit. I see your point about false positives for the more general version. I was sort

[PATCH] D45601: Warn on bool* to bool conversion

Quuxplusone added inline comments. Comment at: lib/Sema/SemaChecking.cpp:2623 +QualType QT = Pointer->getType()->getPointeeType(); +if (!QT.isNull() && QT->isBooleanType()) + // Warn on bool* to bool conversion.

[PATCH] D45601: Warn on bool* to bool conversion

efriedma added inline comments. Comment at: lib/Sema/SemaChecking.cpp:2623 +QualType QT = Pointer->getType()->getPointeeType(); +if (!QT.isNull() && QT->isBooleanType()) + // Warn on bool* to bool conversion. Have

[PATCH] D45601: Warn on bool* to bool conversion

hiraditya updated this revision to Diff 142776. hiraditya added a comment. Warn on bool* to bool conversion during a call only. https://reviews.llvm.org/D45601 Files: include/clang/Basic/DiagnosticSemaKinds.td lib/Sema/SemaChecking.cpp test/SemaCXX/warn-bool-ptr-to-bool.cpp Index:

[PATCH] D45601: Warn on bool* to bool conversion

lebedev.ri added inline comments. Comment at: include/clang/Basic/DiagnosticSemaKinds.td:7806 + "comparing %0 as a boolean">, + InGroup; def warn_format_argument_needs_cast : Warning< thakis wrote: > lebedev.ri wrote: > > Also, this really really should be

[PATCH] D45601: Warn on bool* to bool conversion

thakis added inline comments. Comment at: include/clang/Basic/DiagnosticSemaKinds.td:7806 + "comparing %0 as a boolean">, + InGroup; def warn_format_argument_needs_cast : Warning< lebedev.ri wrote: > Also, this really really should be under it's own flag,

[PATCH] D45601: Warn on bool* to bool conversion

gparker42 added a comment. Note that we recently relaxed a similar diagnostic for `NSNumber *` in the static analyzer. Such code is semantically similar to `inttype *`. https://reviews.llvm.org/D44044 https://reviews.llvm.org/D45601 ___

[PATCH] D45601: Warn on bool* to bool conversion

sberg added a comment. A random data point from trying this patch on the LibreOffice code base: - a little over 100 cases that are easily identified as false positives (many of the form "if (p) *p = ...") - two or three cases that looked suspicious on first glance but turned out to be false

[PATCH] D45601: Warn on bool* to bool conversion

hiraditya added a comment. I'll probably make this warning for function arguments only (when bool* is passed to a function accepting bool). Many conditionals use bool* -> bool conversion as pointed out by @Quuxplusone and @aaron.ballman https://reviews.llvm.org/D45601

[PATCH] D45601: Warn on bool* to bool conversion

lebedev.ri added a comment. In https://reviews.llvm.org/D45601#1067057, @aaron.ballman wrote: > ... > This makes me wary of making this a compiler diagnostic, but clang-tidy may > still be a reasonable place for this functionality to live. In https://reviews.llvm.org/D45601#1066572,

[PATCH] D45601: Warn on bool* to bool conversion

aaron.ballman added a reviewer: aaron.ballman. aaron.ballman added a comment. I'd also be interested to see the number of false positives and true positives when run over some large code bases (both C and C++). For instance, I would imagine code like this to be somewhat common and very

[PATCH] D45601: Warn on bool* to bool conversion

lebedev.ri added inline comments. Comment at: include/clang/Basic/DiagnosticSemaKinds.td:7806 + "comparing %0 as a boolean">, + InGroup; def warn_format_argument_needs_cast : Warning< Also, this really really should be under it's own flag, which in turn may

[PATCH] D45601: Warn on bool* to bool conversion

lebedev.ri added a comment. @hiraditya I personally don't like when i'm being told so, but i'd like to see some numbers... **Please** run this on //some// big C++ project (LLVM (but you'll have to enable this diag specifically), google chrome, ???), and analyse the results. In

[PATCH] D45601: Warn on bool* to bool conversion

Quuxplusone added inline comments. Comment at: test/CXX/expr/expr.unary/expr.unary.op/p6.cpp:18 // -- pointer, bool b6 = ! // expected-warning{{address of 'b4' will always evaluate to 'true'}} +// expected-warning@-1 {{comparing 'bool *' as a boolean}} This

[PATCH] D45601: Warn on bool* to bool conversion

Eugene.Zelenko added a comment. There is Clang-tidy's readability-implicit-bool-conversion check. It may be reasonable to check pointers to integers too, since integers are often implicitly converted to

[PATCH] D45601: Warn on bool* to bool conversion

hiraditya created this revision. hiraditya added reviewers: Eugene.Zelenko, rsmith. We found conversion from bool* to bool to be a common source of bug, so we have implemented this warning. Sample use case: int bar(bool b) { return b; } int baz() { bool *b; bar(b);