[Diagnostics] Added support for -Wint-in-bool-context

[xbolva00]

Summary

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 make it same here.

Current patch passes all tests for -Wint-in-bool-context in GCC's test-suite.

Fixes PR31201

[xbolva00]

Adding @RKSimon as a reviewer since he recently fixed some bugs which GCC's -Wint-in-bool-context caught.

[xbolva00]

Removed unrelated change

[xbolva00]

Ping

[aaron.ballman]

include/clang/Basic/DiagnosticSemaKinds.td

5622

I would appreciate seeing some motivating examples for this case, because it seems like the && suggestion is misleading more often than it's helpful in the current test cases.

Also, what is a "bool context"?

5638

This one seems like it should be in the TautologicalConstantCompare group, no?

lib/Sema/SemaChecking.cpp

11119

const Expr *E?

test/Sema/integer-overflow.c

37 ↗

(On Diff #203839)

I don't think the new diagnostic adds a lot of value here, though GCC does warn on it similarly. Truthfully, the same goes for the other test cases in this file.

[xbolva00]

include/clang/Basic/DiagnosticSemaKinds.td

5622

Bool context - places where we expect boolean expressions.

If you have idea for better terminology, I am happy to apply it.

Yeah, I will remove “&&” suggestion..

5624

Oh, inconsistency. I will use boolean context than bool context :) (if no suggestions)

5638

Yes, we could move it there.

[xbolva00]

Addressed review notes. Thanks @aaron.ballman !

[xbolva00]

Updated forgotten test file.

[xbolva00]

Is it ok now?

[xbolva00]

include/clang/Basic/DiagnosticSemaKinds.td

5625

I will this line.

[xbolva00]

include/clang/Basic/DiagnosticSemaKinds.td

5625

• Ehh.. I will fix this line to use "did you mean".

[ArnaudBienner]

lib/Sema/SemaChecking.cpp

11123

Shouldn't that be "const auto*" instead?
I'm surprised dyn_cast casts away const qualifier, but FWIW having the returned pointer being const makes clearer the variable pointed by this pointer won't be modified.
But maybe this was intended to not make the code too verbose?

[xbolva00]

Addressed review notes. Thanks!

[aaron.ballman]

include/clang/Basic/DiagnosticSemaKinds.td

5622

I don't have a good recommendation for a replacement for "boolean context", so I suppose we can leave it.

5638

Don't *all* of these effectively boil down to a tautological comparison?

[xbolva00]

include/clang/Basic/DiagnosticSemaKinds.td

5638

So you propose put IntInBoolContext into TautologicalConstantCompare ?

[aaron.ballman]

include/clang/Basic/DiagnosticSemaKinds.td

5638

That's what I am thinking, but I was curious if you agreed.

[xbolva00]

include/clang/Basic/DiagnosticSemaKinds.td

5638

But that is not enabled by -Wall :/ I would like to have this under -Wall.

[xbolva00]

include/clang/Basic/DiagnosticSemaKinds.td

5638

I got it. Yes, we can "warn" even if no -Wall.

[xbolva00]

ping

[aaron.ballman]

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 in boolean context will always evaluate to true|false optionally followed by ; did you mean yada? would make it more obvious what is wrong with the code.

Also, I'm not certain why this should be enabled by default when the other tautological comparison categories are not. I think it should follow the rest of the tautological comparisons in that regard, but I also would have expected those to be on by default. @rsmith, what are your opinions here?

include/clang/Basic/DiagnosticGroups.td

767

Spurious change.

[xbolva00]

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

[xbolva00]

“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 have no motivation to diagnose constant muls, sorry. My goal is to make this warning atleast as good as GCC’s implementation.

Okay, we could make it better and append “; did you mean “index * 3 != 0”?

[xbolva00]

Alternative “* in boolean context; did you mean to compare it?”

[aaron.ballman]

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 be off by default even on -Wall, then all work was useless..

Personally, my feeling is that this new diagnostic group belongs under the tautological compare group. That group is currently off by default, but I'm wondering if we want it to be on by default (in a subsequent patch), or whether we're okay having parts of it on by default and parts off by default. I'm hoping @rsmith will voice an opinion here.

In D63082#1560684, @xbolva00 wrote:

“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 have no motivation to diagnose constant muls, sorry. My goal is to make this warning atleast as good as GCC’s implementation.

Okay, we could make it better and append “; did you mean “index * 3 != 0”?

I don't think that actually improves anything. I do not think we want a "did you mean" in this case because I'm not confident anyone can guess what a user was trying for in this situation. However, telling the user the result of the computation does give them very useful information -- it tells them the result will always be tautologically true or false (which is what the other tautological warnings do, as well). As it stands, the current diagnostic wording doesn't help the user understand what's wrong with their code. I'd like to correct that deficiency before we commit this.

[xbolva00]

Implemented suggested "mul in bool context" diagnostic.

[xbolva00]

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

Personally, my feeling is that this new diagnostic group belongs under the tautological compare group. That group is currently off by default, but I'm wondering if we want it to be on by default (in a subsequent patch), or whether we're okay having parts of it on by default and parts off by default. I'm hoping @rsmith will voice an opinion here.

