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.41k stars 663 forks source link

MetaIO 2024-02-12 (9a8aac79) #4461

Closed dzenanz closed 7 months ago

dzenanz commented 7 months ago

Code extracted from:

https://github.com/Kitware/MetaIO.git

at commit 9a8aac793215c8a0bbecfd60e10262a5846eba4a (master).

PR Checklist

dzenanz commented 7 months ago

@seanm A test in ITK need to be updated too. Can you do that? E.g. add another commit on top of this PR.

FAILED: Modules/ThirdParty/MetaIO/src/MetaIO/src/tests/CMakeFiles/testMeta2Object.dir/testMeta2Object.cxx.o 
/usr/bin/c++  -I/home/vsts/work/1/s-build/Modules/ThirdParty/ZLIB/src -I/home/vsts/work/1/s-build/Modules/ThirdParty/ZLIB/src/itkzlib-ng -I/home/vsts/work/1/s/Modules/ThirdParty/ZLIB/src -I/home/vsts/work/1/s-build/Modules/ThirdParty/MetaIO/src/MetaIO/src -I/home/vsts/work/1/s/Modules/ThirdParty/MetaIO/src/MetaIO/src -I/home/vsts/work/1/s/Modules/ThirdParty/ZLIB/src/itkzlib-ng -mtune=generic -march=corei7 -Wall -Wcast-align -Wdisabled-optimization -Wextra -Wformat=2 -Winvalid-pch -Wno-format-nonliteral -Wpointer-arith -Wshadow -Wunused -Wwrite-strings -Wno-strict-overflow -Wno-deprecated -Wno-invalid-offsetof -Woverloaded-virtual -Wstrict-null-sentinel  -msse2 -w -Os -DNDEBUG -std=c++17 -fPIE -MD -MT Modules/ThirdParty/MetaIO/src/MetaIO/src/tests/CMakeFiles/testMeta2Object.dir/testMeta2Object.cxx.o -MF Modules/ThirdParty/MetaIO/src/MetaIO/src/tests/CMakeFiles/testMeta2Object.dir/testMeta2Object.cxx.o.d -o Modules/ThirdParty/MetaIO/src/MetaIO/src/tests/CMakeFiles/testMeta2Object.dir/testMeta2Object.cxx.o -c /home/vsts/work/1/s/Modules/ThirdParty/MetaIO/src/MetaIO/src/tests/testMeta2Object.cxx
/home/vsts/work/1/s/Modules/ThirdParty/MetaIO/src/MetaIO/src/tests/testMeta2Object.cxx: In function 'int main(int, char**)':
/home/vsts/work/1/s/Modules/ThirdParty/MetaIO/src/MetaIO/src/tests/testMeta2Object.cxx:144:86: error: call of overloaded 'MET_ValueToValue(MET_ValueEnumType, char*&, int, MET_ValueEnumType, char*&, int)' is ambiguous
  144 |   if (!MET_ValueToValue(MET_CHAR_ARRAY, inDataChar, 0, MET_CHAR_ARRAY, outDataChar, 1))
      |                                                                                      ^
In file included from /home/vsts/work/1/s/Modules/ThirdParty/MetaIO/src/MetaIO/src/metaObject.h:17,
                 from /home/vsts/work/1/s/Modules/ThirdParty/MetaIO/src/MetaIO/src/tests/testMeta2Object.cxx:3:
/home/vsts/work/1/s/Modules/ThirdParty/MetaIO/src/MetaIO/src/metaUtils.h:312:1: note: candidate: 'bool MET_ValueToValue(MET_ValueEnumType, const void*, std::streamoff, MET_ValueEnumType, void*, double, double, double, double)'
  312 | MET_ValueToValue(MET_ValueEnumType _fromType,
      | ^~~~~~~~~~~~~~~~
/home/vsts/work/1/s/Modules/ThirdParty/MetaIO/src/MetaIO/src/metaUtils.h:324:1: note: candidate: 'bool MET_ValueToValue(MET_ValueEnumType, const void*, std::streamoff, MET_ValueEnumType, void*, size_t, double, double, double, double)'
  324 | MET_ValueToValue(MET_ValueEnumType _fromType,
      | ^~~~~~~~~~~~~~~~
/home/vsts/work/1/s/Modules/ThirdParty/MetaIO/src/MetaIO/src/tests/testMeta2Object.cxx:160:90: error: call of overloaded 'MET_ValueToValue(MET_ValueEnumType, unsigned char*&, int, MET_ValueEnumType, unsigned char*&, int)' is ambiguous
  160 |   if (!MET_ValueToValue(MET_UCHAR_ARRAY, inDataUChar, 0, MET_UCHAR_ARRAY, outDataUChar, 1))
      |                                                                                          ^
In file included from /home/vsts/work/1/s/Modules/ThirdParty/MetaIO/src/MetaIO/src/metaObject.h:17,
                 from /home/vsts/work/1/s/Modules/ThirdParty/MetaIO/src/MetaIO/src/tests/testMeta2Object.cxx:3:
/home/vsts/work/1/s/Modules/ThirdParty/MetaIO/src/MetaIO/src/metaUtils.h:312:1: note: candidate: 'bool MET_ValueToValue(MET_ValueEnumType, const void*, std::streamoff, MET_ValueEnumType, void*, double, double, double, double)'
  312 | MET_ValueToValue(MET_ValueEnumType _fromType,
      | ^~~~~~~~~~~~~~~~
/home/vsts/work/1/s/Modules/ThirdParty/MetaIO/src/MetaIO/src/metaUtils.h:324:1: note: candidate: 'bool MET_ValueToValue(MET_ValueEnumType, const void*, std::streamoff, MET_ValueEnumType, void*, size_t, double, double, double, double)'
  324 | MET_ValueToValue(MET_ValueEnumType _fromType,
      | ^~~~~~~~~~~~~~~~
[1151/5743] Building CXX object Modules/ThirdParty/MetaIO/src/MetaIO/src/tests/CMakeFiles/testMeta1Utils.dir/testMeta1Utils.cxx.o
[1152/5743] Building CXX object Modules/ThirdParty/MetaIO/src/MetaIO/src/tests/CMakeFiles/testMeta3Image.dir/testMeta3Image.cxx.o
ninja: build stopped: subcommand failed.
Command exited with the value: 1
MakeCommand:/usr/local/bin/cmake --build . --config "MinSizeRel"
Error(s) when building project
   2 Compiler errors
   5 Compiler warnings
seanm commented 7 months ago

Oh interesting. I thought adding a new variant would be backwards compatible, but in the face of default arguments, it is apparently not necessarily.

This could break other code too.

Perhaps a better option is to give the new variants new names, like MET_ValueToValueSafe (or whatever)?

dzenanz commented 7 months ago

On the second look, this is the same test from MetaIO. But there is something different within ITK's build system.

seanm commented 7 months ago

Actually, I repro the build error with MetaIO itself. Seems BUILD_TESTING default to OFF, if I turn it on I see the errors.

dzenanz commented 7 months ago

Closing this PR. Once this has been fixed in MetaIO, we can create a new PR.

seanm commented 7 months ago

Shouldn't MetaIO's CI have caught this in https://github.com/Kitware/MetaIO/pull/118 ?

dzenanz commented 7 months ago

It should have. But these build errors were not properly propagated from CircleCI to the GitHub action. I guess MetaIO's infrastructure needs some love.