Add test cases for false positives on -Wnull-arithmetic from Richard

Trieu, and fix them by checking for array and function types as well as
pointer types.

I've added a predicate method on Type to bundle together the logic we're
using here: isPointerLikeType(). I'd welcome better names for this
predicate, this is the best I came up with. It's implemented as a switch
to be a touch lighter weight than all the chained isa<...> casts that
would result otherwise.

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

5 files changed, 56 insertions, 28 deletions

diff --git a/include/clang/AST/Type.h b/include/clang/AST/Type.h
index c5530c1985..08d6202966 100644
--- a/include/clang/AST/Type.h
+++ b/include/clang/AST/Type.h
@@ -1373,6 +1373,7 @@ public:
   bool isFunctionProtoType() const { return getAs<FunctionProtoType>(); }
   bool isPointerType() const;
   bool isAnyPointerType() const;   // Any C pointer or ObjC object pointer
+  bool isPointerLikeType() const;
   bool isBlockPointerType() const;
   bool isVoidPointerType() const;
   bool isReferenceType() const;
diff --git a/lib/AST/Type.cpp b/lib/AST/Type.cpp
index 080bca2198..b30f8966b6 100644
--- a/lib/AST/Type.cpp
+++ b/lib/AST/Type.cpp
@@ -289,6 +289,31 @@ bool Type::isDerivedType() const {
   }
 }
 