In D63082#1560684, @xbolva00 wrote:

“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 have no motivation to diagnose constant muls, sorry. My goal is to make this warning atleast as good as GCC’s implementation.

Okay, we could make it better and append “; did you mean “index * 3 != 0”?

I don't think that actually improves anything. I do not think we want a "did you mean" in this case because I'm not confident anyone can guess what a user was trying for in this situation. However, telling the user the result of the computation does give them very useful information -- it tells them the result will always be tautologically true or false (which is what the other tautological warnings do, as well). As it stands, the current diagnostic wording doesn't help the user understand what's wrong with their code. I'd like to correct that deficiency before we commit this.

Thanks

I decided to implement your suggestions.

[xbolva00]

Ping

[xbolva00]

This comment was removed by xbolva00.

[xbolva00]

Ping @rsmith

[xbolva00]

Ping.

Seems like @rsmith is busy, so I added @jfb as a reviewer.

[aaron.ballman]

test/SemaCXX/warn-int-in-bool-context.cpp

99

Why do we want to diagnose this case in C++ but not in C?

[xbolva00]

test/SemaCXX/warn-int-in-bool-context.cpp

99

I think we can and should but for unknown reason (for me) this is GCC’s behaviour.

Maybe it is too noisy for C codebases?

Maybe introduce -Wenum-in-bool-context as subgroup of -Wint-in-bool-context? And enable it for C and C++?

[aaron.ballman]

test/SemaCXX/warn-int-in-bool-context.cpp

99

I think it's just a bug in GCC (they use separate frontends for C and C++, so these sort of differences crop up sometimes). I think this should be safe to diagnose the same in C and C++.

[xbolva00]

test/SemaCXX/warn-int-in-bool-context.cpp

96

@aaron.ballman : In C (but not in C++ ugh?) mode we have: 'use of logical '||' with constant operand' here..

/home/xbolva00/LLVM/llvm/tools/clang/test/SemaCXX/warn-int-in-bool-context.cpp:106:18: warning: use of logical '||' with constant operand

if (f == apple || orange) // expected-warning {{enum constant in boolean context}}
               ^  ~~~~~~

/home/xbolva00/LLVM/llvm/tools/clang/test/SemaCXX/warn-int-in-bool-context.cpp:106:18: note: use '|' for a bitwise operation

if (f == apple || orange) // expected-warning {{enum constant in boolean context}}
               ^~
               |

/home/xbolva00/LLVM/llvm/tools/clang/test/SemaCXX/warn-int-in-bool-context.cpp:106:21: warning: enum constant in boolean context

if (f == apple || orange) // expected-warning {{enum constant in boolean context}}

[jfb]

include/clang/Basic/DiagnosticSemaKinds.td

5622

IMO it's clearer to say "assigning the result of a multiplication to a boolean variable always yields {true|false}"

5625

Same here, "assigning the result of a left shift to a boolean variable always yields {true|false}"

Same for all the other ones...

test/Sema/warn-int-in-bool-context.c

26

I'm not sure the "did you mean" part is helpful. Do we have data showing that this is what people actually mean?

32

Why wouldn't think one warn?

33

Why does this one warn? It doesn't always yield the same result.

test/Sema/warn-unreachable.c

147

It seems like here the "will never be executed" warning is more useful. Do we want to emit both?

test/SemaCXX/warn-int-in-bool-context.cpp

99

Agreed.

[xbolva00]

test/Sema/warn-int-in-bool-context.c

26

No, no data, just what GCC suggests.

33

GCC warns here..

test/Sema/warn-unreachable.c

147

Useful but -Wunreachable-code is disabled by default (not part of -Wall).

[xbolva00]

test/Sema/warn-int-in-bool-context.c

32

Ah, right. Thanks !

I forgot there is an unary minus operator..

Will fix it.

[jfb]

test/Sema/warn-int-in-bool-context.c

26

I would drop it without knowing that it's actually what people usually mean.

33

Our goal isn't to achieve GCC warning parity. I like warnings that fire when it's extremely likely a bug. Here the diagnostic seems to be inconsistent with other instances of this diagnostic. Maybe this should warn, but then other cases should as well (such as a?3:-2 above). Or maybe it shouldn't warn at all.

test/Sema/warn-unreachable.c

147

I don't think we want two diagnostics when one will do.

[xbolva00]

test/Sema/warn-int-in-bool-context.c

26

Leave just “<<' in boolean context“ seems not ideal for me as a diag message.

Or is it ok for you?

33

I will drop it and leave only “warn_integer_constants_in_conditional_always_true” diag.

test/Sema/warn-unreachable.c

147

I will drop mul handling then..

test/SemaCXX/warn-int-in-bool-context.cpp

96

What is your opinion here (^^) , @jfb ?

Thanks.

[jfb]

test/Sema/warn-int-in-bool-context.c

26

As I said above, I don't think you should say "boolean context". It's not something people will understand. Here we're assigning to a boolean, say that.

test/Sema/warn-unreachable.c

147

