Compilation error while using itk::RelabelComponentImageFilte::GetSizeOfObjectInPhysicalUnits

[kenavolic]

Description

Using the itkRelabelComponentImageFilter filter GetSizeOfObjectInPhysicalUnits with signed integer image causes a compilation error due comparison of integer expressions of different signedness.

Steps to Reproduce

1. Install & build ITK (master)
2. Build test snippet

Expected behavior

Should be able to use the class method

Actual behavior

Compilation error:

In file included from /home/anon/Dev/test_itk/main.cpp:2:
/home/anon/Dev/ITK/Modules/Segmentation/ConnectedComponents/include/itkRelabelComponentImageFilter.h: In instantiation of ‘float itk::RelabelComponentImageFilter<TInputImage, TOutputImage>::GetSizeOfObjectInPhysicalUnits(itk::RelabelComponentImageFilter<TInputImage, TOutputImage>::LabelType) const [with TInputImage = itk::Image<short int, 3>; TOutputImage = itk::Image<short int, 3>; itk::RelabelComponentImageFilter<TInputImage, TOutputImage>::LabelType = short int]’:
/home/anon/Dev/test_itk/main.cpp:13:46:   required from here
/home/anon/Dev/ITK/Modules/Segmentation/ConnectedComponents/include/itkRelabelComponentImageFilter.h:228:24: error: comparison of integer expressions of different signedness: ‘itk::RelabelComponentImageFilter<itk::Image<short int, 3>, itk::Image<short int, 3> >::LabelType’ {aka ‘short int’} and ‘const SizeValueType’ {aka ‘const long unsigned int’} [-Werror=sign-compare]
  228 |     if (obj > 0 && obj <= m_NumberOfObjects)

Reproducibility

Always (under the same compilation flags)

Versions

Current developpement version and older ones (detected on ITK5.2)

Environment

• Ubuntu 18.04
• gcc 10.3
• cmake 3.25

Additional Information

Link to discourse discussion

[dzenanz]

Now I understand why no one encountered this before. The pixel type of input to relabel filter is usually an output of connected components filter, which is unsigned (and usually large) integral type. And if that type is not large enough to hold the number of generated labels, I think there is an exception being raised.

[jhlegarreta]

@kenavolic thanks for the bug report, the test code and the fix !

I wanted to test this locally on my MVSC 2022 compiler and found that using your gist test code the warning would not be raised. I digged a little bit into my compiler flags and saw that I had the /W3 warning flag set by default. I changed it to /Wall, and the warning was indeed raised.

Not only were these warnings reported, but dozens of other warnings were also reported. I did not go through them.

So @dzenanz @thewtex :

• Would it be useful to activate that flag for the build robots that report to the dashboard and the Azure CI configuration machines so that these warnings do not go under the radar? Equivalent flags may be needed for gcc and clang, as it seems like even if they are stricter than MSVC, they still do not capture a number of warnings.
• Eventually, make warnings be reported as errors (at least for the GitHub CIs) since otherwise the GitHub CIs report success?

[dzenanz]

We don't have a lot of resources, so forcing all contributions to resolve ALL the possible warnings would be raising a bar way too high, at least in my opinion. We could enable more warnings for some of the dashboard builds, to raise awareness about them.

[jhlegarreta]

We could enable more warnings for some of the dashboard builds, to raise awareness about them.

That way at least we would be aware of the warnings, they would be openly visible, and could be addressed by anyone having the time and interest.

[dzenanz]

I added /Wall to Windows11-VS22x64-RelWithDebInfo-Favorite-Remotes.

[jhlegarreta]

I added /Wall to Windows11-VS22x64-RelWithDebInfo-Favorite-Remotes.

Thanks for doing this Dženan. Not sure if I see any new warning in the dashboard:

https://open.cdash.org/builds/9951189

Trying it locally would be good on one of your machines to see that there is additional warnings that are raised. I will try to post my CMakeCache.txt to see if that helps explaining the difference.

[dzenanz]

I tried it locally, only to see no new warnings.

[dzenanz]

Suppressing warnings about LabelGeometryImageFilter, reveals some interesting warnings here:
https://open.cdash.org/viewBuildError.php?type=1&buildid=9952121
of the style: warning: iteration 2 invokes undefined behavior [-Waggressive-loop-optimizations]. Whoever has time could look into it.

[jhlegarreta]

Moving this conversation to issue #4877.