[cfe-commits] r45951 - in /cfe/trunk: AST/Expr.cpp test/Sema/conditional-expr.c

[Chris Lattner]

On Jan 13, 2008, at 6:53 PM, Steve Naroff wrote:
> URL: http://llvm.org/viewvc/llvm-project?rev=45951&view=rev
> Log:
>
> Rewrite Expr::isNullPointerConstant() to deal with multiple levels  
> of explicit casts.
>
> Now, isNullPointerConstant() will return true for the following:  
> "(void*)(double*)0"

Unfortunately, this violates the C spec.  :(

How about only accepting this with -pedantic?

Some specific things about the patch:

> +/// Helper function for isNullPointerConstant. This routine skips all
> +/// explicit casts, implicit casts and paren expressions.
> +const Expr * getNullPointerConstantExpr(const Expr *exp) {

Ok, you could make it iterative, but not a big deal.  Please rename  
this to "LookThroughCasts" or something like that, as the function has  
nothing to do with null (the name should reflect what it does, not who  
the client is).  Should it only look through casts to pointer type?   
We don't want to accept something like (void*)(int)(float)0 as a null  
pointer constant.

> /// isNullPointerConstant - C99 6.3.2.3p3 -  Return true if this is  
> either an
> /// integer constant expression with the value zero, or if this is  
> one that is
> /// cast to void*.
> bool Expr::isNullPointerConstant(ASTContext &Ctx) const {
> +  const CastExpr *CE = dyn_cast<CastExpr>(this);
> +  const Expr *e = getNullPointerConstantExpr(this);

This implementation is overly complex.  I'd eliminate "CE" entirely,  
and change the code to be:

const Expr *e = getNullPointerConstantExpr(this);
if (!e->isinteger) return false;

return true see if it's an integer constant expr with value 0.

Isn't that sufficient?

-Chris