No, my feedback isn't about multiplication in particular. My feedback is that it's not useful to have both warnings. We should just emit a single warning, and it should be maximally helpful. Here it's useful to say that calledFunc() will never be executed because 0*x is always false.

test/SemaCXX/warn-int-in-bool-context.cpp

96

C, C++, ObjC, and ObjC++ should emit different diagnostics here. That being said, none of these diagnostics are particular easy to understand. I don't think we should ever say "boolean context" anywhere.

[xbolva00]

include/clang/Basic/DiagnosticSemaKinds.td

5622

But it is very specific.. cannot be used for example here:

bool t() { return 0 * x; }

In the place where the check is performed we know nothing whether expr is assigned/returned/etc.

I don’t believe we have to produce message with such details.

test/SemaCXX/warn-int-in-bool-context.cpp

96

So I will make this as it was - C++ only.

[jfb]

include/clang/Basic/DiagnosticSemaKinds.td

5622

All I'm saying is that "boolean context" ins't something people will understand.
Maybe "converting the result of a multiplication to a boolean always yields {true|false}"?

test/SemaCXX/warn-int-in-bool-context.cpp

96

It'll work in the other languages though, right?

[xbolva00]

include/clang/Basic/DiagnosticSemaKinds.td

5622

+1, like it

test/SemaCXX/warn-int-in-bool-context.cpp

96

Sight, making it C++ only leaves C with not diagnosed case:

_Bool f() { return orange; }

And I don’t know why ‘use of logical || with constant operand’ is C only :/..

[aaron.ballman]

test/Sema/warn-int-in-bool-context.c

26

As I said above, I don't think you should say "boolean context". It's not something people will understand. Here we're assigning to a boolean, say that.

Strong +1.

As for the "did you mean", I actually see that as somewhat reasonable text. << is a reasonable typo for < (which yields a bool), similar for >> and >. However, if we had data to back up that presumption, it would be better.

test/SemaCXX/warn-int-in-bool-context.cpp

96

And I don’t know why ‘use of logical || with constant operand’ is C only :/..

It may be worth doing the svn blame dance to see if it's discussed as part of the original review. One possibility is that || can be overloaded in C++ and so perhaps || with a constant operand is not a bug. But that seems like pretty weak rationale (we can always see if there's an overloaded || and not issue the diagnostic in that circumstance).

[xbolva00]

test/SemaCXX/warn-int-in-bool-context.cpp

96

But even if we fix ‘use of logical .. with constants” for C++, we the break this patch (clang will produce two similar warnings) and if we remove it from this patch, some cases stay undiagnosed:

bool foo() return orange;

In order to support this case, we cant use this place to implement the check.

Not good..

[xbolva00]

C FE:

-IfStmt 0x55a225bc4dc8 <line:39:3, line:40:12>

| |-BinaryOperator 0x55a225bc4d48 <line:39:7, col:21> 'int' '||'
| | |-BinaryOperator 0x55a225bc4d08 <col:7, col:12> 'int' '=='
| | | |-ImplicitCastExpr 0x55a225bc4cf0 <col:7> 'int' <IntegralCast>
| | | | `-ImplicitCastExpr 0x55a225bc4cd8 <col:7> 'enum fruit':'enum fruit' <LValueToRValue>
| | | |   `-DeclRefExpr 0x55a225bc4c98 <col:7> 'enum fruit':'enum fruit' lvalue ParmVar 0x55a225bc4a90 'f' 'enum fruit':'enum fruit'
| | | `-DeclRefExpr 0x55a225bc4cb8 <col:12> 'int' EnumConstant 0x55a225bc4610 'apple' 'int'
| | `-DeclRefExpr 0x55a225bc4d28 <col:21> 'int' EnumConstant 0x55a225bc4520 'orange' 'int'

C++ FE:
IfStmt 0x55cfd2ae5d20 <line:39:3, line:40:12>

| |-BinaryOperator 0x55cfd2ae5ca0 <line:39:7, col:21> 'bool' '||'
| | |-BinaryOperator 0x55cfd2ab9940 <col:7, col:12> 'bool' '=='
| | | |-ImplicitCastExpr 0x55cfd2ab9910 <col:7> 'int' <IntegralCast>
| | | | `-ImplicitCastExpr 0x55cfd2ab98f8 <col:7> 'enum fruit':'fruit' <LValueToRValue>
| | | |   `-DeclRefExpr 0x55cfd2ab98b8 <col:7> 'enum fruit':'fruit' lvalue ParmVar 0x55cfd2ab96b0 'f' 'enum fruit':'fruit'
| | | `-ImplicitCastExpr 0x55cfd2ab9928 <col:12> 'int' <IntegralCast>
| | |   `-DeclRefExpr 0x55cfd2ab98d8 <col:12> 'fruit' EnumConstant 0x55cfd2ab8ef8 'apple' 'fruit'
| | `-ImplicitCastExpr 0x55cfd2ab9980 <col:21> 'bool' <IntegralToBoolean>
| |   `-DeclRefExpr 0x55cfd2ab9960 <col:21> 'fruit' EnumConstant 0x55cfd2ab8e08 'orange' 'fruit'

