[cfe-commits] r86143 - in /cfe/trunk: include/clang/Basic/DiagnosticSemaKinds.td include/clang/Parse/Scope.h lib/Sema/SemaExpr.cpp test/SemaCXX/warn-for-var-in-else.cpp

[Douglas Gregor]

Author: dgregor
Date: Thu Nov  5 11:49:26 2009
New Revision: 86143

URL: http://llvm.org/viewvc/llvm-project?rev=86143&view=rev
Log:
Eliminate some false positives due to a thinko in the "'blah' is
always zero in this context" warning logic. Also, make the diagnostic
itself more precise when referring to pointer values ("NULL" vs. "zero").

Modified:
    cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td
    cfe/trunk/include/clang/Parse/Scope.h
    cfe/trunk/lib/Sema/SemaExpr.cpp
    cfe/trunk/test/SemaCXX/warn-for-var-in-else.cpp

Modified: cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td?rev=86143&r1=86142&r2=86143&view=diff

==============================================================================
--- cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td (original)
+++ cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td Thu Nov  5 11:49:26 2009
@@ -1787,8 +1787,8 @@
   "assignment as a condition without parentheses">,
   InGroup<Parentheses>;
 
-def warn_value_always_zero : Warning<"%0 is always zero in this context">;
-def warn_value_always_false : Warning<"%0 is always false in this context">;
+def warn_value_always_zero : Warning<
+  "%0 is always %select{zero|false|NULL}1 in this context">;
 
 // assignment related diagnostics (also for argument passing, returning, etc).
 // FIXME: %2 is an english string here.

Modified: cfe/trunk/include/clang/Parse/Scope.h
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Parse/Scope.h?rev=86143&r1=86142&r2=86143&view=diff

==============================================================================
--- cfe/trunk/include/clang/Parse/Scope.h (original)
+++ cfe/trunk/include/clang/Parse/Scope.h Thu Nov  5 11:49:26 2009
@@ -275,7 +275,8 @@
     AnyParent = Parent;
     Depth = AnyParent ? AnyParent->Depth+1 : 0;
     Flags = ScopeFlags;
-
+    WithinElse = false;
+    
     if (AnyParent) {
       FnParent       = AnyParent->FnParent;
       BreakParent    = AnyParent->BreakParent;
@@ -283,13 +284,10 @@
       ControlParent = AnyParent->ControlParent;
       BlockParent  = AnyParent->BlockParent;
       TemplateParamParent = AnyParent->TemplateParamParent;
-      WithinElse = AnyParent->WithinElse;
-
     } else {
       FnParent = BreakParent = ContinueParent = BlockParent = 0;
       ControlParent = 0;
       TemplateParamParent = 0;
-      WithinElse = false;
     }
 
     // If this scope is a function or contains breaks/continues, remember it.

Modified: cfe/trunk/lib/Sema/SemaExpr.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/SemaExpr.cpp?rev=86143&r1=86142&r2=86143&view=diff

==============================================================================
--- cfe/trunk/lib/Sema/SemaExpr.cpp (original)
+++ cfe/trunk/lib/Sema/SemaExpr.cpp Thu Nov  5 11:49:26 2009
@@ -814,22 +814,18 @@
     // information to check this property.
     if (Var->isDeclaredInCondition() && Var->getType()->isScalarType()) {
       Scope *CheckS = S;
-      while (CheckS) {
+      while (CheckS && CheckS->getControlParent()) {
         if (CheckS->isWithinElse() &&
             CheckS->getControlParent()->isDeclScope(DeclPtrTy::make(Var))) {
-          if (Var->getType()->isBooleanType())
-            ExprError(Diag(Loc, diag::warn_value_always_false)
-                      << Var->getDeclName());
-          else
-            ExprError(Diag(Loc, diag::warn_value_always_zero)
-                      << Var->getDeclName());
+          ExprError(Diag(Loc, diag::warn_value_always_zero)
+            << Var->getDeclName()
+            << (Var->getType()->isPointerType()? 2 :
+                Var->getType()->isBooleanType()? 1 : 0));
           break;
         }
 
-        // Move up one more control parent to check again.
-        CheckS = CheckS->getControlParent();
-        if (CheckS)
-          CheckS = CheckS->getParent();
+        // Move to the parent of this scope.
+        CheckS = CheckS->getParent();
       }
     }
   } else if (FunctionDecl *Func = dyn_cast<FunctionDecl>(D)) {

Modified: cfe/trunk/test/SemaCXX/warn-for-var-in-else.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/SemaCXX/warn-for-var-in-else.cpp?rev=86143&r1=86142&r2=86143&view=diff

==============================================================================
--- cfe/trunk/test/SemaCXX/warn-for-var-in-else.cpp (original)
+++ cfe/trunk/test/SemaCXX/warn-for-var-in-else.cpp Thu Nov  5 11:49:26 2009
@@ -2,6 +2,7 @@
 // rdar://6425550
 int bar();
 void do_something(int);
+int *get_ptr();
 
 int foo() {
   if (int X = bar()) {
@@ -25,7 +26,20 @@
       do_something(B); // expected-warning{{'B' is always false in this context}}
     } else if (B2) {  // expected-warning{{'B2' is always false in this context}}
       do_something(B); // expected-warning{{'B' is always false in this context}}
+      do_something(B2); // expected-warning{{'B2' is always false in this context}}
     }
     return B; // expected-warning{{'B' is always false in this context}}
   }
 }
+
+void foo3() {  
+  if (int *P1 = get_ptr())
+    do_something(*P1);
+  else if (int *P2 = get_ptr()) {
+    do_something(*P1); // expected-warning{{'P1' is always NULL in this context}}
+    do_something(*P2);
+  } else {
+    do_something(*P1); // expected-warning{{'P1' is always NULL in this context}}
+    do_something(*P2); // expected-warning{{'P2' is always NULL in this context}}
+  }
+}