r177162 - c: add the missing binary operatory when checking

[David Blaikie]

On Fri, Mar 15, 2013 at 10:11 AM, jahanian <fjahanian at apple.com> wrote:
>
> On Mar 15, 2013, at 10:10 AM, David Blaikie <dblaikie at gmail.com> wrote:
>
> On Fri, Mar 15, 2013 at 9:36 AM, Fariborz Jahanian <fjahanian at apple.com>
> wrote:
>
> Author: fjahanian
> Date: Fri Mar 15 11:36:04 2013
> New Revision: 177162
>
> URL: http://llvm.org/viewvc/llvm-project?rev=177162&view=rev
> Log:
> c: add the missing binary operatory when checking
> for integer overflow. // rdar://13423975
>
> Modified:
>    cfe/trunk/lib/Sema/SemaChecking.cpp
>    cfe/trunk/test/Sema/switch-1.c
>
> Modified: cfe/trunk/lib/Sema/SemaChecking.cpp
> URL:
> http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/SemaChecking.cpp?rev=177162&r1=177161&r2=177162&view=diff
> ==============================================================================
> --- cfe/trunk/lib/Sema/SemaChecking.cpp (original)
> +++ cfe/trunk/lib/Sema/SemaChecking.cpp Fri Mar 15 11:36:04 2013
> @@ -5188,7 +5188,7 @@ void Sema::CheckImplicitConversions(Expr
> void Sema::CheckForIntOverflow (Expr *E) {
>   if (const BinaryOperator *BExpr =
> dyn_cast<BinaryOperator>(E->IgnoreParens())) {
>     unsigned Opc = BExpr->getOpcode();
> -    if (Opc != BO_Add && Opc != BO_Sub && Opc != BO_Mul)
> +    if (Opc != BO_Add && Opc != BO_Sub && Opc != BO_Mul && Opc != BO_Div)
>       return;
>     llvm::SmallVector<PartialDiagnosticAt, 4> Diags;
>     E->EvaluateForOverflow(Context, &Diags);
>
> Modified: cfe/trunk/test/Sema/switch-1.c
> URL:
> http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Sema/switch-1.c?rev=177162&r1=177161&r2=177162&view=diff
> ==============================================================================
> --- cfe/trunk/test/Sema/switch-1.c (original)
> +++ cfe/trunk/test/Sema/switch-1.c Fri Mar 15 11:36:04 2013
> @@ -1,6 +1,7 @@
> // RUN: %clang_cc1 -fsyntax-only -verify -triple x86_64-apple-darwin10 %s
> // RUN: %clang_cc1 -x c++ -fsyntax-only -verify -triple
> x86_64-apple-darwin10 %s
> // rdar://11577384
> +// rdar://13423975
>
> int f(int i) {
>   switch (i) {
> @@ -10,6 +11,8 @@ int f(int i) {
>       return 2;
>     case (123456 *789012) + 1:  // expected-warning {{overflow in
> expression; result is -1375982336 with type 'int'}}
>       return 3;
> +    case (2147483647*4)/4:     // expected-warning {{overflow in
> expression; result is -4 with type 'int'}}
>
>
> Yeah, I'm with Jordan here - why are we warning about the division.
> Division can't cause overflow. Shouldn't we be warning about that
> multiplication?
>
>
> We are warning about multiplication.

Then why are we selecting which binary operators to "see" through?
Which expression would we not want to see through to perform this
warning?