[PATCH] D63082: [Diagnostics] Added support for -Wint-in-bool-context

xbolva00 added a comment. Happy to hear :) CHANGES SINCE LAST ACTION https://reviews.llvm.org/D63082/new/ https://reviews.llvm.org/D63082 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-

[PATCH] D63082: [Diagnostics] Added support for -Wint-in-bool-context

thakis added a comment. It looks like bits of this landed recently. I don't know which of the commits it was, so I'm commenting here: The -Wint-in-bool context that clang emits are very useful and have found several bugs in Chromium, with just one false positive (which was in confusing code, so

[PATCH] D63082: [Diagnostics] Added support for -Wint-in-bool-context

xbolva00 added a comment. I will split enum constants handling to other patch in near future. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D63082/new/ https://reviews.llvm.org/D63082 ___ cfe-commits mailing list cfe-commits@lists.llvm.org h

[PATCH] D63082: [Diagnostics] Added support for -Wint-in-bool-context

xbolva00 added a comment. C FE: -IfStmt 0x55a225bc4dc8 | |-BinaryOperator 0x55a225bc4d48 'int' '||' | | |-BinaryOperator 0x55a225bc4d08 'int' '==' | | | |-ImplicitCastExpr 0x55a225bc4cf0 'int' | | | | `-ImplicitCastExpr 0x55a225bc4cd8 'enum fruit':'enum fruit' | | | | `-DeclR

[PATCH] D63082: [Diagnostics] Added support for -Wint-in-bool-context

xbolva00 marked an inline comment as done. xbolva00 added inline comments. Comment at: test/SemaCXX/warn-int-in-bool-context.cpp:96 + + if (f == apple || orange) // expected-warning {{enum constant in boolean context}} +return a; aaron.ballman wrote: > xbol

[PATCH] D63082: [Diagnostics] Added support for -Wint-in-bool-context

aaron.ballman added inline comments. Comment at: test/Sema/warn-int-in-bool-context.c:26 + r = a << 7; // expected-warning {{'<<' in boolean context; did you mean '<'?}} + r = ONE << b; // expected-warning {{'<<' in boolean context; did you mean '<'?}} + jf

[PATCH] D63082: [Diagnostics] Added support for -Wint-in-bool-context

xbolva00 marked 2 inline comments as done. xbolva00 added inline comments. Comment at: include/clang/Basic/DiagnosticSemaKinds.td:5609 +def warn_mul_in_bool_context : Warning< + "'*' in bool context, maybe you mean '&&'?">, + InGroup; jfb wrote: > xbolva00 wrot

[PATCH] D63082: [Diagnostics] Added support for -Wint-in-bool-context

jfb added inline comments. Comment at: include/clang/Basic/DiagnosticSemaKinds.td:5609 +def warn_mul_in_bool_context : Warning< + "'*' in bool context, maybe you mean '&&'?">, + InGroup; xbolva00 wrote: > jfb wrote: > > aaron.ballman wrote: > > > xbolva00 wrote

[PATCH] D63082: [Diagnostics] Added support for -Wint-in-bool-context

xbolva00 marked 2 inline comments as done and an inline comment as not done. xbolva00 added inline comments. Comment at: include/clang/Basic/DiagnosticSemaKinds.td:5609 +def warn_mul_in_bool_context : Warning< + "'*' in bool context, maybe you mean '&&'?">, + InGroup; -

[PATCH] D63082: [Diagnostics] Added support for -Wint-in-bool-context

jfb added inline comments. Comment at: test/Sema/warn-int-in-bool-context.c:26 + r = a << 7; // expected-warning {{'<<' in boolean context; did you mean '<'?}} + r = ONE << b; // expected-warning {{'<<' in boolean context; did you mean '<'?}} + xbolva00 wro

[PATCH] D63082: [Diagnostics] Added support for -Wint-in-bool-context

xbolva00 marked 4 inline comments as done and an inline comment as not done. xbolva00 added inline comments. Comment at: test/Sema/warn-int-in-bool-context.c:26 + r = a << 7; // expected-warning {{'<<' in boolean context; did you mean '<'?}} + r = ONE << b; // expected-warni

[PATCH] D63082: [Diagnostics] Added support for -Wint-in-bool-context

jfb added inline comments. Comment at: test/Sema/warn-int-in-bool-context.c:26 + r = a << 7; // expected-warning {{'<<' in boolean context; did you mean '<'?}} + r = ONE << b; // expected-warning {{'<<' in boolean context; did you mean '<'?}} + xbolva00 wro

[PATCH] D63082: [Diagnostics] Added support for -Wint-in-bool-context

xbolva00 marked an inline comment as done. xbolva00 added inline comments. Comment at: test/Sema/warn-int-in-bool-context.c:32 + r = a ? -2 : 0; + r = a ? 3 : -2; + r = a ? 0 : TWO; // expected-warning {{'?:' with integer constants in boolean context}} jfb wr

[PATCH] D63082: [Diagnostics] Added support for -Wint-in-bool-context

xbolva00 marked 3 inline comments as done. xbolva00 added inline comments. Comment at: test/Sema/warn-int-in-bool-context.c:26 + r = a << 7; // expected-warning {{'<<' in boolean context; did you mean '<'?}} + r = ONE << b; // expected-warning {{'<<' in boolean context; did

[PATCH] D63082: [Diagnostics] Added support for -Wint-in-bool-context

jfb added inline comments. Comment at: include/clang/Basic/DiagnosticSemaKinds.td:5609 +def warn_mul_in_bool_context : Warning< + "'*' in bool context, maybe you mean '&&'?">, + InGroup; aaron.ballman wrote: > xbolva00 wrote: > > aaron.ballman wrote: > > > I wo

[PATCH] D63082: [Diagnostics] Added support for -Wint-in-bool-context

xbolva00 marked an inline comment as done. xbolva00 added inline comments. Comment at: test/SemaCXX/warn-int-in-bool-context.cpp:96 + + if (f == apple || orange) // expected-warning {{enum constant in boolean context}} +return a; @aaron.ballman : In C (but

[PATCH] D63082: [Diagnostics] Added support for -Wint-in-bool-context

xbolva00 marked an inline comment as done. xbolva00 added inline comments. Comment at: test/SemaCXX/warn-int-in-bool-context.cpp:99 + + if (ans == yes || no) // expected-warning {{enum constant in boolean context}} +return a; aaron.ballman wrote: > Why do w

[PATCH] D63082: [Diagnostics] Added support for -Wint-in-bool-context

aaron.ballman added inline comments. Comment at: test/SemaCXX/warn-int-in-bool-context.cpp:99 + + if (ans == yes || no) // expected-warning {{enum constant in boolean context}} +return a; Why do we want to diagnose this case in C++ but not in C? CHANGES S

[PATCH] D63082: [Diagnostics] Added support for -Wint-in-bool-context

xbolva00 added a comment. Ping. Seems like @rsmith is busy, so I added @jfb as a reviewer. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D63082/new/ https://reviews.llvm.org/D63082 ___ cfe-commits mailing list cfe-commits@lists.llvm.org htt

[PATCH] D63082: [Diagnostics] Added support for -Wint-in-bool-context

xbolva00 added a comment. Ping @rsmith CHANGES SINCE LAST ACTION https://reviews.llvm.org/D63082/new/ https://reviews.llvm.org/D63082 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-comm

[PATCH] D63082: [Diagnostics] Added support for -Wint-in-bool-context

xbolva00 added a comment. It would be nice to land this before 9.0 branch date... CHANGES SINCE LAST ACTION https://reviews.llvm.org/D63082/new/ https://reviews.llvm.org/D63082 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists

[PATCH] D63082: [Diagnostics] Added support for -Wint-in-bool-context

xbolva00 added a comment. Ping CHANGES SINCE LAST ACTION https://reviews.llvm.org/D63082/new/ https://reviews.llvm.org/D63082 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D63082: [Diagnostics] Added support for -Wint-in-bool-context

xbolva00 added a comment. In D63082#1564114 , @aaron.ballman wrote: > In D63082#1560667 , @xbolva00 wrote: > > > I wanted to follow GCC’s behaviour -Wall but then dicussion moved to > > “tautological compare group

[PATCH] D63082: [Diagnostics] Added support for -Wint-in-bool-context

xbolva00 updated this revision to Diff 207623. xbolva00 added a comment. Implemented suggested "mul in bool context" diagnostic. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D63082/new/ https://reviews.llvm.org/D63082 Files: include/clang/Basic/DiagnosticGroups.td include/clang/Bas

[PATCH] D63082: [Diagnostics] Added support for -Wint-in-bool-context

aaron.ballman added a comment. In D63082#1560667 , @xbolva00 wrote: > I wanted to follow GCC’s behaviour -Wall but then dicussion moved to > “tautological compare group” - I was fine with that too.. > I can again revert it to -Wall... > If this should b

[PATCH] D63082: [Diagnostics] Added support for -Wint-in-bool-context

xbolva00 updated this revision to Diff 206842. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D63082/new/ https://reviews.llvm.org/D63082 Files: include/clang/Basic/DiagnosticGroups.td include/clang/Basic/DiagnosticSemaKinds.td lib/Sema/SemaChecking.cpp test/Analysis/misc-ps.c te

[PATCH] D63082: [Diagnostics] Added support for -Wint-in-bool-context

xbolva00 added a comment. Alternative “* in boolean context; did you mean to compare it?” CHANGES SINCE LAST ACTION https://reviews.llvm.org/D63082/new/ https://reviews.llvm.org/D63082 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https

[PATCH] D63082: [Diagnostics] Added support for -Wint-in-bool-context

xbolva00 added a comment. “Perhaps appending something about the resulting value always being true|false would help most of these diagnostics be more obvious” I dont know, this could only diagnose constant multiplications (maybe this is already handled by some “always true” check). Anyway, I ha

[PATCH] D63082: [Diagnostics] Added support for -Wint-in-bool-context

xbolva00 added a comment. I wanted to follow GCC’s behaviour -Wall but then dicussion moved to “tautological compare group” - I was fine with that too.. I can again revert it to -Wall... If this should be off by default even on -Wall, then all work was useless.. CHANGES SINCE LAST ACTION http

[PATCH] D63082: [Diagnostics] Added support for -Wint-in-bool-context

aaron.ballman added a comment. In terms of the code, I think this is ready to go. However, I'm still not happy with `'*' in boolean context` as a diagnostic. Perhaps appending something about the resulting value always being true|false would help most of these diagnostics be more obvious. `blah

[PATCH] D63082: [Diagnostics] Added support for -Wint-in-bool-context

xbolva00 added a comment. ping CHANGES SINCE LAST ACTION https://reviews.llvm.org/D63082/new/ https://reviews.llvm.org/D63082 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D63082: [Diagnostics] Added support for -Wint-in-bool-context

xbolva00 updated this revision to Diff 206075. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D63082/new/ https://reviews.llvm.org/D63082 Files: include/clang/Basic/DiagnosticGroups.td include/clang/Basic/DiagnosticSemaKinds.td lib/Sema/SemaChecking.cpp test/Analysis/misc-ps.c te

[PATCH] D63082: [Diagnostics] Added support for -Wint-in-bool-context

xbolva00 marked an inline comment as done. xbolva00 added inline comments. Comment at: include/clang/Basic/DiagnosticSemaKinds.td:5625 + "evaluate to 'true'">, + InGroup; + xbolva00 wrote: > aaron.ballman wrote: > > xbolva00 wrote: > > > aaron.ballman wrote: >

[PATCH] D63082: [Diagnostics] Added support for -Wint-in-bool-context

xbolva00 marked an inline comment as done. xbolva00 added inline comments. Comment at: include/clang/Basic/DiagnosticSemaKinds.td:5625 + "evaluate to 'true'">, + InGroup; + aaron.ballman wrote: > xbolva00 wrote: > > aaron.ballman wrote: > > > xbolva00 wrote: >

[PATCH] D63082: [Diagnostics] Added support for -Wint-in-bool-context

aaron.ballman added inline comments. Comment at: include/clang/Basic/DiagnosticSemaKinds.td:5625 + "evaluate to 'true'">, + InGroup; + xbolva00 wrote: > aaron.ballman wrote: > > xbolva00 wrote: > > > aaron.ballman wrote: > > > > This one seems like it should be

[PATCH] D63082: [Diagnostics] Added support for -Wint-in-bool-context

xbolva00 marked an inline comment as done. xbolva00 added inline comments. Comment at: include/clang/Basic/DiagnosticSemaKinds.td:5625 + "evaluate to 'true'">, + InGroup; + aaron.ballman wrote: > xbolva00 wrote: > > aaron.ballman wrote: > > > This one seems lik

[PATCH] D63082: [Diagnostics] Added support for -Wint-in-bool-context

aaron.ballman added inline comments. Comment at: include/clang/Basic/DiagnosticSemaKinds.td:5609 +def warn_mul_in_bool_context : Warning< + "'*' in bool context, maybe you mean '&&'?">, + InGroup; xbolva00 wrote: > aaron.ballman wrote: > > I would appreciate se

[PATCH] D63082: [Diagnostics] Added support for -Wint-in-bool-context

xbolva00 updated this revision to Diff 205696. xbolva00 added a comment. Addressed review notes. Thanks! CHANGES SINCE LAST ACTION https://reviews.llvm.org/D63082/new/ https://reviews.llvm.org/D63082 Files: include/clang/Basic/DiagnosticGroups.td include/clang/Basic/DiagnosticSemaKinds.t

[PATCH] D63082: [Diagnostics] Added support for -Wint-in-bool-context

ArnaudBienner added inline comments. Comment at: lib/Sema/SemaChecking.cpp:11103 + + if (auto *BO = dyn_cast(E)) { +BinaryOperator::Opcode Opc = BO->getOpcode(); Shouldn't that be "const auto*" instead? I'm surprised dyn_cast casts away const qualifier, but

[PATCH] D63082: [Diagnostics] Added support for -Wint-in-bool-context

xbolva00 marked an inline comment as done. xbolva00 added inline comments. Comment at: include/clang/Basic/DiagnosticSemaKinds.td:5612 +def warn_left_shift_in_bool_context : Warning< + "'<<' in boolean context, maybe you mean '<'?">, + InGroup; xbolva00 wrote:

[PATCH] D63082: [Diagnostics] Added support for -Wint-in-bool-context

xbolva00 marked an inline comment as done. xbolva00 added inline comments. Comment at: include/clang/Basic/DiagnosticSemaKinds.td:5612 +def warn_left_shift_in_bool_context : Warning< + "'<<' in boolean context, maybe you mean '<'?">, + InGroup; I will this line

[PATCH] D63082: [Diagnostics] Added support for -Wint-in-bool-context

xbolva00 added a comment. Is it ok now? CHANGES SINCE LAST ACTION https://reviews.llvm.org/D63082/new/ https://reviews.llvm.org/D63082 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-com

[PATCH] D63082: [Diagnostics] Added support for -Wint-in-bool-context

xbolva00 updated this revision to Diff 205365. xbolva00 added a comment. Updated forgotten test file. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D63082/new/ https://reviews.llvm.org/D63082 Files: include/clang/Basic/DiagnosticGroups.td include/clang/Basic/DiagnosticSemaKinds.td

[PATCH] D63082: [Diagnostics] Added support for -Wint-in-bool-context

xbolva00 updated this revision to Diff 205364. xbolva00 added a comment. Addressed review notes. Thanks @aaron.ballman ! CHANGES SINCE LAST ACTION https://reviews.llvm.org/D63082/new/ https://reviews.llvm.org/D63082 Files: include/clang/Basic/DiagnosticGroups.td include/clang/Basic/Diagn

[PATCH] D63082: [Diagnostics] Added support for -Wint-in-bool-context

xbolva00 marked 3 inline comments as done. xbolva00 added inline comments. Comment at: include/clang/Basic/DiagnosticSemaKinds.td:5609 +def warn_mul_in_bool_context : Warning< + "'*' in bool context, maybe you mean '&&'?">, + InGroup; aaron.ballman wrote: > I w

[PATCH] D63082: [Diagnostics] Added support for -Wint-in-bool-context

aaron.ballman added inline comments. Comment at: include/clang/Basic/DiagnosticSemaKinds.td:5609 +def warn_mul_in_bool_context : Warning< + "'*' in bool context, maybe you mean '&&'?">, + InGroup; I would appreciate seeing some motivating examples for this case

[PATCH] D63082: [Diagnostics] Added support for -Wint-in-bool-context

xbolva00 added a comment. Ping CHANGES SINCE LAST ACTION https://reviews.llvm.org/D63082/new/ https://reviews.llvm.org/D63082 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D63082: [Diagnostics] Added support for -Wint-in-bool-context

xbolva00 updated this revision to Diff 203839. xbolva00 added a comment. Removed unrelated change CHANGES SINCE LAST ACTION https://reviews.llvm.org/D63082/new/ https://reviews.llvm.org/D63082 Files: include/clang/Basic/DiagnosticGroups.td include/clang/Basic/DiagnosticSemaKinds.td lib

[PATCH] D63082: [Diagnostics] Added support for -Wint-in-bool-context

xbolva00 added a subscriber: RKSimon. xbolva00 added a comment. Adding @RKSimon as a reviewer since he recently fixed some bugs which GCC's -Wint-in-bool-context caught. Repository: rC Clang CHANGES SINCE LAST ACTION https://reviews.llvm.org/D63082/new/ https://reviews.llvm.org/D63082

[PATCH] D63082: [Diagnostics] Added support for -Wint-in-bool-context

xbolva00 created this revision. xbolva00 added reviewers: rsmith, rnk. Herald added a project: clang. Herald added a subscriber: cfe-commits. I consider -Wint-in-bool-context as quite useful warning so I ported it to Clang. -Wint-in-bool-context is on by default, it is part of GCC's -Wall so I m