Skip to content

Commit d9957c7

Browse files
committed
[Sema] Add a 'Semantic' parameter to Expr::isKnownToHaveBooleanValue
Some clients of this function want to know about any expression that is known to produce a 0/1 value, and others care about expressions that are semantically boolean. This fixes a -Wswitch-bool regression I introduced in 8bfb353, pointed out by Chris Hamilton!
1 parent a0da875 commit d9957c7

File tree

4 files changed

+30
-16
lines changed

4 files changed

+30
-16
lines changed

clang/include/clang/AST/Expr.h

Lines changed: 7 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -494,7 +494,13 @@ class Expr : public ValueStmt {
494494
/// that is known to return 0 or 1. This happens for _Bool/bool expressions
495495
/// but also int expressions which are produced by things like comparisons in
496496
/// C.
497-
bool isKnownToHaveBooleanValue() const;
497+
///
498+
/// \param Semantic If true, only return true for expressions that are known
499+
/// to be semantically boolean, which might not be true even for expressions
500+
/// that are known to evaluate to 0/1. For instance, reading an unsigned
501+
/// bit-field with width '1' will evaluate to 0/1, but doesn't necessarily
502+
/// semantically correspond to a bool.
503+
bool isKnownToHaveBooleanValue(bool Semantic = true) const;
498504

499505
/// isIntegerConstantExpr - Return true if this expression is a valid integer
500506
/// constant expression, and, if so, return its value in Result. If not a

clang/lib/AST/Expr.cpp

Lines changed: 11 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -127,11 +127,7 @@ const Expr *Expr::skipRValueSubobjectAdjustments(
127127
return E;
128128
}
129129

130-
/// isKnownToHaveBooleanValue - Return true if this is an integer expression
131-
/// that is known to return 0 or 1. This happens for _Bool/bool expressions
132-
/// but also int expressions which are produced by things like comparisons in
133-
/// C.
134-
bool Expr::isKnownToHaveBooleanValue() const {
130+
bool Expr::isKnownToHaveBooleanValue(bool Semantic) const {
135131
const Expr *E = IgnoreParens();
136132

137133
// If this value has _Bool type, it is obvious 0/1.
@@ -142,7 +138,7 @@ bool Expr::isKnownToHaveBooleanValue() const {
142138
if (const UnaryOperator *UO = dyn_cast<UnaryOperator>(E)) {
143139
switch (UO->getOpcode()) {
144140
case UO_Plus:
145-
return UO->getSubExpr()->isKnownToHaveBooleanValue();
141+
return UO->getSubExpr()->isKnownToHaveBooleanValue(Semantic);
146142
case UO_LNot:
147143
return true;
148144
default:
@@ -152,8 +148,9 @@ bool Expr::isKnownToHaveBooleanValue() const {
152148

153149
// Only look through implicit casts. If the user writes
154150
// '(int) (a && b)' treat it as an arbitrary int.
151+
// FIXME: Should we look through any cast expression in !Semantic mode?
155152
if (const ImplicitCastExpr *CE = dyn_cast<ImplicitCastExpr>(E))
156-
return CE->getSubExpr()->isKnownToHaveBooleanValue();
153+
return CE->getSubExpr()->isKnownToHaveBooleanValue(Semantic);
157154

158155
if (const BinaryOperator *BO = dyn_cast<BinaryOperator>(E)) {
159156
switch (BO->getOpcode()) {
@@ -172,27 +169,27 @@ bool Expr::isKnownToHaveBooleanValue() const {
172169
case BO_Xor: // Bitwise XOR operator.
173170
case BO_Or: // Bitwise OR operator.
174171
// Handle things like (x==2)|(y==12).
175-
return BO->getLHS()->isKnownToHaveBooleanValue() &&
176-
BO->getRHS()->isKnownToHaveBooleanValue();
172+
return BO->getLHS()->isKnownToHaveBooleanValue(Semantic) &&
173+
BO->getRHS()->isKnownToHaveBooleanValue(Semantic);
177174

178175
case BO_Comma:
179176
case BO_Assign:
180-
return BO->getRHS()->isKnownToHaveBooleanValue();
177+
return BO->getRHS()->isKnownToHaveBooleanValue(Semantic);
181178
}
182179
}
183180

184181
if (const ConditionalOperator *CO = dyn_cast<ConditionalOperator>(E))
185-
return CO->getTrueExpr()->isKnownToHaveBooleanValue() &&
186-
CO->getFalseExpr()->isKnownToHaveBooleanValue();
182+
return CO->getTrueExpr()->isKnownToHaveBooleanValue(Semantic) &&
183+
CO->getFalseExpr()->isKnownToHaveBooleanValue(Semantic);
187184

188185
if (isa<ObjCBoolLiteralExpr>(E))
189186
return true;
190187

191188
if (const auto *OVE = dyn_cast<OpaqueValueExpr>(E))
192-
return OVE->getSourceExpr()->isKnownToHaveBooleanValue();
189+
return OVE->getSourceExpr()->isKnownToHaveBooleanValue(Semantic);
193190

194191
if (const FieldDecl *FD = E->getSourceBitField())
195-
if (FD->getType()->isUnsignedIntegerType() &&
192+
if (!Semantic && FD->getType()->isUnsignedIntegerType() &&
196193
!FD->getBitWidth()->isValueDependent() &&
197194
FD->getBitWidthValue(FD->getASTContext()) == 1)
198195
return true;

clang/lib/Sema/SemaChecking.cpp

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -11845,7 +11845,7 @@ static void CheckImplicitConversion(Sema &S, Expr *E, QualType T,
1184511845
return;
1184611846

1184711847
if (isObjCSignedCharBool(S, T) && !Source->isCharType() &&
11848-
!E->isKnownToHaveBooleanValue()) {
11848+
!E->isKnownToHaveBooleanValue(/*Semantic=*/false)) {
1184911849
return adornObjCBoolConversionDiagWithTernaryFixit(
1185011850
S, E,
1185111851
S.Diag(CC, diag::warn_impcast_int_to_objc_signed_char_bool)

clang/test/Sema/switch.c

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -283,6 +283,10 @@ void test16() {
283283
}
284284
}
285285

286+
struct bitfield_member {
287+
unsigned bf : 1;
288+
};
289+
286290
// PR7359
287291
void test17(int x) {
288292
switch (x >= 17) { // expected-warning {{switch condition has boolean value}}
@@ -292,6 +296,13 @@ void test17(int x) {
292296
switch ((int) (x <= 17)) {
293297
case 0: return;
294298
}
299+
300+
struct bitfield_member bm;
301+
switch (bm.bf) { // no warning
302+
case 0:
303+
case 1:
304+
return;
305+
}
295306
}
296307

297308
int test18() {

0 commit comments

Comments
 (0)