[cfe-dev] Constant expression checking rewrite

[Ted Kremenek]

On May 18, 2008, at 10:09 AM, Eli Friedman wrote:

> Essentially, yes; that's the biggest issue.  Implementing this stuff
> outside of Sema would require a complicated return convention for
> reasonable diagnostics. It might end up being the best approach, but
> it's a lot more complicated.

Understood.  The patch as you have also may just be an intermediate  
design to a final solution, as there is also the issue of your last  
point about undefined operations) where one may need to (partially)  
compute constant values (see below).

> Also, there's really a few different kinds of constant expressions.
> First, there are integer constant expressions, which are precisely
> defined in the C99 standard to be essentially integers and integer
> arithmetic.
>
> Then, there are general constant expressions, also defined in the C99
> standard, which are essentially either arithmetic constants (computed
> with integer/floating-point arithmetic), or an easily computable
> address (the details are slightly more complicated).  This is what my
> patch implements.
>
> Then, there is what Expr::isConstantExpr returns, which isn't exactly
> clear; it's currently buggy, but it apparently tries to compute
> whether an expression is constant in the sense that it doesn't depend
> on the values of any variables/globals.  This could potentially be
> useful, but it's not acceptable for the Sema checking; we cannot
> accept all expressions in this category as constant expressions
> because some of them might not be computable by the linker.

That is an extremely lucid answer.  Thank you!

> One issue with my patch I haven't mentioned: it doesn't try to check
> whether an expression has a defined result when doing
> constant-expression checking.  Unfortunately, this will significantly
> complicate the code, and I'm not sure what the best approach is yet
> (maybe leaving it to an analysis pass would be best).  I wouldn't
> normally worry about that sort of thing, but division by zero in a
> global currently crashes llc, so it would be best if we emitted a hard
> error.

Forgive my naivete, could Expr::isIntegerConstantExpr be used for some  
of these purposes?  It seems like there are cases where you actually  
want to compute the constant value to do such checking, and this might  
be a valid approach (although I can see where it would be  
incomplete).  I'm not certain if it would satisfy all the restrictions  
of a C99 constant expression.

Alternatively, it does seem to me that to catch such cases in general  
one would need to recursively evaluate the subexpressions, compute the  
actual constants (when necessary), and see in what cases where an  
undefined result could occur (e.g., divide-by-zero, shift by too many  
bits).  The checking done in your patch is written in a recursive  
fashion; it seems like the evaluation of the constant values could be  
done in this way also.  I think the main complication it would add to  
the code is that it would create several more recursive methods to do  
the checking (e.g., one for checking integer binary operations,  
floating point binary operations, pointer arithmetic, etc., instead of  
just having a single case for BinaryOperator).  Such logic would not  
be all that different from what is going on in the static analysis  
engine when evaluating constant values along paths.