Home All Groups Group Topic Archive Search About

"select case" statement optimization in VB and C# problems

Author
1 Jun 2006 3:33 PM
Dave Markle
Good afternoon.  I was just going through my code, analyzing it with FXCop,
and FxCop gave me the following error on this code:

MY CODE:
        Select Case termYears
            Case 5 : retVal.Append("1")
            Case 7 : retVal.Append("2")
            Case 10 : retVal.Append("3")
            Case 15 : retVal.Append("4")
            Case 20 : retVal.Append("5")
            Case 30 : retVal.Append("6")
            Case Else
                Throw New ArgumentException("Term (in years) not recognized!")
        End Select

FXCOP's ERROR:
* Operations should not overflow.
   Correct the potential overflow in the operation 'settleMonth-10' in (my
function).

So I found this to be QUITE a curious result, of course, since I wasn't
performing a subtraction on the code.  I looked into the documentation of the
"Select Case" statement, and I didn't find anything about random overflow
exceptions...

So I read this article:
http://weblogs.asp.net/justin_rogers/archive/2004/03/25/95806.aspx
which was quite good.  And I ran Reflector on my code and disassembled it. 
Sure enough, the code disassembled (and reassembled into C# by reflector) to:

REFLECTOR'S GENERATED CODE:
      switch (((short) (termYears - 5)))
      {
            case 0:
                  builder1.Append("1");
                  break;

            case 2:
                  builder1.Append("2");
                  break;

            case 5:
                  builder1.Append("3");
                  break;

            case 10:
                  builder1.Append("4");
                  break;

            case 15:
                  builder1.Append("5");
                  break;

            case 0x19:
                  builder1.Append("6");
                  break;
      }

OK, now that I've set the scene for my question, I'll ask it.  HOW IN THE
WORLD IS THIS NOT A MAJOR DESIGN BUG IN THE .NET COMPILER?  There's NO way I,
the developer, can know that my code's going to throw an OverflowException if
my code gets short.MinValue for the termYears parameter.  Anyone reading my
original code should be expecting ArgumentException, but instead, they'll get
this mysterious OverflowException?  Is this going to be fixed somehow?  I
understand that this is a speed optimization, but you CAN'T make
optimizations under the covers which change code semantics! 

Thanks a bunch.  I'd appreciate a (well thought out, not dismissive) reply
by someone the VB/.NET/C# team as soon as possible.  IMO this is potentially
a major issue.

-Dave

Author
1 Jun 2006 11:13 PM
Mattias Sjögren
Dave,

>REFLECTOR'S GENERATED CODE:
>      switch (((short) (termYears - 5)))
[...]
>OK, now that I've set the scene for my question, I'll ask it.  HOW IN THE
>WORLD IS THIS NOT A MAJOR DESIGN BUG IN THE .NET COMPILER?  There's NO way I,
>the developer, can know that my code's going to throw an OverflowException if
>my code gets short.MinValue for the termYears parameter. Anyone reading my
>original code should be expecting ArgumentException, but instead, they'll get
>this mysterious OverflowException?

Did you try that? Did you actually get an OverflowException? I always
get the ArgumentException as expected.

If we ignore the Reflector output and look at the IL produced, I see
the following generated by all VB compiler versions I have, after
compiling the code you posted

ldloc.1        // load termYears
ldc.i4.5
sub
switch ( ...

Note that the sub instruction is used, regardless of your
/removeintchecks setting. The sub instruction never throws, it simply
overflows the result. Plus it works on 32 bit arguments or bigger, so
even if termYears is Int16.MinValue, the result will be Int16.MinValue
- 5 stored as a Int32. The switch instruction then interprets this
value as unsigned, and it simply doesn't match any of the cases.

I don't know where Reflector got the truncating cast to short from,
it's not represented in the IL I was able to generate. A more correct
decompiled version would look like this

switch (unchecked(termYears - 5))


Mattias

--
Mattias Sjögren [C# MVP]  mattias @ mvps.org
http://www.msjogren.net/dotnet/ | http://www.dotnetinterop.com
Please reply only to the newsgroup.
Author
2 Jun 2006 1:45 PM
Dave Markle
Mattias:

You're absolutely right, thanks.  Btw, my termYears variable was a "short". 
FxCop was leading me astray.  I kneel in deference to your mad IL skills.  I
guess this one is for the FXCop team as FxCop thinks that it's a checked
operation.

-Dave

Show quoteHide quote
"Mattias Sjögren" wrote:

> Dave,
>
> >REFLECTOR'S GENERATED CODE:
> >      switch (((short) (termYears - 5)))
> [...]
> >OK, now that I've set the scene for my question, I'll ask it.  HOW IN THE
> >WORLD IS THIS NOT A MAJOR DESIGN BUG IN THE .NET COMPILER?  There's NO way I,
> >the developer, can know that my code's going to throw an OverflowException if
> >my code gets short.MinValue for the termYears parameter. Anyone reading my
> >original code should be expecting ArgumentException, but instead, they'll get
> >this mysterious OverflowException?
>
> Did you try that? Did you actually get an OverflowException? I always
> get the ArgumentException as expected.
>
> If we ignore the Reflector output and look at the IL produced, I see
> the following generated by all VB compiler versions I have, after
> compiling the code you posted
>
> ldloc.1        // load termYears
> ldc.i4.5
> sub
> switch ( ...
>
> Note that the sub instruction is used, regardless of your
> /removeintchecks setting. The sub instruction never throws, it simply
> overflows the result. Plus it works on 32 bit arguments or bigger, so
> even if termYears is Int16.MinValue, the result will be Int16.MinValue
> - 5 stored as a Int32. The switch instruction then interprets this
> value as unsigned, and it simply doesn't match any of the cases.
>
> I don't know where Reflector got the truncating cast to short from,
> it's not represented in the IL I was able to generate. A more correct
> decompiled version would look like this
>
> switch (unchecked(termYears - 5))
>
>
> Mattias
>
> --
> Mattias Sjögren [C# MVP]  mattias @ mvps.org
> http://www.msjogren.net/dotnet/ | http://www.dotnetinterop.com
> Please reply only to the newsgroup.
>