On Thu, May 26, 2011 at 3:10 AM, Chandler Carruth <[email protected]>wrote:
> On Mon, May 2, 2011 at 5:48 PM, Richard Trieu <[email protected]> wrote:
>
>> Add warning when NULL constant value is used improperly in expressions.
>>
>>
> Some high-level comments:
>
> 1) Why only handle GNUNull null-pointer-constants? I think there is a good
> reason here (making them behave as much like nullptr as possible) but it
> would be good to document that in comments at least.
>
The other kinds of nulls are zero integer and c++0x nullptr. Integral zero
is valid for these operations. nullptr is a different type which already
produces an error in these cases.
>
> 2) What drives the set of invalid operations? I assume its those that
> nullptr would be invalid for? If so, can you provide standard citations
> against the draft standard? Also, is there no way to just turn on the C++0x
> errors for nullptr when we see the GNUNull in C++03, but down-grade them to
> warnings?
>
These operations are the ones where the null pointer would be used as an
integer instead of a pointer. I've also copied the test, but using nullptr
instead to show that they error in same places.
It should be possible to change the NULL into a nullptr and then run the
checks, but that would probably involve touching code in all the of
operation checking functions. I feel that it would be better to keep this
code in one place instead of spread across so many functions.
>
> 3) Because these are warnings, we can't return ExprError; that changes the
> parse result.
>
Removed the early exit with ExprError.
Index: test/SemaCXX/null_in_arithmetic_ops.cpp
===================================================================
--- test/SemaCXX/null_in_arithmetic_ops.cpp (revision 0)
+++ test/SemaCXX/null_in_arithmetic_ops.cpp (revision 0)
@@ -0,0 +1,49 @@
+// RUN: %clang_cc1 -fsyntax-only -verify %s
+#include <stddef.h>
+
+void f() {
+ int a;
+ bool b;
+
+ 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}}
+ a = 0 ? NULL / a : a / NULL; // expected-warning 2{{use of NULL in arithmetic operation}} \
+ // expected-warning {{division by zero is undefined}}
+ 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}}
+ 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}} \
+ expected-warning {{remainder by zero is undefined}}
+ 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}}
+ a = 0 ? NULL ^ a : a ^ NULL; // expected-warning 2{{use of NULL in arithmetic operation}}
+
+ a += NULL; // expected-warning{{use of NULL in arithmetic operation}}
+ a -= NULL; // expected-warning{{use of NULL in arithmetic operation}}
+ a /= NULL; // expected-warning{{use of NULL in arithmetic operation}} \
+ // expected-warning{{division by zero is undefined}}
+ a *= NULL; // expected-warning{{use of NULL in arithmetic operation}}
+ a >>= NULL; // expected-warning{{use of NULL in arithmetic operation}}
+ a <<= NULL; // expected-warning{{use of NULL in arithmetic operation}}
+ a %= NULL; // expected-warning{{use of NULL in arithmetic operation}} \
+ // expected-warning{{remainder by zero is undefined}}
+ a &= NULL; // expected-warning{{use of NULL in arithmetic operation}}
+ a |= NULL; // expected-warning{{use of NULL in arithmetic operation}}
+ a ^= NULL; // expected-warning{{use of NULL in arithmetic operation}}
+
+ b = a < NULL || NULL < a; // expected-warning 2{{use of NULL in arithmetic operation}}
+ b = a > NULL || NULL > a; // expected-warning 2{{use of NULL in arithmetic operation}}
+ b = a <= NULL || NULL <= a; // expected-warning 2{{use of NULL in arithmetic operation}}
+ b = a >= NULL || NULL >= a; // expected-warning 2{{use of NULL in arithmetic operation}}
+ b = a == NULL || NULL == a; // expected-warning 2{{use of NULL in arithmetic operation}}
+ b = a != NULL || NULL != a; // expected-warning 2{{use of NULL in arithmetic operation}}
+
+ b = &a < NULL || NULL < &a || &a > NULL || NULL > &a;
+ b = &a <= NULL || NULL <= &a || &a >= NULL || NULL >= &a;
+ b = &a == NULL || NULL == &a || &a != NULL || NULL != &a;
+
+ b = 0 == a;
+ b = 0 == &a;
+
+ b = ((NULL)) != a; // expected-warning{{use of NULL in arithmetic operation}}
+}
Index: include/clang/Basic/DiagnosticSemaKinds.td
===================================================================
--- include/clang/Basic/DiagnosticSemaKinds.td (revision 130739)
+++ include/clang/Basic/DiagnosticSemaKinds.td (working copy)
@@ -2699,6 +2699,8 @@
def warn_comparison_of_mixed_enum_types : Warning<
"comparison of two values with different enumeration types (%0 and %1)">,
InGroup<DiagGroup<"enum-compare">>;
+def warn_null_in_arithmetic_operation : Warning<
+ "use of NULL in arithmetic operation">;
def err_invalid_this_use : Error<
"invalid use of 'this' outside of a nonstatic member function">;
Index: lib/Sema/SemaExpr.cpp
===================================================================
--- lib/Sema/SemaExpr.cpp (revision 130739)
+++ lib/Sema/SemaExpr.cpp (working copy)
@@ -8570,6 +8570,45 @@
rhs = move(resolvedRHS);
}
+ bool LeftNull = Expr::NPCK_GNUNull ==
+ lhs.get()->isNullPointerConstant(Context,
+ Expr::NPC_ValueDependentIsNotNull);
+ bool RightNull = Expr::NPCK_GNUNull ==
+ rhs.get()->isNullPointerConstant(Context,
+ Expr::NPC_ValueDependentIsNotNull);
+
+ // Detect when a NULL constant is used improperly in an expression.
+ 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.
+ 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();
+ } 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) && !lhs.get()->getType()->isPointerType() &&
+ !rhs.get()->getType()->isPointerType()) {
+ 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();
+ }
+ }
+ }
+
switch (Opc) {
case BO_Assign:
ResultTy = CheckAssignmentOperands(lhs.get(), rhs, OpLoc, QualType());
_______________________________________________
cfe-commits mailing list
[email protected]
http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
