[cfe-commits] r101576 - in /cfe/trunk: include/clang/AST/Expr.h include/clang/Basic/DiagnosticSemaKinds.td lib/AST/Expr.cpp lib/Sema/SemaStmt.cpp test/Sema/statements.c

Chris Lattner sabre at nondot.org
Fri Apr 16 16:34:13 PDT 2010


Author: lattner
Date: Fri Apr 16 18:34:13 2010
New Revision: 101576

URL: http://llvm.org/viewvc/llvm-project?rev=101576&view=rev
Log:
make our existing "switch on bool" warning work for C.  Since
the result of comparisons are 'int' in C, it doesn't work to
test just the result type of the expression.

Modified:
    cfe/trunk/include/clang/AST/Expr.h
    cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td
    cfe/trunk/lib/AST/Expr.cpp
    cfe/trunk/lib/Sema/SemaStmt.cpp
    cfe/trunk/test/Sema/statements.c

Modified: cfe/trunk/include/clang/AST/Expr.h
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/AST/Expr.h?rev=101576&r1=101575&r2=101576&view=diff
==============================================================================
--- cfe/trunk/include/clang/AST/Expr.h (original)
+++ cfe/trunk/include/clang/AST/Expr.h Fri Apr 16 18:34:13 2010
@@ -199,6 +199,12 @@
   /// \brief Returns whether this expression refers to a vector element.
   bool refersToVectorElement() const;
   
+  /// 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 isKnownToHaveBooleanValue() const;
+  
   /// isIntegerConstantExpr - Return true if this expression is a valid integer
   /// constant expression, and, if so, return its value in Result.  If not a
   /// valid i-c-e, return false and fill in Loc (if specified) with the location
@@ -304,7 +310,7 @@
   ///  its subexpression.  If that subexpression is also a ParenExpr,
   ///  then this method recursively returns its subexpression, and so forth.
   ///  Otherwise, the method returns the current Expr.
-  Expr* IgnoreParens();
+  Expr *IgnoreParens();
 
   /// IgnoreParenCasts - Ignore parentheses and casts.  Strip off any ParenExpr
   /// or CastExprs, returning their operand.
@@ -333,7 +339,7 @@
   /// temporary object.
   const Expr *getTemporaryObject() const;
 
-  const Expr* IgnoreParens() const {
+  const Expr *IgnoreParens() const {
     return const_cast<Expr*>(this)->IgnoreParens();
   }
   const Expr *IgnoreParenCasts() const {

Modified: cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td?rev=101576&r1=101575&r2=101576&view=diff
==============================================================================
--- cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td (original)
+++ cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td Fri Apr 16 18:34:13 2010
@@ -2782,7 +2782,7 @@
   "'default' statement not in switch statement">;
 def err_case_not_in_switch : Error<"'case' statement not in switch statement">;
 def warn_bool_switch_condition : Warning<
-  "switch condition is a bool">;
+  "switch condition has boolean value">;
 def warn_case_value_overflow : Warning<
   "overflow converting case value to switch condition type (%0 to %1)">,
   InGroup<DiagGroup<"switch">>;

Modified: cfe/trunk/lib/AST/Expr.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/AST/Expr.cpp?rev=101576&r1=101575&r2=101576&view=diff
==============================================================================
--- cfe/trunk/lib/AST/Expr.cpp (original)
+++ cfe/trunk/lib/AST/Expr.cpp Fri Apr 16 18:34:13 2010
@@ -27,6 +27,65 @@
 #include <algorithm>
 using namespace clang;
 
+/// 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 {
+  // If this value has _Bool type, it is obvious 0/1.
+  if (getType()->isBooleanType()) return true;
+  // If this is a non-scalar-integer type, we don't care enough to try. 
+  if (!getType()->isIntegralType()) return false;
+  
+  if (const ParenExpr *PE = dyn_cast<ParenExpr>(this))
+    return PE->getSubExpr()->isKnownToHaveBooleanValue();
+  
+  if (const UnaryOperator *UO = dyn_cast<UnaryOperator>(this)) {
+    switch (UO->getOpcode()) {
+    case UnaryOperator::Plus:
+    case UnaryOperator::Extension:
+      return UO->getSubExpr()->isKnownToHaveBooleanValue();
+    default:
+      return false;
+    }
+  }
+  
+  if (const CastExpr *CE = dyn_cast<CastExpr>(this))
+    return CE->getSubExpr()->isKnownToHaveBooleanValue();
+  
+  if (const BinaryOperator *BO = dyn_cast<BinaryOperator>(this)) {
+    switch (BO->getOpcode()) {
+    default: return false;
+    case BinaryOperator::LT:   // Relational operators.
+    case BinaryOperator::GT:
+    case BinaryOperator::LE:
+    case BinaryOperator::GE:
+    case BinaryOperator::EQ:   // Equality operators.
+    case BinaryOperator::NE:
+    case BinaryOperator::LAnd: // AND operator.
+    case BinaryOperator::LOr:  // Logical OR operator.
+      return true;
+        
+    case BinaryOperator::And:  // Bitwise AND operator.
+    case BinaryOperator::Xor:  // Bitwise XOR operator.
+    case BinaryOperator::Or:   // Bitwise OR operator.
+      // Handle things like (x==2)|(y==12).
+      return BO->getLHS()->isKnownToHaveBooleanValue() &&
+             BO->getRHS()->isKnownToHaveBooleanValue();
+        
+    case BinaryOperator::Comma:
+    case BinaryOperator::Assign:
+      return BO->getRHS()->isKnownToHaveBooleanValue();
+    }
+  }
+  
+  if (const ConditionalOperator *CO = dyn_cast<ConditionalOperator>(this))
+    return CO->getTrueExpr()->isKnownToHaveBooleanValue() &&
+           CO->getFalseExpr()->isKnownToHaveBooleanValue();
+  
+  return false;
+}
+
 //===----------------------------------------------------------------------===//
 // Primary Expressions.
 //===----------------------------------------------------------------------===//

Modified: cfe/trunk/lib/Sema/SemaStmt.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/SemaStmt.cpp?rev=101576&r1=101575&r2=101576&view=diff
==============================================================================
--- cfe/trunk/lib/Sema/SemaStmt.cpp (original)
+++ cfe/trunk/lib/Sema/SemaStmt.cpp Fri Apr 16 18:34:13 2010
@@ -593,7 +593,7 @@
       return StmtError();
     }
 
-    if (CondTypeBeforePromotion->isBooleanType()) {
+    if (CondExpr->isKnownToHaveBooleanValue()) {
       // switch(bool_expr) {...} is often a programmer error, e.g.
       //   switch(n && mask) { ... }  // Doh - should be "n & mask".
       // One can always use an if statement instead of switch(bool_expr).

Modified: cfe/trunk/test/Sema/statements.c
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Sema/statements.c?rev=101576&r1=101575&r2=101576&view=diff
==============================================================================
--- cfe/trunk/test/Sema/statements.c (original)
+++ cfe/trunk/test/Sema/statements.c Fri Apr 16 18:34:13 2010
@@ -41,3 +41,13 @@
   {
   }
 }
+
+// rdar://3271964
+enum Numbers { kOne,  kTwo,  kThree,  kFour};
+int test12(enum Numbers num) {
+  switch (num == kOne) {// expected-warning {{switch condition has boolean value}}
+  default: 
+  case kThree:
+    break;
+  }
+}
\ No newline at end of file





More information about the cfe-commits mailing list