+/// \brief Tests whether the type behaves like a pointer type.
+///
+/// This includes all of the obviously pointer types including block pointers,
+/// member pointers, and ObjC Object pointers. It also includes function and
+/// array types which behave as pointers due to decay.
+///
+/// \returns True for types which act like pointer types.
+bool Type::isPointerLikeType() const {
+  switch (CanonicalType->getTypeClass()) {
+  case Pointer:
+  case BlockPointer:
+  case MemberPointer:
+  case ConstantArray:
+  case IncompleteArray:
+  case VariableArray:
+  case DependentSizedArray:
+  case FunctionProto:
+  case FunctionNoProto:
+  case ObjCObjectPointer:
+    return true;
+  default:
+    return false;
+  }
+}
+
 bool Type::isClassType() const {
   if (const RecordType *RT = getAs<RecordType>())
     return RT->getDecl()->isClass();
diff --git a/lib/Sema/SemaExpr.cpp b/lib/Sema/SemaExpr.cpp
index d3b7b10145..15751d078b 100644
--- a/lib/Sema/SemaExpr.cpp
+++ b/lib/Sema/SemaExpr.cpp
@@ -8953,37 +8953,20 @@ ExprResult Sema::CreateBuiltinBinOp(SourceLocation OpLoc,
         Opc == BO_AndAssign || Opc == BO_OrAssign || Opc == BO_XorAssign) {
       // These are the operations that would not make sense with a null pointer
       // no matter what the other expression is.
-      if (LeftNull && RightNull) {
-        Diag(OpLoc, diag::warn_null_in_arithmetic_operation)
-             << lhs.get()->getSourceRange() << rhs.get()->getSourceRange();
-      } else if (LeftNull) {
-        Diag(OpLoc, diag::warn_null_in_arithmetic_operation)
-             << lhs.get()->getSourceRange();
-      } else if (RightNull) {
-        Diag(OpLoc, diag::warn_null_in_arithmetic_operation)
-             << rhs.get()->getSourceRange();
-      }
+      Diag(OpLoc, diag::warn_null_in_arithmetic_operation)
+        << (LeftNull ? lhs.get()->getSourceRange() : SourceRange())
+        << (RightNull ? rhs.get()->getSourceRange() : SourceRange());
     } else if (Opc == BO_LE || Opc == BO_LT || Opc == BO_GE || Opc == BO_GT ||
                Opc == BO_EQ || Opc == BO_NE) {
       // These are the operations that would not make sense with a null pointer
       // if the other expression the other expression is not a pointer.
       QualType LeftType = lhs.get()->getType();
       QualType RightType = rhs.get()->getType();
-      bool LeftPointer = LeftType->isPointerType() ||
-                         LeftType->isBlockPointerType() ||
-                         LeftType->isMemberPointerType() ||
-                         LeftType->isObjCObjectPointerType();
-      bool RightPointer = RightType->isPointerType() ||
-                          RightType->isBlockPointerType() ||
-                          RightType->isMemberPointerType() ||
-                          RightType->isObjCObjectPointerType();
-      if ((LeftNull != RightNull) && !LeftPointer && !RightPointer) {
-        if (LeftNull)
-          Diag(OpLoc, diag::warn_null_in_arithmetic_operation)
-               << lhs.get()->getSourceRange();
-        if (RightNull)
-          Diag(OpLoc, diag::warn_null_in_arithmetic_operation)
-               << rhs.get()->getSourceRange();
+      if (LeftNull != RightNull &&
+          !LeftType->isPointerLikeType() && !RightType->isPointerLikeType()) {
+        Diag(OpLoc, diag::warn_null_in_arithmetic_operation)
+          << (LeftNull ? lhs.get()->getSourceRange()
+                       : rhs.get()->getSourceRange());
       }
     }
   }
diff --git a/test/SemaCXX/null_in_arithmetic_ops.cpp b/test/SemaCXX/null_in_arithmetic_ops.cpp
index b0303177c2..78c76e7e4c 100644
--- a/test/SemaCXX/null_in_arithmetic_ops.cpp
+++ b/test/SemaCXX/null_in_arithmetic_ops.cpp
@@ -59,6 +59,10 @@ void f() {
   b = 0 == a;
   b = 0 == &a;
 
+  b = NULL < NULL || NULL > NULL;
+  b = NULL <= NULL || NULL >= NULL;
+  b = NULL == NULL || NULL != NULL;
+
   b = ((NULL)) != a;  // expected-warning{{use of NULL in arithmetic operation}}
 
   void (^c)();
@@ -67,4 +71,11 @@ void f() {
   class X;
   void (X::*d) ();
   b = d == NULL || NULL == d || d != NULL || NULL != d;
+
+  extern void e();
+  b = e == NULL || NULL == e || e != NULL || NULL != e;
+
+  int f[2];
+  b = f == NULL || NULL == f || f != NULL || NULL != f;
+  b = "f" == NULL || NULL == "f" || "f" != NULL || NULL != "f";
 }
diff --git a/test/SemaCXX/nullptr_in_arithmetic_ops.cpp b/test/SemaCXX/nullptr_in_arithmetic_ops.cpp
index 350d18a13f..e839ed116f 100644
--- a/test/SemaCXX/nullptr_in_arithmetic_ops.cpp
+++ b/test/SemaCXX/nullptr_in_arithmetic_ops.cpp
@@ -1,6 +1,6 @@
 // RUN: %clang_cc1 -fsyntax-only -fblocks -std=c++0x -verify %s
 
-void f() {
+void foo() {
   int a;
   bool b;
 
@@ -49,8 +49,9 @@ void f() {
   b = &a <= nullptr || nullptr <= &a || &a >= nullptr || nullptr >= &a;
   b = &a == nullptr || nullptr == &a || &a != nullptr || nullptr != &a;
 
-  b = 0 == a;
-  b = 0 == &a;
+  b = nullptr < nullptr || nullptr > nullptr;
+  b = nullptr <= nullptr || nullptr >= nullptr;
+  b = nullptr == nullptr || nullptr != nullptr;
 
   b = ((nullptr)) != a;  // expected-error{{invalid operands to binary expression}}
 
@@ -62,4 +63,11 @@ void f() {
   void (X::*d) ();
   d = nullptr;
   b = d == nullptr || nullptr == d || d != nullptr || nullptr != d;
+
+  extern void e();
+  b = e == nullptr || nullptr == e || e != nullptr || nullptr != e;
+
+  int f[2];
+  b = f == nullptr || nullptr == f || f != nullptr || nullptr != f;
+  b = "f" == nullptr || nullptr == "f" || "f" != nullptr || nullptr != "f";
 }