https://github.com/llvm-mirror/clang/blob/9b127f3b44e685cbe513595b7e0115b0884b0604/lib/Sema/SemaExpr.cpp#L6701

[xbolva00]

I will split enum constants handling to other patch in near future.

[thakis]

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 we changed that one too). Thank you for this useful warning :)

(https://crbug.com/1007367 lists what this founds, if you're curious.)

[xbolva00]

Happy to hear :)

include/clang/Basic/DiagnosticGroups.td

 def StaticLocalInInline : DiagGroup<"static-local-in-inline">;
 def GNUStaticFloatInit : DiagGroup<"gnu-static-float-init">;
 def StaticFloatInit : DiagGroup<"static-float-init", [GNUStaticFloatInit]>;
 def GNUStatementExpression : DiagGroup<"gnu-statement-expression">;
 def StringCompare : DiagGroup<"string-compare">;
 def StringPlusInt : DiagGroup<"string-plus-int">;
 def StringPlusChar : DiagGroup<"string-plus-char">;
 def StrncatSize : DiagGroup<"strncat-size">;
+def IntInBoolContext : DiagGroup<"int-in-bool-context">;
 def TautologicalTypeLimitCompare : DiagGroup<"tautological-type-limit-compare">;
 def TautologicalUnsignedZeroCompare : DiagGroup<"tautological-unsigned-zero-compare">;
 def TautologicalUnsignedEnumZeroCompare : DiagGroup<"tautological-unsigned-enum-zero-compare">;
 def TautologicalInRangeCompare : DiagGroup<"tautological-constant-in-range-compare",
                                            [TautologicalTypeLimitCompare,
                                             TautologicalUnsignedZeroCompare,
                                             TautologicalUnsignedEnumZeroCompare]>;
 // For compatibility with GCC; -Wtype-limits = -Wtautological-constant-in-range-compare
 def TypeLimits : DiagGroup<"type-limits", [TautologicalInRangeCompare]>;
 def TautologicalOutOfRangeCompare : DiagGroup<"tautological-constant-out-of-range-compare">;
 def TautologicalConstantCompare : DiagGroup<"tautological-constant-compare",
                                             [TautologicalOutOfRangeCompare]>;
 def TautologicalPointerCompare : DiagGroup<"tautological-pointer-compare">;
 def TautologicalOverlapCompare : DiagGroup<"tautological-overlap-compare">;
 def TautologicalUndefinedCompare : DiagGroup<"tautological-undefined-compare">;
 def TautologicalCompare : DiagGroup<"tautological-compare",
                                     [TautologicalConstantCompare,
                                      TautologicalPointerCompare,
                                      TautologicalOverlapCompare,
-                                     TautologicalUndefinedCompare]>;
+                                     TautologicalUndefinedCompare,
+                                     IntInBoolContext]>;
 def HeaderHygiene : DiagGroup<"header-hygiene">;
 def DuplicateDeclSpecifier : DiagGroup<"duplicate-decl-specifier">;
 def CompareDistinctPointerType : DiagGroup<"compare-distinct-pointer-types">;
 def GNUUnionCast : DiagGroup<"gnu-union-cast">;
 def GNUVariableSizedTypeNotAtEnd : DiagGroup<"gnu-variable-sized-type-not-at-end">;
 def Varargs : DiagGroup<"varargs">;
 
 def Unsequenced : DiagGroup<"unsequenced">;
def Format : DiagGroup<"format",
              DiagCategory<"Format String Issue">;
 def FormatNonLiteral : DiagGroup<"format-nonliteral">;
 def Format2 : DiagGroup<"format=2",
                         [FormatNonLiteral, FormatSecurity, FormatY2K]>;
 
 def TypeSafety : DiagGroup<"type-safety">;
 
 def IncompatibleExceptionSpec : DiagGroup<"incompatible-exception-spec">;
 
 def IntToVoidPointerCast : DiagGroup<"int-to-void-pointer-cast">;
 def IntToPointerCast : DiagGroup<"int-to-pointer-cast",
                                  [IntToVoidPointerCast]>;
 
 def Move : DiagGroup<"move", [
     PessimizingMove,
     RedundantMove,
     ReturnStdMove,

include/clang/Basic/DiagnosticSemaKinds.td

def note_precedence_silence : Note<
   "place parentheses around the '%0' expression to silence this warning">;
 
 def warn_precedence_conditional : Warning<
   "operator '?:' has lower precedence than '%0'; '%0' will be evaluated first">,
   InGroup<Parentheses>;
 def note_precedence_conditional_first : Note<
   "place parentheses around the '?:' expression to evaluate it first">;
 
+def warn_mul_in_bool_context : Warning<
+  "'*' in boolean context, the expression will always "
+  "evaluate to '%select{true|false}0'">,
+  InGroup<IntInBoolContext>;
+def warn_left_shift_in_bool_context : Warning<
+  "'<<' in boolean context; did you mean '<'?">,
+  InGroup<IntInBoolContext>;
+def warn_enum_constant_in_bool_context : Warning<
+  "enum constant in boolean context">,
+  InGroup<IntInBoolContext>;
+
+def warn_integer_constants_in_conditional : Warning<
+  "'?:' with integer constants in boolean context">,
+  InGroup<IntInBoolContext>;
+def warn_integer_constants_in_conditional_always_true : Warning<
+  "'?:' with integer constants in boolean context, the expression will always "
+  "evaluate to 'true'">,
+  InGroup<IntInBoolContext>;
+
 def warn_logical_instead_of_bitwise : Warning<
   "use of logical '%0' with constant operand">,
   InGroup<DiagGroup<"constant-logical-operand">>;
 def note_logical_instead_of_bitwise_change_operator : Note<
   "use '%0' for a bitwise operation">;
 def note_logical_instead_of_bitwise_remove_constant : Note<
   "remove constant to silence this warning">;
 

lib/Sema/SemaChecking.cpp

const char FirstContextCharacter =
         S.getSourceManager().getCharacterData(CC)[0];
     if (FirstContextCharacter == '{')
       return false;
   }
 
   return true;
 }
 
