Move away from the poor "abstraction" I added to Type. John argued

effectively that this abstraction simply doesn't exist. That is
highlighted by the fact that by using it we were papering over a more
serious error in this warning: the fact that we warned for *invalid*
constructs involving member pointers and block pointers.

I've fixed the obvious issues with the warning here, but this is
confirming an original suspicion that this warning's implementation is
flawed. I'm looking into how we can implement this more reasonably. WIP
on that front.

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

diff --git a/lib/Sema/SemaExpr.cpp b/lib/Sema/SemaExpr.cpp
index 42ec82ac4a211679aa0df14eaf90af8296f17bdf..5d6291006899596395d90748818e15ef9007b803 100644 (file)
--- a/lib/Sema/SemaExpr.cpp
+++ b/lib/Sema/SemaExpr.cpp
@@ -8945,31 +8945,39 @@ ExprResult Sema::CreateBuiltinBinOp(SourceLocation OpLoc,
   // are mainly cases where the null pointer is used as an integer instead
   // of a pointer.
   if (LeftNull || RightNull) {
-    if (Opc == BO_Mul || Opc == BO_Div || Opc == BO_Rem || Opc == BO_Add ||
-        Opc == BO_Sub || Opc == BO_Shl || Opc == BO_Shr || Opc == BO_And ||
-        Opc == BO_Xor || Opc == BO_Or || Opc == BO_MulAssign ||
-        Opc == BO_DivAssign || Opc == BO_AddAssign || Opc == BO_SubAssign ||
-        Opc == BO_RemAssign || Opc == BO_ShlAssign || Opc == BO_ShrAssign ||
-        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.
-      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();
-      if (LeftNull != RightNull &&
-          !LeftType->isPointerLikeType() &&
-          !LeftType->canDecayToPointerType() &&
-          !RightType->isPointerLikeType() &&
-          !RightType->canDecayToPointerType()) {
+    // Avoid analyzing cases where the result will either be invalid (and
+    // diagnosed as such) or entirely valid and not something to warn about.
+    QualType LeftType = lhs.get()->getType();
+    QualType RightType = rhs.get()->getType();
+    if (!LeftType->isBlockPointerType() && !LeftType->isMemberPointerType() &&
+        !LeftType->isFunctionType() &&
+        !RightType->isBlockPointerType() &&
+        !RightType->isMemberPointerType() &&
+        !RightType->isFunctionType()) {
+      if (Opc == BO_Mul || Opc == BO_Div || Opc == BO_Rem || Opc == BO_Add ||
+          Opc == BO_Sub || Opc == BO_Shl || Opc == BO_Shr || Opc == BO_And ||
+          Opc == BO_Xor || Opc == BO_Or || Opc == BO_MulAssign ||
+          Opc == BO_DivAssign || Opc == BO_AddAssign || Opc == BO_SubAssign ||
+          Opc == BO_RemAssign || Opc == BO_ShlAssign || Opc == BO_ShrAssign ||
+          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.
         Diag(OpLoc, diag::warn_null_in_arithmetic_operation)
-          << (LeftNull ? lhs.get()->getSourceRange()
-                       : rhs.get()->getSourceRange());
+          << (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.
+        if (LeftNull != RightNull &&
+            !LeftType->isAnyPointerType() &&
+            !LeftType->canDecayToPointerType() &&
+            !RightType->isAnyPointerType() &&
+            !RightType->canDecayToPointerType()) {
+          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 78c76e7e4c273b40f681f090622e8b954f0f2608..9665c3959e7c0403a39ae164387a4f844522ff41 100644 (file)
--- a/test/SemaCXX/null_in_arithmetic_ops.cpp
+++ b/test/SemaCXX/null_in_arithmetic_ops.cpp
@@ -4,6 +4,12 @@
 void f() {
   int a;
   bool b;
+  void (^c)();
+  class X;
+  void (X::*d) ();
+  extern void e();
+  int f[2];
+  const void *v;
 
   a = 0 ? NULL + a : a + NULL; // expected-warning 2{{use of NULL in arithmetic operation}}
   a = 0 ? NULL - a : a - NULL; // expected-warning 2{{use of NULL in arithmetic operation}}
@@ -18,6 +24,19 @@ void f() {
   a = 0 ? NULL | a : a | NULL; // expected-warning 2{{use of NULL in arithmetic operation}}
   a = 0 ? NULL ^ a : a ^ NULL; // expected-warning 2{{use of NULL in arithmetic operation}}
 
+  // Check for warnings or errors when doing arithmetic on pointers and other
+  // types.
+  v = 0 ? NULL + &a : &a + NULL; // expected-warning 2{{use of NULL in arithmetic operation}}
+  v = 0 ? NULL + c : c + NULL; // \
+    expected-error {{invalid operands to binary expression ('long' and 'void (^)()')}} \
+    expected-error {{invalid operands to binary expression ('void (^)()' and 'long')}}
+  v = 0 ? NULL + d : d + NULL; // \
+    expected-error {{invalid operands to binary expression ('long' and 'void (X::*)()')}} \
+    expected-error {{invalid operands to binary expression ('void (X::*)()' and 'long')}}
+  v = 0 ? NULL + e : e + NULL; // expected-error 2{{arithmetic on pointer to function type}}
+  v = 0 ? NULL + f : f + NULL; // expected-warning 2{{use of NULL in arithmetic operation}}
+  v = 0 ? NULL + "f" : "f" + NULL; // expected-warning 2{{use of NULL in arithmetic operation}}
+
   // Using two NULLs should only give one error instead of two.
   a = NULL + NULL; // expected-warning{{use of NULL in arithmetic operation}}
   a = NULL - NULL; // expected-warning{{use of NULL in arithmetic operation}}
@@ -65,17 +84,10 @@ void f() {
 
   b = ((NULL)) != a;  // expected-warning{{use of NULL in arithmetic operation}}
 
-  void (^c)();
+  // Check that even non-standard pointers don't warn.
   b = c == NULL || NULL == c || c != NULL || NULL != c;
-
-  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";
 }