From: Douglas Gregor <dgregor@apple.com>
Date: Thu, 5 Nov 2009 17:49:26 +0000 (+0000)
Subject: Eliminate some false positives due to a thinko in the "'blah' is
X-Git-Url: https://granicus.if.org/sourcecode?a=commitdiff_plain;h=9c4b838782132ec670fd7e48d1a7a7fd433fed06;p=clang

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").


git-svn-id: https://llvm.org/svn/llvm-project/cfe/trunk@86143 91177308-0d34-0410-b5e6-96231b3b80d8
---

diff --git a/include/clang/Basic/DiagnosticSemaKinds.td b/include/clang/Basic/DiagnosticSemaKinds.td
index 2309908df7..06b09a0b73 100644
--- a/include/clang/Basic/DiagnosticSemaKinds.td
+++ b/include/clang/Basic/DiagnosticSemaKinds.td
@@ -1787,8 +1787,8 @@ def warn_condition_is_assignment : Warning<"using the result of an "
   "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.
diff --git a/include/clang/Parse/Scope.h b/include/clang/Parse/Scope.h
index 480b94f73f..c6a1e53472 100644
--- a/include/clang/Parse/Scope.h
+++ b/include/clang/Parse/Scope.h
@@ -275,7 +275,8 @@ public:
     AnyParent = Parent;
     Depth = AnyParent ? AnyParent->Depth+1 : 0;
     Flags = ScopeFlags;
-
+    WithinElse = false;
+    
     if (AnyParent) {
       FnParent       = AnyParent->FnParent;
       BreakParent    = AnyParent->BreakParent;
@@ -283,13 +284,10 @@ public:
       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.
diff --git a/lib/Sema/SemaExpr.cpp b/lib/Sema/SemaExpr.cpp
index f1d6f2bb17..f94017129b 100644
--- a/lib/Sema/SemaExpr.cpp
+++ b/lib/Sema/SemaExpr.cpp
@@ -814,22 +814,18 @@ Sema::ActOnDeclarationNameExpr(Scope *S, SourceLocation Loc,
     // 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)) {
diff --git a/test/SemaCXX/warn-for-var-in-else.cpp b/test/SemaCXX/warn-for-var-in-else.cpp
index f73c606894..c46b30640f 100644
--- a/test/SemaCXX/warn-for-var-in-else.cpp
+++ b/test/SemaCXX/warn-for-var-in-else.cpp
@@ -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 @@ bool foo2() {
       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}}
+  }
+}