[clang] d9957c7 - [Sema] Add a 'Semantic' parameter to Expr::isKnownToHaveBooleanValue

[Erik Pilkington via cfe-commits]

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() {