COMP: Remove ITK_DISALLOW_COPY_AND_MOVE from UserData struct's#1202
COMP: Remove ITK_DISALLOW_COPY_AND_MOVE from UserData struct's#1202N-Dekker merged 1 commit intoSuperElastix:mainfrom
Conversation
|
@N-Dekker there may be a better solution to keep some of the constraints, but this was a quick fix to get the build to succeed on my system. |
|
Thanks @thewtex Do you have a link to the entire log? I'm still trying to reproduce the error, but I haven't seen it yet: The following just compiles: https://godbolt.org/z/3jejxcqMa #include <vector>
#define ITK_DISALLOW_COPY_AND_MOVE(TypeName) \
TypeName(const TypeName &) = delete; \
TypeName & operator=(const TypeName &) = delete; \
TypeName(TypeName &&) = delete; \
TypeName & operator=(TypeName &&) = delete
using InputImageType = int;
using MaskType = int;
using SampleGridSpacingType = int;
using WorkUnit = int;
struct UserData
{
ITK_DISALLOW_COPY_AND_MOVE(UserData);
const InputImageType & InputImage;
const MaskType * const Mask{};
const SampleGridSpacingType GridSpacing{};
std::vector<WorkUnit> WorkUnits{};
};
std::vector<WorkUnit> GenerateWorkUnits();
void f(
const InputImageType & inputImage,
const MaskType * const mask,
const SampleGridSpacingType gridSpacing
)
{
UserData userData{ inputImage,
mask,
gridSpacing,
GenerateWorkUnits() };
} |
|
@N-Dekker the commit / pr comment has the full error. There are many more emitted for all the locations where the macro was present. |
Thanks Matt. So then, should the errors be reproducible by the code I just posted? At #1202 (comment) and https://godbolt.org/z/3jejxcqMa ? It looks like a compiler bug, but I'm not sure 🤷 Such @thewtex Can you please tell me, did these errors occur only at a local build, or also at a CI build? Is the full compilation log available online? I'm especially interested to see the compiler command-line flags. Would it be possible for you to reproduce the errors by just compiling the minimal example from https://godbolt.org/z/3jejxcqMa ? I think it's important to know if there is indeed a limitation to the use of The errors do appear at https://godbolt.org/z/EvTcnPx13 when doing a C++11 build, using GCC compiler flag |
|
@thewtex Finally I managed to reproduce your issue! At https://github.com/InsightSoftwareConsortium/ITKElastix/actions/runs/10575617835/job/29299639880 So now I finally see the compiler command-line flags that I wanted to see. It has Looks like this is the breaking change between C++17 and C++20: p1008r1 - Prohibit aggregates with user-declared constructors. @thewtex Can you please mention in your commit message that your adjustments are necessary for C++20 support? |
On GCC 12.2.2 (Debian) with C++20 enabled:
/home/matt/src/elastix/Common/ImageSamplers/itkImageGridSampler.hxx: In instantiation of ‘static void itk::ImageGridSampler<TInputImage>::MultiThreadedGenerateData(itk::MultiThreaderBase&, itk::ThreadIdType, const TInputImage&, const MaskType*, const typename Superclass::InputImageRegionType&, const SampleGridSpacingType&, std::vector<typename itk::ImageSamplerBase<TInputImage>::ImageSampleType>&) [with TInputImage = itk::Image<short int, 4>; itk::ThreadIdType = unsigned int; MaskType = itk::ImageMaskSpatialObject<4, unsigned char>; typename Superclass::InputImageRegionType = itk::ImageRegion<4>; Superclass = itk::ImageSamplerBase<itk::Image<short int, 4> >; SampleGridSpacingType = itk::Offset<4>; typename itk::ImageSamplerBase<TInputImage>::ImageSampleType = itk::ImageSample<itk::Image<short int, 4> >]’:
/home/matt/src/elastix/Common/ImageSamplers/itkImageGridSampler.hxx:277:30: required from ‘void itk::ImageGridSampler<TInputImage>::GenerateData() [with TInputImage = itk::Image<short int, 4>]’
/home/matt/src/elastix/Common/ImageSamplers/itkImageGridSampler.hxx:253:1: required from here
/home/matt/src/elastix/Common/ImageSamplers/itkImageGridSampler.hxx:212:12: error: no matching function for call to ‘itk::ImageGridSampler<itk::Image<short int, 4> >::UserData::UserData(<brace-enclosed initializer list>)’
212 | UserData userData{ inputImage,
| ^~~~~~~~
/home/matt/src/elastix/Common/ImageSamplers/itkImageGridSampler.h:169:32: note: candidate: ‘itk::ImageGridSampler<TInputImage>::UserData::UserData(itk::ImageGridSampler<TInputImage>::UserData&&) [with TInputImage = itk::Image<short int, 4>]’ (deleted)
169 | ITK_DISALLOW_COPY_AND_MOVE(UserData);
| ^~~~~~~~
/home/matt/src/ITK/Modules/Core/Common/include/itkMacro.h:397:3: note: in definition of macro ‘ITK_DISALLOW_COPY_AND_MOVE’
397 | TypeName(TypeName &&) = delete; \
| ^~~~~~~~
/home/matt/src/elastix/Common/ImageSamplers/itkImageGridSampler.h:169:32: note: candidate expects 1 argument, 4 provided
169 | ITK_DISALLOW_COPY_AND_MOVE(UserData);
| ^~~~~~~~
/home/matt/src/ITK/Modules/Core/Common/include/itkMacro.h:397:3: note: in definition of macro ‘ITK_DISALLOW_COPY_AND_MOVE’
397 | TypeName(TypeName &&) = delete; \
| ^~~~~~~~
/home/matt/src/elastix/Common/ImageSamplers/itkImageGridSampler.h:169:32: note: candidate: ‘itk::ImageGridSampler<TInputImage>::UserData::UserData(const itk::ImageGridSampler<TInputImage>::UserData&) [with TInputImage = itk::Image<short int, 4>]’ (deleted)
169 | ITK_DISALLOW_COPY_AND_MOVE(UserData);
| ^~~~~~~~
/home/matt/src/ITK/Modules/Core/Common/include/itkMacro.h:395:3: note: in definition of macro ‘ITK_DISALLOW_COPY_AND_MOVE’
395 | TypeName(const TypeName &) = delete; \
| ^~~~~~~~
/home/matt/src/elastix/Common/ImageSamplers/itkImageGridSampler.h:169:32: note: candidate expects 1 argument, 4 provided
169 | ITK_DISALLOW_COPY_AND_MOVE(UserData);
| ^~~~~~~~
/home/matt/src/ITK/Modules/Core/Common/include/itkMacro.h:395:3: note: in definition of macro ‘ITK_DISALLOW_COPY_AND_MOVE’
395 | TypeName(const TypeName &) = delete; \
| ^~~~~~~~
Thanks Matt, I will merge your PR as soon as the CI allows me to!
Cool.... but I see, it even required C++23, nowadays: https://github.com/stephenberry/glaze?tab=readme-ov-file#compilersystem-support I guess ITK-Wasm uses an older version of glaze, right? |
N-Dekker
left a comment
There was a problem hiding this comment.
Thanks Matt! Sorry it took me a while to approve your PR, as it wasn't clear to me why ITK_DISALLOW_COPY_AND_MOVE(UserData) would cause compile errors. It's only now that I know that it needs to compile on C++20 that I can approve it full-heartedly!
| //ITK_DISALLOW_COPY_AND_MOVE(UserData); | ||
|
|
There was a problem hiding this comment.
Thanks @thewtex. I will just remove this line of code afterwards. (No need to keep it commented out.)
There was a problem hiding this comment.
- FYI, done: pull request STYLE: Remove commented out
ITK_DISALLOW_COPY_AND_MOVEcall #1220 commit ab98782
|
Thanks for the review and integration, Neils 🙏
Yes, it was just added recently: but it seems to be supported by the same compilers. |
|
@thewtex Should we then consider to upgrade ITK and elastix to C++20? 🤔 |
ITK may be difficult do to the community's needs to support older compilers. But we could do more testing / prep for ITK with elastix, ITK-Wasm, ... |
May be, indeed. But for ITKElastix, both ITK and elastix now need to be C++20-compatible (= compilable), right? |
Yes. This is the case for ITK, and it should be the case for elastix with my patches. The ITK-Wasm build of both uses C++20. |
On GCC 12.2.2 (Debian):
/home/matt/src/elastix/Common/ImageSamplers/itkImageGridSampler.hxx: In instantiation of ‘static void itk::ImageGridSampler::MultiThreadedGenerateData(itk::MultiThreaderBase&, itk::ThreadIdType, const TInputImage&, const MaskType*, const typename Superclass::InputImageRegionType&, const SampleGridSpacingType&, std::vector<typename itk::ImageSamplerBase::ImageSampleType>&) [with TInputImage = itk::Image<short int, 4>; itk::ThreadIdType = unsigned int; MaskType = itk::ImageMaskSpatialObject<4, unsigned char>; typename Superclass::InputImageRegionType = itk::ImageRegion<4>; Superclass = itk::ImageSamplerBase<itk::Image<short int, 4> >; SampleGridSpacingType = itk::Offset<4>; typename itk::ImageSamplerBase::ImageSampleType = itk::ImageSample<itk::Image<short int, 4> >]’:
/home/matt/src/elastix/Common/ImageSamplers/itkImageGridSampler.hxx:277:30: required from ‘void itk::ImageGridSampler::GenerateData() [with TInputImage = itk::Image<short int, 4>]’
/home/matt/src/elastix/Common/ImageSamplers/itkImageGridSampler.hxx:253:1: required from here
/home/matt/src/elastix/Common/ImageSamplers/itkImageGridSampler.hxx:212:12: error: no matching function for call to ‘itk::ImageGridSampler<itk::Image<short int, 4> >::UserData::UserData()’
212 | UserData userData{ inputImage,
| ^~~~~~~~
/home/matt/src/elastix/Common/ImageSamplers/itkImageGridSampler.h:169:32: note: candidate: ‘itk::ImageGridSampler::UserData::UserData(itk::ImageGridSampler::UserData&&) [with TInputImage = itk::Image<short int, 4>]’ (deleted)
169 | ITK_DISALLOW_COPY_AND_MOVE(UserData);
| ^~~~~~~~
/home/matt/src/ITK/Modules/Core/Common/include/itkMacro.h:397:3: note: in definition of macro ‘ITK_DISALLOW_COPY_AND_MOVE’
397 | TypeName(TypeName &&) = delete;
| ^~~~~~~~
/home/matt/src/elastix/Common/ImageSamplers/itkImageGridSampler.h:169:32: note: candidate expects 1 argument, 4 provided
169 | ITK_DISALLOW_COPY_AND_MOVE(UserData);
| ^~~~~~~~
/home/matt/src/ITK/Modules/Core/Common/include/itkMacro.h:397:3: note: in definition of macro ‘ITK_DISALLOW_COPY_AND_MOVE’
397 | TypeName(TypeName &&) = delete;
| ^~~~~~~~
/home/matt/src/elastix/Common/ImageSamplers/itkImageGridSampler.h:169:32: note: candidate: ‘itk::ImageGridSampler::UserData::UserData(const itk::ImageGridSampler::UserData&) [with TInputImage = itk::Image<short int, 4>]’ (deleted)
169 | ITK_DISALLOW_COPY_AND_MOVE(UserData);
| ^~~~~~~~
/home/matt/src/ITK/Modules/Core/Common/include/itkMacro.h:395:3: note: in definition of macro ‘ITK_DISALLOW_COPY_AND_MOVE’
395 | TypeName(const TypeName &) = delete;
| ^~~~~~~~
/home/matt/src/elastix/Common/ImageSamplers/itkImageGridSampler.h:169:32: note: candidate expects 1 argument, 4 provided
169 | ITK_DISALLOW_COPY_AND_MOVE(UserData);
| ^~~~~~~~
/home/matt/src/ITK/Modules/Core/Common/include/itkMacro.h:395:3: note: in definition of macro ‘ITK_DISALLOW_COPY_AND_MOVE’
395 | TypeName(const TypeName &) = delete;
| ^~~~~~~~