InsightSoftwareConsortium
WIP: [Do not merge] Change the default `char` type to unsigned
Just as an experiment!
Triggered by @blowekamp at https://github.com/InsightSoftwareConsortium/ITK/pull/5450#issuecomment-3048788622
itkFrequencyIteratorsGTest test failure (https://open.cdash.org/tests/2103241946):
[ RUN ] FrequencyIterators.Even2D
Unequal images, with 29 unequal pixels
/Users/runner/work/1/s/Modules/Filtering/ImageFrequency/test/itkFrequencyIteratorsGTest.cxx:260: Failure
Value of: fullAndHermitian
Actual: false
Expected: true
I think that can be solved by replacing char with signed char, in the test itself, at https://github.com/InsightSoftwareConsortium/ITK/blob/8de372e88cb836a172786c225cccf7efcea74922/Modules/Filtering/ImageFrequency/test/itkFrequencyIteratorsGTest.cxx#L108
itkHDF5ImageIOTest fails because of a bug (missing unsigned char support) in itkHDF5ImageIO.cxx, I think. Related to this code: https://github.com/InsightSoftwareConsortium/ITK/blob/8de372e88cb836a172786c225cccf7efcea74922/Modules/IO/HDF5/src/itkHDF5ImageIO.cxx#L783-L787 So it treats NATIVE_CHAR different from NATIVE_UCHAR, even when the native (default) char is unsigned (equivalent to unsigned char).
However, I must say, I don't understand exactly what's going on there 🤷
Should this be closed or rebased, given that #5473 was merged?
Thanks for asking, @dzenanz. It's still useful for me to keep this PR open for a little while, although it is never meant to be merged. I believe there are still a few small char/signed char issues in the source tree, which can be revealed by testing ITK with /J (MSVC) or -funsigned-char (GCC), as this PR does.
Should second and third commit be put into a separate PR, meant to be merged?
Should second and third commit be put into a separate PR, meant to be merged?
- OK, for VTKPolyDataMeshIO: pull request #5548
The HDF5ImageIO commit ("ENH: Support H5::PredType::NATIVE_SCHAR...") might be OK as well, but as I still see a failure of itkHDF5ImageIOTest, it may not be good enough yet 🤷