[cfe-commits] r172016 - in /cfe/trunk: include/clang/AST/Expr.h lib/AST/ExprConstant.cpp lib/Sema/SemaStmt.cpp test/Sema/switch-1.c

[Richard Smith]

On Mon, Jan 14, 2013 at 12:23 PM, jahanian <fjahanian at apple.com> wrote:

>
> On Jan 11, 2013, at 2:55 PM, Richard Smith <richard at metafoo.co.uk> wrote:
>
> There are two different kinds of overflow that can happen in a case
> expression:
>
> 1) An overflow occurs while evaluating the case value:
>
> switch (n)
>   case 123456 * 123456:
>
> 2) An overflow occurs while converting the evaluated case value to the
> promoted type of the switch expression:
>
> int n = /*...*/;
> switch (n)
>   case 1ULL << 45:
>
> For situation (1), it doesn't seem pertinent that this is happening within
> a case expression; we should warn no matter where it happens; this is the
> focus of PR2729. (2) should already be being checked by the call to
> ConvertIntegerToTypeWarnOnOverflow.
>
>
> This Patch checks and warns for overflow.
>

Thanks!

> Yes, that would make sense to me, although we will need some mechanism to
> suppress the warning in the case where the overflow produces an error (as
> happens in constant expressions in C++11), and we'd need to avoid
> repeatedly evaluating subexpressions.
>
>
> I am afraid I don't see how  this warning can be avoided in case of
> constant expressions in C++11. Expression evaluations are disjoint from
> declarations specs.
> For example, clang already issues this warning (and error for constant
> expressions ) in the case of left-shift operations.
> constexpr int ll = (2147483647 << 2);
>
> Can't we have the additional warning? Please advise.
>

I think the additional warning is fine for the time being. In the longer
term, we could perform this check in ActOnFinishFullExpr (alongside the
call to CheckImplicitConversions), and suppress it if the full expression
is required to be a constant expression.

Does this patch have acceptable performance for large constant expressions?
I'm a little concerned that it appears it will repeatedly evaluate
subexpressions (the constant expression evaluator doesn't perform any
caching).

- Fariborz
>
>
>
>
>
>> Also, I changed the warning per your other comments.
>> In r172102.
>>
>>
>> Thanks! The "results in a new value" wording seems a bit awkward, how
>> about just saying "overflow in case constant expression results in
>> value %0"?
>>
>>
>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20130114/173eb9f4/attachment.html>