+static void DiagnoseIntInBoolContext(Sema &S, const Expr *E) {
+  E = E->IgnoreParenImpCasts();
+  SourceLocation ExprLoc = E->getExprLoc();
+
+  if (const auto *BO = dyn_cast<BinaryOperator>(E)) {
+    BinaryOperator::Opcode Opc = BO->getOpcode();
+    if (Opc == BO_Mul) {
+      const auto *LHS = dyn_cast<IntegerLiteral>(BO->getLHS());
+      const auto *RHS = dyn_cast<IntegerLiteral>(BO->getRHS());
+      if (LHS && RHS) {
+        bool AlwaysFalse = LHS->getValue() == 0 || RHS->getValue() == 0;
+        S.Diag(ExprLoc, diag::warn_mul_in_bool_context)
+            << AlwaysFalse;
+      } else if ((LHS && LHS->getValue() == 0) ||
+                 (RHS && RHS->getValue() == 0)) {
+        S.Diag(ExprLoc, diag::warn_mul_in_bool_context) << 1;
+      }
+    }
+    if (Opc == BO_Shl)
+      S.Diag(ExprLoc, diag::warn_left_shift_in_bool_context);
+  }
+
+  const auto *DRE = dyn_cast<DeclRefExpr>(E);
+  if (DRE && S.getLangOpts().CPlusPlus) {
+    const auto *ECD = dyn_cast<EnumConstantDecl>(DRE->getDecl());
+    if (ECD && ECD->getInitVal() != 0 && ECD->getInitVal() != 1)
+      S.Diag(ExprLoc, diag::warn_enum_constant_in_bool_context);
+  }
+
+  if (const auto *CO = dyn_cast<ConditionalOperator>(E)) {
+    const auto *LHS = dyn_cast<IntegerLiteral>(CO->getTrueExpr());
+    const auto *RHS = dyn_cast<IntegerLiteral>(CO->getFalseExpr());
+    if (LHS && RHS) {
+      if ((LHS->getValue() == 0 || LHS->getValue() == 1) &&
+          (RHS->getValue() == 0 || RHS->getValue() == 1))
+        // Do not diagnose common idioms
+        return;
+      if (LHS->getValue() != 0 && LHS->getValue() != 0)
+        S.Diag(ExprLoc,
+               diag::warn_integer_constants_in_conditional_always_true);
+      else
+        S.Diag(ExprLoc, diag::warn_integer_constants_in_conditional);
+    }
+  }
+}
+
 static void
 CheckImplicitConversion(Sema &S, Expr *E, QualType T, SourceLocation CC,
                         bool *ICContext = nullptr) {
   if (E->isTypeDependent() || E->isValueDependent()) return;
 
   const Type *Source = S.Context.getCanonicalType(E->getType()).getTypePtr();
   const Type *Target = S.Context.getCanonicalType(T).getTypePtr();
   if (Source == Target) return;
if (Source->isIntegerType()) {
       }
     }
   }
 
   DiagnoseNullConversion(S, E, T, CC);
 
   S.DiscardMisalignedMemberAddress(Target, E);
 
+  if (Target->isBooleanType())
+    DiagnoseIntInBoolContext(S, E);
+
   if (!Source->isIntegerType() || !Target->isIntegerType())
     return;
 
   // TODO: remove this early return once the false positives for constant->bool
   // in templates, macros, etc, are reduced or removed.
   if (Target->isSpecificBuiltinType(BuiltinType::Bool))
     return;
 
