Author: Erik Pilkington Date: 2019-11-20T16:29:31-08:00 New Revision: d9957c7405bc726422dbc2736ad62f704916fbe8
URL: https://github.com/llvm/llvm-project/commit/d9957c7405bc726422dbc2736ad62f704916fbe8 DIFF: https://github.com/llvm/llvm-project/commit/d9957c7405bc726422dbc2736ad62f704916fbe8.diff LOG: [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 8bfb353bb33c, pointed out by Chris Hamilton! Added: Modified: clang/include/clang/AST/Expr.h clang/lib/AST/Expr.cpp clang/lib/Sema/SemaChecking.cpp clang/test/Sema/switch.c Removed: ################################################################################ diff --git a/clang/include/clang/AST/Expr.h b/clang/include/clang/AST/Expr.h index ffa7d4db96a4..867615ded162 100644 --- a/clang/include/clang/AST/Expr.h +++ b/clang/include/clang/AST/Expr.h @@ -494,7 +494,13 @@ class Expr : public ValueStmt { /// that is known to return 0 or 1. This happens for _Bool/bool expressions /// but also int expressions which are produced by things like comparisons in /// C. - bool isKnownToHaveBooleanValue() const; + /// + /// \param Semantic If true, only return true for expressions that are known + /// to be semantically boolean, which might not be true even for expressions + /// that are known to evaluate to 0/1. For instance, reading an unsigned + /// bit-field with width '1' will evaluate to 0/1, but doesn't necessarily + /// semantically correspond to a bool. + bool isKnownToHaveBooleanValue(bool Semantic = true) const; /// isIntegerConstantExpr - Return true if this expression is a valid integer /// constant expression, and, if so, return its value in Result. If not a diff --git a/clang/lib/AST/Expr.cpp b/clang/lib/AST/Expr.cpp index d5c35e53059b..e5bb8a778c18 100644 --- a/clang/lib/AST/Expr.cpp +++ b/clang/lib/AST/Expr.cpp @@ -127,11 +127,7 @@ const Expr *Expr::skipRValueSubobjectAdjustments( return E; } -/// isKnownToHaveBooleanValue - Return true if this is an integer expression -/// that is known to return 0 or 1. This happens for _Bool/bool expressions -/// but also int expressions which are produced by things like comparisons in -/// C. -bool Expr::isKnownToHaveBooleanValue() const { +bool Expr::isKnownToHaveBooleanValue(bool Semantic) const { const Expr *E = IgnoreParens(); // If this value has _Bool type, it is obvious 0/1. @@ -142,7 +138,7 @@ bool Expr::isKnownToHaveBooleanValue() const { if (const UnaryOperator *UO = dyn_cast<UnaryOperator>(E)) { switch (UO->getOpcode()) { case UO_Plus: - return UO->getSubExpr()->isKnownToHaveBooleanValue(); + return UO->getSubExpr()->isKnownToHaveBooleanValue(Semantic); case UO_LNot: return true; default: @@ -152,8 +148,9 @@ bool Expr::isKnownToHaveBooleanValue() const { // Only look through implicit casts. If the user writes // '(int) (a && b)' treat it as an arbitrary int. + // FIXME: Should we look through any cast expression in !Semantic mode? if (const ImplicitCastExpr *CE = dyn_cast<ImplicitCastExpr>(E)) - return CE->getSubExpr()->isKnownToHaveBooleanValue(); + return CE->getSubExpr()->isKnownToHaveBooleanValue(Semantic); if (const BinaryOperator *BO = dyn_cast<BinaryOperator>(E)) { switch (BO->getOpcode()) { @@ -172,27 +169,27 @@ bool Expr::isKnownToHaveBooleanValue() const { case BO_Xor: // Bitwise XOR operator. case BO_Or: // Bitwise OR operator. // Handle things like (x==2)|(y==12). - return BO->getLHS()->isKnownToHaveBooleanValue() && - BO->getRHS()->isKnownToHaveBooleanValue(); + return BO->getLHS()->isKnownToHaveBooleanValue(Semantic) && + BO->getRHS()->isKnownToHaveBooleanValue(Semantic); case BO_Comma: case BO_Assign: - return BO->getRHS()->isKnownToHaveBooleanValue(); + return BO->getRHS()->isKnownToHaveBooleanValue(Semantic); } } if (const ConditionalOperator *CO = dyn_cast<ConditionalOperator>(E)) - return CO->getTrueExpr()->isKnownToHaveBooleanValue() && - CO->getFalseExpr()->isKnownToHaveBooleanValue(); + return CO->getTrueExpr()->isKnownToHaveBooleanValue(Semantic) && + CO->getFalseExpr()->isKnownToHaveBooleanValue(Semantic); if (isa<ObjCBoolLiteralExpr>(E)) return true; if (const auto *OVE = dyn_cast<OpaqueValueExpr>(E)) - return OVE->getSourceExpr()->isKnownToHaveBooleanValue(); + return OVE->getSourceExpr()->isKnownToHaveBooleanValue(Semantic); if (const FieldDecl *FD = E->getSourceBitField()) - if (FD->getType()->isUnsignedIntegerType() && + if (!Semantic && FD->getType()->isUnsignedIntegerType() && !FD->getBitWidth()->isValueDependent() && FD->getBitWidthValue(FD->getASTContext()) == 1) return true; diff --git a/clang/lib/Sema/SemaChecking.cpp b/clang/lib/Sema/SemaChecking.cpp index 65e4112d5e5a..a4218add4e71 100644 --- a/clang/lib/Sema/SemaChecking.cpp +++ b/clang/lib/Sema/SemaChecking.cpp @@ -11845,7 +11845,7 @@ static void CheckImplicitConversion(Sema &S, Expr *E, QualType T, return; if (isObjCSignedCharBool(S, T) && !Source->isCharType() && - !E->isKnownToHaveBooleanValue()) { + !E->isKnownToHaveBooleanValue(/*Semantic=*/false)) { return adornObjCBoolConversionDiagWithTernaryFixit( S, E, S.Diag(CC, diag::warn_impcast_int_to_objc_signed_char_bool) diff --git a/clang/test/Sema/switch.c b/clang/test/Sema/switch.c index 638fd997679a..f704c886c96f 100644 --- a/clang/test/Sema/switch.c +++ b/clang/test/Sema/switch.c @@ -283,6 +283,10 @@ void test16() { } } +struct bitfield_member { + unsigned bf : 1; +}; + // PR7359 void test17(int x) { switch (x >= 17) { // expected-warning {{switch condition has boolean value}} @@ -292,6 +296,13 @@ void test17(int x) { switch ((int) (x <= 17)) { case 0: return; } + + struct bitfield_member bm; + switch (bm.bf) { // no warning + case 0: + case 1: + return; + } } int test18() { _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits