COMP: Fix implicit copy deprecated by user-declared destructor#6538
COMP: Fix implicit copy deprecated by user-declared destructor#6538blowekamp wants to merge 1 commit into
Conversation
Add ITK_DEFAULT_COPY_AND_MOVE to classes that have a user-declared destructor but rely on implicit copy/move, silencing -Wdeprecated-copy-with-dtor (Apple Clang) and -Wdeprecated-copy-with-user-provided-dtor. Affected classes: - DefaultVectorPixelAccessor (itkDefaultVectorPixelAccessor.h) - ElementWrapperInterface (itkPriorityQueueContainer.h) - ElementWrapperPointerInterface (itkPriorityQueueContainer.h) - MinPriorityQueueElementWrapper (itkPriorityQueueContainer.h) - MaxPriorityQueueElementWrapper (itkPriorityQueueContainer.h) Seen on CDash build 11383687 (Mac14.x-AppleClang-dbg-Universal, 199 warnings).
| {} | ||
|
|
||
| ~DefaultVectorPixelAccessor() = default; | ||
| ITK_DEFAULT_COPY_AND_MOVE(DefaultVectorPixelAccessor); |
There was a problem hiding this comment.
@blowekamp I ran into similar situations (with gdcm I think), and you may be able to use the "rule of zero" and remove the lines 118, 126 and 127 to get the same behavior.
I think that this class would use the rule of zero and remove all the = defaults, ITK_DEFAULT_COPY_AND_MOVE, and only keep the L121 constructor.
There was a problem hiding this comment.
PS: I'm not opposed to this as is, and my assertions above have not been tested locally.
hjmjohnson
left a comment
There was a problem hiding this comment.
For DefaultVectorPixelAccessor the rule of zero is a cleaner fit than the macro here. The only special member that triggers -Wdeprecated-copy-with-dtor is the user-declared destructor, and the two data members are trivially-copyable with in-class initializers, so the implicitly-generated copy/move are already correct. Dropping the defaulted destructor (and not adding ITK_DEFAULT_COPY_AND_MOVE) silences both warnings with identical behavior.
I verified locally under -Wdeprecated-copy-dtor: destructor-only removal compiles clean (0 warnings, 0 errors). One caveat -- the defaulted default constructor on L118 must stay, because NthElementPixelAccessor (and VectorImageToImagePixelAccessor) default-construct this base and then call SetVectorLength; removing it fails to compile. The inline suggestion below applies just the rule-of-zero part and is safe to commit as-is.
(The other four classes in this PR have virtual destructors, so the rule of zero does not apply to them -- ITK_DEFAULT_COPY_AND_MOVE remains the right fix there.)
|
|
||
| ~DefaultVectorPixelAccessor() = default; | ||
| ITK_DEFAULT_COPY_AND_MOVE(DefaultVectorPixelAccessor); | ||
|
|
There was a problem hiding this comment.
Rule of zero: drop the defaulted destructor (and the macro). The members are trivially copyable, so the implicit copy/move are correct and no longer deprecated. This keeps the defaulted default constructor above (L118), which NthElementPixelAccessor relies on.
| ~DefaultVectorPixelAccessor() = default; | |
| ITK_DEFAULT_COPY_AND_MOVE(DefaultVectorPixelAccessor); |
Add
ITK_DEFAULT_COPY_AND_MOVEto five classes that declare a destructor without explicit copy/move operations, silencing-Wdeprecated-copy-with-dtor(Apple Clang) and-Wdeprecated-copy-with-user-provided-dtorwarnings.Root cause
Apple Clang and newer GCC warn when a class declares any special member function (including
= defaultdestructor) without explicitly declaring all copy/move operations. ITK provides theITK_DEFAULT_COPY_AND_MOVEmacro for exactly this purpose.Affected classes in ITK proper (ThirdParty/GDCM warnings excluded):
itk::DefaultVectorPixelAccessor—itkDefaultVectorPixelAccessor.hitk::ElementWrapperInterface—itkPriorityQueueContainer.hitk::ElementWrapperPointerInterface—itkPriorityQueueContainer.hitk::MinPriorityQueueElementWrapper—itkPriorityQueueContainer.hitk::MaxPriorityQueueElementWrapper—itkPriorityQueueContainer.hAll classes have only POD or trivially copyable members, so the defaulted copy/move operations are semantically correct.
Reported on CDash build 11383687 (Mac14.x-AppleClang-dbg-Universal, 199 warnings).
AI assistance
Warnings identified via CDash API. Fixes applied and verified with a local build of
ITKCommon— zero warnings from the edited files after the change.