static void AnalyzeImplicitConversions(Sema &S, Expr *OrigE,
   if (E->isTypeDependent() || E->isValueDependent())
     return;
 
   // For conditional operators, we analyze the arguments as if they
   // were being fed directly into the output.
   if (isa<ConditionalOperator>(E)) {
     ConditionalOperator *CO = cast<ConditionalOperator>(E);
     CheckConditionalOperator(S, CO, CC, T);
+    if (T->isBooleanType())
+      DiagnoseIntInBoolContext(S, E);
     return;
   }
 
   // Check implicit argument conversions for function calls.
   if (CallExpr *Call = dyn_cast<CallExpr>(E))
     CheckImplicitArgumentConversions(S, Call, CC);
 
   // Go ahead and check any implicit conversions we might have skipped.

test/Sema/warn-int-in-bool-context.c

+// RUN: %clang_cc1 -x c -fsyntax-only -verify -Wint-in-bool-context %s
+// RUN: %clang_cc1 -x c -fsyntax-only -verify %s
+
+#define ONE 1
+#define TWO 2
+
+enum answer {
+  yes,
+  no,
+  unknown,
+};
+
+void g(_Bool b);
+
+_Bool test(int a, int b, enum answer ans) {
+  _Bool r = TWO * 7; // expected-warning {{'*' in boolean context, the expression will always evaluate to 'true'}}
+  r = 3 * 7;         // expected-warning {{'*' in boolean context, the expression will always evaluate to 'true'}}
+  r = a * 7;
+  r = a * b;
+  r = a * 0; // expected-warning {{'*' in boolean context, the expression will always evaluate to 'false'}}
+  r = 0 * b; // expected-warning {{'*' in boolean context, the expression will always evaluate to 'false'}}
+  g(3 * 9); // expected-warning {{'*' in boolean context, the expression will always evaluate to 'true'}}
+
+  r = TWO << 7; // expected-warning {{'<<' in boolean context; did you mean '<'?}}
+  r = a << 7;   // expected-warning {{'<<' in boolean context; did you mean '<'?}}
+  r = ONE << b; // expected-warning {{'<<' in boolean context; did you mean '<'?}}
+
+  r = a ? ONE : TWO; // expected-warning {{'?:' with integer constants in boolean context, the expression will always evaluate to 'true'}}
+  r = a ? (1) : TWO;
+  r = a ? 3 : TWO; // expected-warning {{'?:' with integer constants in boolean context, the expression will always evaluate to 'true'}}
+  r = a ? -2 : 0;
+  r = a ? 3 : -2;
+  r = a ? 0 : TWO; // expected-warning {{'?:' with integer constants in boolean context}}
+  r = a ? 3 : ONE; // expected-warning {{'?:' with integer constants in boolean context, the expression will always evaluate to 'true'}}
+  r = a ? ONE : 0;
+  r = a ? 0 : 0;
+  r = a ? ONE : 0;
+  r = a ? ONE : ONE;
+  g(a ? 3 : 9); // expected-warning {{'?:' with integer constants in boolean context, the expression will always evaluate to 'true'}}
+
+  r = a ? yes
+        : no;
+  r = ans == yes || no;
+  r = yes || ans == no;
+  r = ans == yes || ans == no;
+  r = !yes;
+  r = !no;
+  r = !unknown;
+  g(ans == yes || no);
+
+  if (8 * 4) // expected-warning {{'*' in boolean context, the expression will always evaluate to 'true'}}
+    return a;
+
+  if (a * 4)
+    return a;
+
+  if (-TWO * b)
+    return a;
+
+  if (TWO << 4) // expected-warning {{'<<' in boolean context; did you mean '<'?}}
+    return a;
+
+  if (a << TWO) // expected-warning {{'<<' in boolean context; did you mean '<'?}}
+    return a;
+
+  if (ONE << b) // expected-warning {{'<<' in boolean context; did you mean '<'?}}
+    return a;
+
+  if (a ? ONE : TWO) // expected-warning {{'?:' with integer constants in boolean context, the expression will always evaluate to 'true'}}
+    return a;
+
+  if (a ? 32 : TWO) // expected-warning {{'?:' with integer constants in boolean context, the expression will always evaluate to 'true'}}
+    return a;
+
+  if (a ? 7 : TWO) // expected-warning {{'?:' with integer constants in boolean context, the expression will always evaluate to 'true'}}
+    return a;
+
+  if (ans == yes || no)
+    return a;
+
+  if (yes || ans == no)
+    return a;
+
+  if (r || a ? 7 : TWO) // expected-warning {{'?:' with integer constants in boolean context, the expression will always evaluate to 'true'}}
+    return a;
+
+  if (b && a ? 7 : TWO) // expected-warning {{'?:' with integer constants in boolean context, the expression will always evaluate to 'true'}}
+    return a;
+
+  if ((a ? 7 : TWO) && b == 32) // expected-warning {{'?:' with integer constants in boolean context, the expression will always evaluate to 'true'}}
+    return a;
+
+  if (a ? 0 : 1)
+    return a;
+
+  if (a ? 0 : 2) // expected-warning {{'?:' with integer constants in boolean context}}
+    return a;
+
+  if (a ? ONE : 0)
+    return a;
+
+  for (; 1 * 7;) // expected-warning {{'*' in boolean context, the expression will always evaluate to 'true'}}
+    ;
+
+  for (; a ? 7 : TWO;) // expected-warning {{'?:' with integer constants in boolean context, the expression will always evaluate to 'true'}}
+    a--;
+
+  for (; a ? 0 : ONE;)
+    a--;
+
+  for (; 1 << 7;) // expected-warning {{'<<' in boolean context; did you mean '<'?}}
+    ;
+
+  for (; ans == yes || no;)
+    ;
+
+  for (; ans == yes || ans == no;)
+    ;
+
+  while (a * b)
+    ;
+
+  while (a ? 7 : TWO) // expected-warning {{'?:' with integer constants in boolean context, the expression will always evaluate to 'true'}}
+    ;
+
+  while (a ? 0 : ONE)
+    ;
+
+  while (a << b) // expected-warning {{'<<' in boolean context; did you mean '<'?}}
+    ;
+
+  while (ans == yes || no)
+    ;
+
+  while (ans == yes || ans == no)
+    ;
+
+  if (ONE)
+    return 5 * b;
+
+  if (ONE)
+    return ONE << a; // expected-warning {{'<<' in boolean context; did you mean '<'?}}
+
+  if (ONE)
+    return ans == yes || no;
+
+  if (ONE)
+    return b ? 8 : 64; // expected-warning {{'?:' with integer constants in boolean context, the expression will always evaluate to 'true'}}
+
+  return a << b; // expected-warning {{'<<' in boolean context; did you mean '<'?}}
+}

test/Sema/warn-unreachable.c

 }
 
 // Test case for <rdar://problem/11005770>.  We should treat code guarded
 // by 'x & 0' and 'x * 0' as unreachable.
 int calledFun();
 void test_mul_and_zero(int x) {
   if (x & 0) calledFun(); // expected-warning {{will never be executed}}
   if (0 & x) calledFun(); // expected-warning {{will never be executed}}
+  // expected-warning@+1 {{'*' in boolean context, the expression will always evaluate to 'false'}}
   if (x * 0) calledFun(); // expected-warning {{will never be executed}}
+  // expected-warning@+1 {{'*' in boolean context, the expression will always evaluate to 'false'}}
   if (0 * x) calledFun(); // expected-warning {{will never be executed}}
 }
 
 void raze() __attribute__((noreturn));
 void warn_here();
 
 int test_break_preceded_by_noreturn(int i) {
   switch (i) {
     case 1:

test/SemaCXX/nested-name-spec.cpp

 template <int N> struct T1 {
   int a,b,c;
   static const unsigned N1 = N;
   typedef unsigned C1;
 };
 T1<C2::N1> var_1a;
 T1<C2:N1> var_1b;  // expected-error{{unexpected ':' in nested name specifier; did you mean '::'?}}
 template<int N> int F() {}
-int (*X1)() = (B1::B2 ? F<1> : F<2>);
+int (*X1)() = (B1::B2 ? F<1> : F<2>); // expected-warning {{enum constant in boolean context}}
 int (*X2)() = (B1:B2 ? F<1> : F<2>);  // expected-error{{unexpected ':' in nested name specifier; did you mean '::'?}}
-
+// expected-warning@-1 {{enum constant in boolean context}}
 // Bit fields + templates
 struct S7a {
   T1<B1::B2>::C1 m1 : T1<B1::B2>::N1;
 };
 struct S7b {
   T1<B1:B2>::C1 m1 : T1<B1::B2>::N1;  // expected-error{{unexpected ':' in nested name specifier; did you mean '::'?}}
 };
 struct S7c {

test/SemaCXX/warn-int-in-bool-context.cpp

+// RUN: %clang_cc1 -x c++ -fsyntax-only -verify -Wint-in-bool-context %s
+// RUN: %clang_cc1 -x c++ -fsyntax-only -verify %s
+
+#define ONE 1
+#define TWO 2
+
+enum answer {
+  foo,
+  bar,
+  yes,
+  no,
+  unknown,
+};
+
+enum fruit {
+  orange = -3,
+  lemon,
+  banana,
+  apple,
+};
+
+void g(bool b);
+
+template <bool>
+void h() {}
+template <int>
+void h() {}
+
+bool test(int a, int b, enum answer ans, enum fruit f) {
+  bool r = TWO * 7; // expected-warning {{'*' in boolean context, the expression will always evaluate to 'true'}}
+  r = 3 * 7;        // expected-warning {{'*' in boolean context, the expression will always evaluate to 'true'}}
+  r = a * 7;
+  r = a * b;
+  r = a * 0; // expected-warning {{'*' in boolean context, the expression will always evaluate to 'false'}}
+  r = 0 * b; // expected-warning {{'*' in boolean context, the expression will always evaluate to 'false'}}
+  g(3 * 9);  // expected-warning {{'*' in boolean context, the expression will always evaluate to 'true'}}
+  h<3 * 9>();
+
+  r = TWO << 7; // expected-warning {{'<<' in boolean context; did you mean '<'?}}
+  r = a << 7;   // expected-warning {{'<<' in boolean context; did you mean '<'?}}
+  r = ONE << b; // expected-warning {{'<<' in boolean context; did you mean '<'?}}
+
+  r = a ? ONE : TWO; // expected-warning {{'?:' with integer constants in boolean context, the expression will always evaluate to 'true'}}
+  r = a ? (1) : TWO;
+  r = a ? 3 : TWO; // expected-warning {{'?:' with integer constants in boolean context, the expression will always evaluate to 'true'}}
+  r = a ? -2 : 0;
+  r = a ? 3 : -2;
+  r = a ? 0 : TWO; // expected-warning {{'?:' with integer constants in boolean context}}
+  r = a ? 3 : ONE; // expected-warning {{'?:' with integer constants in boolean context, the expression will always evaluate to 'true'}}
+  r = a ? ONE : 0;
+  r = a ? 0 : 0;
+  r = a ? ONE : 0;
+  r = a ? ONE : ONE;
+  g(a ? 3 : 9); // expected-warning {{'?:' with integer constants in boolean context, the expression will always evaluate to 'true'}}
+
+  r = a ? yes           // expected-warning {{enum constant in boolean context}}
+        : no;           // expected-warning {{enum constant in boolean context}}
+  r = ans == yes || no; // expected-warning {{enum constant in boolean context}}
+  r = yes || ans == no; // expected-warning {{enum constant in boolean context}}
+  r = ans == yes || ans == no;
+  r = !foo;
+  r = !bar;
+  r = !yes;            // expected-warning {{enum constant in boolean context}}
+  g(ans == yes || no); // expected-warning {{enum constant in boolean context}}
+
+  if (8 * 4) // expected-warning {{'*' in boolean context, the expression will always evaluate to 'true'}}
+    return a;
+
+  if (a * 4)
+    return a;
+
+  if (-TWO * b)
+    return a;
+
+  if (TWO << 4) // expected-warning {{'<<' in boolean context; did you mean '<'?}}
+    return a;
+
+  if (a << TWO) // expected-warning {{'<<' in boolean context; did you mean '<'?}}
+    return a;
+
+  if (ONE << b) // expected-warning {{'<<' in boolean context; did you mean '<'?}}
+    return a;
+
+  if (a ? ONE : TWO) // expected-warning {{'?:' with integer constants in boolean context, the expression will always evaluate to 'true'}}
+    return a;
+
+  if (a ? 32 : TWO) // expected-warning {{'?:' with integer constants in boolean context, the expression will always evaluate to 'true'}}
+    return a;
+
+  if (a ? 7 : TWO) // expected-warning {{'?:' with integer constants in boolean context, the expression will always evaluate to 'true'}}
+    return a;
+
+  if (ans == yes || foo)
+    return a;
+
+  if (f == apple || orange) // expected-warning {{enum constant in boolean context}}
+    return a;
+
+  if (ans == yes || no) // expected-warning {{enum constant in boolean context}}
+    return a;
+
+  if (yes || ans == no) // expected-warning {{enum constant in boolean context}}
+    return a;
+
+  if (r || a ? 7 : TWO) // expected-warning {{'?:' with integer constants in boolean context, the expression will always evaluate to 'true'}}
+    return a;
+
+  if (b && a ? 7 : TWO) // expected-warning {{'?:' with integer constants in boolean context, the expression will always evaluate to 'true'}}
+    return a;
+
+  if ((a ? 7 : TWO) && b == 32) // expected-warning {{'?:' with integer constants in boolean context, the expression will always evaluate to 'true'}}
+    return a;
+
+  if (a ? 0 : 1)
+    return a;
+
+  if (a ? 0 : 2) // expected-warning {{'?:' with integer constants in boolean context}}
+    return a;
+
+  if (a ? ONE : 0)
+    return a;
+
+  for (; 1 * 7;) // expected-warning {{'*' in boolean context, the expression will always evaluate to 'true'}}
+    ;
+
+  for (; a ? 7 : TWO;) // expected-warning {{'?:' with integer constants in boolean context, the expression will always evaluate to 'true'}}
+    a--;
+
+  for (; a ? 0 : ONE;)
+    a--;
+
+  for (; 1 << 7;) // expected-warning {{'<<' in boolean context; did you mean '<'?}}
+    ;
+
+  for (; ans == yes || no;) // expected-warning {{enum constant in boolean context}}
+    ;
+
+  for (; ans == yes || ans == no;)
+    ;
+
+  while (a * b)
+    ;
+
+  while (a ? 7 : TWO) // expected-warning {{'?:' with integer constants in boolean context, the expression will always evaluate to 'true'}}
+    ;
+
+  while (a ? 0 : ONE)
+    ;
+
+  while (a << b) // expected-warning {{'<<' in boolean context; did you mean '<'?}}
+    ;
+
+  while (ans == yes || no) // expected-warning {{enum constant in boolean context}}
+    ;
+
+  while (ans == yes || ans == no)
+    ;
+
+  if (ONE)
+    return 5 * b;
+
+  if (ONE)
+    return ONE << a; // expected-warning {{'<<' in boolean context; did you mean '<'?}}
+
+  if (ONE)
+    return ans == yes || no; // expected-warning {{enum constant in boolean context}}
+
+  if (ONE)
+    return b ? 8 : 64; // expected-warning {{'?:' with integer constants in boolean context, the expression will always evaluate to 'true'}}
+
+  return a << b; // expected-warning {{'<<' in boolean context; did you mean '<'?}}
+}