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

[jahanian]

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?).
> 
> 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.

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