On Jun 13, 2011, at 5:00 PM, Richard Trieu wrote:
>
>
> 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.
+ 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) {
I think you want:
if (LeftNull != RightNull) {
here? At the very least, we shouldn't emit two warnings when both sides of the
operator are NULL.
+ // 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()) {
You also need to check for member pointers, block pointers, and Objective-C
pointers here.
- Doug
_______________________________________________
cfe-commits mailing list
[email protected]
http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
