InsightSoftwareConsortium / ITK

Insight Toolkit (ITK) -- Official Repository. ITK builds on a proven, spatially-oriented architecture for processing, segmentation, and registration of scientific images in two, three, or more dimensions.
https://itk.org
Apache License 2.0
1.4k stars 664 forks source link

Vector Warning in Windows Python build #3312

Closed tbirdso closed 2 years ago

tbirdso commented 2 years ago

Description

Compiling ITK Python wrappings on Windows produces a warning about unsigned vector operations. It is unclear what change introduced this warning.

Steps to Reproduce

Compile ITKCommon Python wrappings on Windows.

Expected behavior

No warnings

Actual behavior

...
[2018/3230] Building CXX object Wrapping\Modules\ITKCommon\CMakeFiles\ITKCommonPython.dir\itkVectorPython.cpp.obj
D:\a\1\s\Modules\Core\Common\include\itkVector.hxx(71): warning C4146: unary minus operator applied to unsigned type, result still unsigned
D:\a\1\s\Modules\Core\Common\include\itkVector.hxx(66): note: while compiling class template member function 'itk::Vector<unsigned long,1> itk::Vector<unsigned long,1>::operator -(void) const'
D:\a\1\s-build\Wrapping\Modules\ITKCommon\itkVectorPython.cpp(36974): note: see reference to function template instantiation 'itk::Vector<unsigned long,1> itk::Vector<unsigned long,1>::operator -(void) const' being compiled
D:\a\1\s-build\Wrapping\Modules\ITKCommon\itkVectorPython.cpp(3776): note: see reference to class template instantiation 'itk::Vector<unsigned long,1>' being compiled
[2019/3230] Building CXX object Wrapping\Modules\ITKCommon\CMakeFiles\ITKCommonPython.dir\vnl_c_vectorPython.cpp.obj
...

Reproducibility

100%

Versions

Environment

Windows 10

Additional Information

CDash results

PranjalSahu commented 2 years ago

I will check if it is due to mesh serialization change.

tbirdso commented 2 years ago

It looks like the warning was first picked up on March 16th

https://open.cdash.org/index.php?project=Insight&begin=2021-09-23&end=2022-03-21&filtercount=3&showfilters=1&filtercombine=and&field1=buildwarnings&compare1=43&value1=0&field2=buildname&compare2=63&value2=python&field3=buildname&compare3=63&value3=windows

PranjalSahu commented 2 years ago

Yes, I added new Python wrapping for Vector for UL and ULL in the mesh serialization change. Checking if it can be avoided.

Mesh Serialization

UNIQUE(vector_types "UC;F;D;UL;ULL;${WRAP_ITK_SCALAR}")

tbirdso commented 2 years ago

Strange that the unsigned types warning did not appear for the unsigned char type. When I rebuild without UL and ULL I do not see the warnings, though.

I think I would be in favor of suppressing and ignoring this warning. Users are already expected to be responsible for avoiding overflow with unsigned types such as itk.UL and the same responsibility should apply when working with vectors. However I would appreciate an additional perspective from @dzenanz or @thewtex .

If we go this route we can suppress the warning by adding a regex string to CTEST_CUSTOM_WARNING_EXCEPTION in CTestCustom.cmake. We currently do this to suppress a similar warning that appears from VNL vector wrappings.

PranjalSahu commented 2 years ago

I am building it without those additional wrappings however, it failed previously. If we can't do without those wrappings then yes we have to do something similar to what you are suggesting.

thewtex commented 2 years ago

Green again for Windows Python!

tbirdso commented 2 years ago

Thank you @PranjalSahu !