STYLE/COMP: Silence 611 -Wunnecessary-virtual-specifier warnings via itkNonVirtual* macros#1972
Merged
Merged
Conversation
Migrate 14 final-class headers from itkSetMacro/itkGetMacro/... to their itkNonVirtual* equivalents. This silences 611 of the 626 -Wunnecessary-virtual-specifier warnings reported by Clang on a clean build against ITK main (with the new macro variants added in InsightSoftwareConsortium/ITK). The legacy itkSetMacro/itkGetMacro/itkBooleanMacro families emit 'virtual' on the generated member function. When the enclosing class is marked 'final', that 'virtual' is meaningless and Clang flags it. The new itkNonVirtual* family emits the same body without 'virtual', which is the correct form for new member functions in a final class (C++ only permits 'final' on functions that override a base virtual). Also fix the latent typo itkNotVirtualGetConstReferenceMacro -> itkNonVirtualGetConstReferenceMacro in itkANTSAffine3DTransform.h; the misspelled name never existed in ITK. Migrated files (167 macro call sites converted): Utilities/antsCommandLineOption.h Utilities/antsSCCANObject.h Utilities/antsCommandLineParser.h Temporary/antsFastMarchingImageFilter.h Utilities/itkMultiScaleLaplacianBlobDetectorImageFilter.h Utilities/itkLabeledPointSetFileWriter.h Utilities/itkSurfaceImageCurvature.h Utilities/itkPulsedArterialSpinLabeledCerebralBloodFlowImageFilter.h Utilities/itkAlternatingValueDifferenceImageFilter.h Utilities/itkLabeledPointSetFileReader.h Utilities/itkSliceTimingCorrectionImageFilter.h Utilities/antsMatrixUtilities.h Utilities/itkAlternatingValueSimpleSubtractionImageFilter.h Utilities/itkAverageOverDimensionImageFilter.h ImageRegistration/itkANTSAffine3DTransform.h The 15 remaining -Wunnecessary-virtual-specifier warnings are on hand-written 'virtual' keywords (not macro calls); those will be addressed separately. Requires the four-variant macro additions in ITK (itkVirtual*, itkOverride*, itkFinal*, itkNonVirtual* Get/Set families) merging upstream first.
The preceding STYLE commit migrates 14 final-class headers from the legacy itkSetMacro/itkGetMacro/itkBooleanMacro families to their itkNonVirtual* equivalents. Those macro variants are introduced by InsightSoftwareConsortium/ITK PR #6253 and are not present in any released ITK (5.x series, nor early 6.x master before that PR). To keep ANTs buildable against both ITK 5.x and pre-6253 ITK 6.x while the PR matures upstream, add a backward-compatibility shim header that provides the itkVirtual* / itkFinal* / itkNonVirtual* families. The sentinel `#if !defined(itkSetMacroImpl)` collapses the shim to an empty translation unit once ITK ships with PR #6253, so the file can be retired without source churn at that time. Wire the shim into all 15 headers that consume the new macros. The legacy itkSetMacro/itkGetMacro/itkBooleanMacro names are intentionally NOT redefined here; pre-6253 ITK already provides them as virtual- emitting forms, and redefinition would trigger -Wmacro-redefined. Validated by two clean SuperBuilds from scratch (build-ITKv5/ and build-ITKv6/, both against the pinned pre-6253 ITK GIT_TAG 18d89fd6, 2026-03-18): 424/424 inner targets + 17/17 SuperBuild steps, 0 errors in each.
ntustison
approved these changes
May 13, 2026
ntustison
left a comment
ntustison
left a comment
Member
There was a problem hiding this comment.
Looks good to me. Thanks @hjmjohnson .
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Silence 611 of 626
-Wunnecessary-virtual-specifierwarnings reported by Clang on a clean ANTs build, by migrating 14finalclasses fromitk{Set,Get,Boolean}Macroto their newitkNonVirtual*equivalents (from upcoming ITK PR InsightSoftwareConsortium/ITK#6253). A backward-compatibility shim header makes the work usable against current ITK 5.x and ITK 6.x master today, without waiting for #6253 to merge.Why the warning fires (Clang's
-Wunnecessary-virtual-specifier)The legacy
itkSetMacro/itkGetMacro/itkBooleanMacrofamilies emitvirtualon every generated member function. When the enclosing class is markedfinal, thatvirtualis meaningless — C++ only permitsfinalon functions that override a base virtual, so avirtualfunction declared inside afinalclass with no matching base method cannot be further overridden. Clang flags every such occurrence.The new
itkNonVirtual*family emits the same function body withoutvirtual, which is the correct spelling for a freshly-introduced member function inside afinalclass.Scope of the cleanup
14 headers migrated (15 files touched once the shim include is added to each):
ImageRegistration/itkANTSAffine3DTransform.hANTSAffine3DTransformTemporary/antsFastMarchingImageFilter.hFastMarchingImageFilterUtilities/antsCommandLineOption.hCommandLineOptionUtilities/antsCommandLineParser.hCommandLineParserUtilities/antsMatrixUtilities.hantsMatrixUtilitiesUtilities/antsSCCANObject.hantsSCCANObjectUtilities/itkAlternatingValueDifferenceImageFilter.hAlternatingValueDifferenceImageFilterUtilities/itkAlternatingValueSimpleSubtractionImageFilter.hAlternatingValueSimpleSubtractionImageFilterUtilities/itkAverageOverDimensionImageFilter.hAverageOverDimensionImageFilterUtilities/itkLabeledPointSetFileReader.hLabeledPointSetFileReaderUtilities/itkLabeledPointSetFileWriter.hLabeledPointSetFileWriterUtilities/itkMultiScaleLaplacianBlobDetectorImageFilter.hMultiScaleLaplacianBlobDetectorImageFilterUtilities/itkPulsedArterialSpinLabeledCerebralBloodFlowImageFilter.hPulsedArterialSpinLabeledCerebralBloodFlowImageFilterUtilities/itkSliceTimingCorrectionImageFilter.hSliceTimingCorrectionImageFilterUtilities/itkSurfaceImageCurvature.hSurfaceImageCurvatureAlso fixed: a latent typo
itkNotVirtualGetConstReferenceMacro→itkNonVirtualGetConstReferenceMacro.Backward compatibility:
Utilities/ITKGetterSetterMacroShims.hitkNonVirtual*and itsitkVirtual*/itkFinal*cousins are introduced by ITK PR #6253 and are absent from every released ITK to date — both the 5.x series and current 6.xmaster.To keep ANTs buildable while that PR matures upstream, this PR adds a shim header that supplies the new macro family. The sentinel:
uses
itkSetMacroImpl— a brand-new identifier introduced by #6253 — as the trip-wire. Once ITK ships with #6253, the entire shim collapses to an empty translation unit and the file can be deleted in a one-line follow-up commit. No source churn at the migrated call sites.Design notes:
itkVirtual*,itkFinal*,itkNonVirtual*) plus the internal*Impl(virtualKeyword, finalKeyword, ...)helpers.itkSetMacro/itkGetMacro/etc. names are not redefined — pre-6253 ITK already provides them asvirtual-emitting forms, and redefinition would trigger-Wmacro-redefined.Set/GetInput,Set/GetDecoratedInput,SetGetDecoratedInput,Set/GetDecoratedObjectInput,SetGetDecoratedObjectInput,Set,Get,GetConst,GetConstReference,SetEnum,GetEnum,SetString,GetString,SetClamp,SetObject,GetObject,GetConstObject,GetModifiableObject(honoringITK_FUTURE_LEGACY_REMOVE),GetConstReferenceObject,SetConstObject,Boolean,SetVector,GetVector,Set/GetDecoratedOutput.Local validation
Two clean SuperBuilds from scratch, both against the SuperBuild's pinned pre-6253 ITK
GIT_TAG=18d89fd6(2026-03-18):build-ITKv5/build-ITKv6/(The "ITKv5" name is documentary — the ANTs SuperBuild only supports
ITK_VERSION_MAJOR=6. Both builds exercise the shim's sentinel-false branch since the pinned ITK predates #6253.)ccache was used to share dependency objects between the two builds. ANTs has no
.pre-commit-config.yaml, so the per-PR pre-commit gate does not apply.