[vtk-developers] Help with 2 warnings in TestFastNumericConversion.cxx and vtkDistributedGraphHelper.cxx

[Sean McBride]

David,

Do I understand you are volunteering to remove the old FastNumericConversion crud?  I've enabled this warning on Rogue7:

<http://open.cdash.org/viewBuildError.php?type=1&buildid=2870795>

Sean



On Wed, 3 Apr 2013 13:59:23 -0600, David Gobbi said:

>All the FastNumericConversion stuff is obsolete and it would probably
>be easier to remove it than to fix it.  It's a bunch of tricks that are
>specifically for the i387 FPU but which do more harm than good on any
>CPU manufactured in the last 10 years.  There are a couple places in
>volume rendering where this stuff is still used, but I think that I
>can easily patch them up.
>
> - David
>
>On Wed, Apr 3, 2013 at 1:47 PM, Sean McBride <sean at rogue-research.com> wrote:
>> Hi all,
>>
>> I've been working on enabling more clang warnings on my dashboard...
>And need help with:
>>
>> 1)
>> Common/Math/Testing/Cxx/TestFastNumericConversion.cxx:55:18: warning:
>dereference of type 'int *' that was reinterpret_cast from type 'double
>*' has undefined behavior [-Wundefined-reinterpret-cast]
>>       ival[i] = *reinterpret_cast<int *>(&dval[i]);
>>                  ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
>>
>> This is indeed wonky.  As 'double' is usually 64 bit and 'int' is
>usually 32, this is reinterpreting only half the bits of the double
>(which half depending on CPU endianness).  It does this in a big loop,
>times it, and uses that as a baseline time later compared to other
>techniques.  The test's entire foundation is suspect... what to do?
>>
>> 2)
>> Common/DataModel/vtkDistributedGraphHelper.cxx:191:49: warning: signed
>shift result (0x80000000) sets the sign bit of the shift expression's type
>>       ('vtkIdType' (aka 'int')) and becomes negative [-Wshift-sign-overflow]
>>   this->signBitMask = static_cast<vtkIdType>(1) << ((sizeof(vtkIdType)
>* CHAR_BIT) - 1);
>>                       ~~~~~~~~~~~~~~~~~~~~~~~~~ ^ 
>~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
>>
>> Shifting a 1 bit into or past the sign bit is undefined behaviour.  If
>I'm not mistaken, we can just use VTK_ID_MIN here, right?