[cfe-commits] r173377 - in /cfe/trunk: include/clang/AST/Expr.h include/clang/Basic/DiagnosticASTKinds.td include/clang/Basic/DiagnosticSemaKinds.td include/clang/Sema/Sema.h lib/AST/ExprConstant.cpp lib/Sema/SemaChecking.cpp lib/Sema/SemaDecl.cpp lib/Sema/SemaExprCXX.cpp lib/Sema/SemaStmt.cpp test/Sema/switch-1.c

[John McCall]

On Jan 24, 2013, at 3:33 PM, jahanian <fjahanian at apple.com> wrote:
> On Jan 24, 2013, at 3:27 PM, John McCall <rjmccall at apple.com> wrote:
>> On Jan 24, 2013, at 3:11 PM, Richard Smith <richard at metafoo.co.uk> wrote:
>>> On Thu, Jan 24, 2013 at 2:11 PM, Fariborz Jahanian <fjahanian at apple.com> wrote:
>>>> Author: fjahanian
>>>> Date: Thu Jan 24 16:11:45 2013
>>>> New Revision: 173377
>>>> 
>>>> URL: http://llvm.org/viewvc/llvm-project?rev=173377&view=rev
>>>> Log:
>>>> Patch to check for integer overflow. It has been
>>>> commented on and approved by Richard Smith.
>>>> 
>>>> Modified:
>>>>  cfe/trunk/include/clang/AST/Expr.h
>>>>  cfe/trunk/include/clang/Basic/DiagnosticASTKinds.td
>>>>  cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td
>>>>  cfe/trunk/include/clang/Sema/Sema.h
>>>>  cfe/trunk/lib/AST/ExprConstant.cpp
>>>>  cfe/trunk/lib/Sema/SemaChecking.cpp
>>>>  cfe/trunk/lib/Sema/SemaDecl.cpp
>>>>  cfe/trunk/lib/Sema/SemaExprCXX.cpp
>>>>  cfe/trunk/lib/Sema/SemaStmt.cpp
>>>>  cfe/trunk/test/Sema/switch-1.c
>>>> 
>>>> Modified: cfe/trunk/include/clang/AST/Expr.h
>>>> URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/AST/Expr.h?rev=173377&r1=173376&r2=173377&view=diff
>>>> ==============================================================================
>>>> --- cfe/trunk/include/clang/AST/Expr.h (original)
>>>> +++ cfe/trunk/include/clang/AST/Expr.h Thu Jan 24 16:11:45 2013
>>>> @@ -570,6 +570,9 @@
>>>> /// integer.
>>>> llvm::APSInt EvaluateKnownConstInt(const ASTContext &Ctx,
>>>>                         SmallVectorImpl<PartialDiagnosticAt> *Diag=0) const;
>>>> +
>>>> +  void EvaluateForOverflow(const ASTContext &Ctx,
>>>> +                           SmallVectorImpl<PartialDiagnosticAt> *Diag) const;
>>>> 
>>>> /// EvaluateAsLValue - Evaluate an expression to see if we can fold it to an
>>>> /// lvalue with link time known address, with no side-effects.
>>>> 
>>>> Modified: cfe/trunk/include/clang/Basic/DiagnosticASTKinds.td
>>>> URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Basic/DiagnosticASTKinds.td?rev=173377&r1=173376&r2=173377&view=diff
>>>> ==============================================================================
>>>> --- cfe/trunk/include/clang/Basic/DiagnosticASTKinds.td (original)
>>>> +++ cfe/trunk/include/clang/Basic/DiagnosticASTKinds.td Thu Jan 24 16:11:45 2013
>>>> @@ -106,6 +106,9 @@
>>>> "(skipping %0 call%s0 in backtrace; use -fconstexpr-backtrace-limit=0 to "
>>>> "see all)">;
>>>> def note_constexpr_call_here : Note<"in call to '%0'">;
>>>> +def warn_integer_constant_overflow : Warning<
>>>> +  "overflow in case constant expression results in value %0 of type %1">,
>>>> +  InGroup<DiagGroup<"integer-overflow">>;
>>> 
>>> This diagnostic text is wrong, since we're checking for overflow in
>>> all expressions, not just case constant expressions.
>> 
>> It's also confusingly worded.  A better diagnostic, if we keep a specialized
>> message for case expressions, would be something like:
>> "overflow in 'case' expression;  resulting value is %0 of type %1"
>> In particular, the quotation marks are importing in order to prevent
>> people from interpreting "in case ..." as the common English idiom.
>> It is also unnecessary to point out that this is a constant expression.
> 
> No longer specialized to case expression.
> So, I need to mention that it is a constant expression.

Okay.

> Currently it says. overflow of constant expression results in value %0 of type %1
> I think your suggesting would be something like:
> 
> "overflow of constant expression;  resulting  value is %0 of type %1"

Definitely use "in" instead of "of".  It's too bad that the type is important;
it's awkward to fit in because we want to say the value first.  Let's go with:
  "overflow in constant expression; result is %0 with type %1"

So that would give us something like this:
  warning: overflow in constant expression; result is 3 with type 'uint8_t' (aka 'unsigned char')

That works for me.

John.