[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 Fri, Jan 11, 2013 at 11:32 AM, jahanian <fjahanian at apple.com> wrote:

>
> On Jan 10, 2013, at 6:39 PM, Richard Smith <richard at metafoo.co.uk> wrote:
>
> On Thu, Jan 10, 2013 at 12:31 PM, jahanian <fjahanian at apple.com> wrote:
>
>
> On Jan 9, 2013, at 5:58 PM, Richard Smith <richard at metafoo.co.uk> wrote:
>
>
>
> I want to issue warning when case value overflows. EvaluateKnownConstInt
> detects
> this by returning a note and callers are expected to issue the warning
> based on the note returned.
> Similar case is handled at end of CheckConvertedConstantExpression.
>
>
> Well, in CheckConvertedConstantExpression, a diagnostic is produced
> any time the evaluation produces notes. In this case, we only issue a
> diagnostic for one of the notes. I'm concerned that this code would be
> broken if we split that note into multiple notes with more details for
> some cases -- and, in fact, there already is such a separate note for
> signed left shift overflow.
>
>
> There are many notes. But there is one note_constexpr_overflow for the
> overflows.
> Are you suggesting something different than checking for this note?
>  (Maybe you are suggesting that we separately check for overflow condition
> only?).
>

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.

> Have you thought about always warning when an expression contains an
> overflowing calculation, not only when such an overflow appears in a
> case expression (like we already do for left shifts, for instance)?
>
>
> Are you suggesting moving the warning to individual operations;
> CheckAdditionOperands,
> CheckMultiplyDivideOperands and the like? left shift diagnostic
> is done in CheckShiftOperands.
>

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.


> 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/20130111/da311043/attachment.html>