ENH: Add itkVirtual/itkFinal/itkNonVirtual macro variants#6253
Open
hjmjohnson wants to merge 2 commits into
Open
ENH: Add itkVirtual/itkFinal/itkNonVirtual macro variants#6253hjmjohnson wants to merge 2 commits into
hjmjohnson wants to merge 2 commits into
Conversation
74eef42 to
beacd3c
Compare
hjmjohnson
commented
May 11, 2026
Member
Author
|
@greptile review |
This comment was marked as resolved.
This comment was marked as resolved.
beacd3c to
ea5329c
Compare
ea5329c to
016b685
Compare
N-Dekker
reviewed
May 13, 2026
Bump the .gitattributes hooks-max-size attribute for Modules/Core/Common/include/itkMacro.h from the 100000-byte default to 200000 bytes ahead of upcoming work that adds the itkVirtual/itkFinal/itkNonVirtual macro variants. After those families are added the header grows from ~90 KB to ~110 KB, exceeding the default ghostflow/kw-pre-commit size guard. Matches the precedent set for itkPatchBasedDenoisingImageFilter.hxx (120000 bytes).
Introduce explicit virtual/final/non-virtual specifier variants for
the ITK Set/Get macro families:
* itkSetMacro, itkGetMacro, itkGetConstMacro,
itkGetConstReferenceMacro, itkBooleanMacro
* itkSet/Get/SetGetInput, itkSet/Get/SetGetDecoratedInput,
itkSet/Get/SetGetDecoratedObjectInput,
itkSet/GetDecoratedOutput
* itkSet/GetEnum, itkSet/GetString, itkSetClamp, itkSetObject,
itkGet/GetConst/GetModifiable/GetConstReferenceObject,
itkSetConstObject, itkSet/GetVector
Each family now expands a single internal ITK_DETAIL_*MacroImpl
helper parametrized by virtual-specifier and trailing
virt-specifier. The three new variants itkVirtual*, itkFinal*,
itkNonVirtual* expand to that helper with the appropriate
specifiers; the legacy itk*Macro form expands to the itkVirtual*
variant. The Object family preserves the ITK_FUTURE_LEGACY_REMOVE
ifdef branch. No behavioral or ABI change.
Internal helpers use the ITK_DETAIL_<X>MacroImpl namespace
(uppercase ITK_DETAIL_ prefix mirrors ITK's existing
"namespace detail" convention for implementation helpers). Each
helper carries a "Do not invoke ... directly from user code"
comment above its definition.
No itkOverride* variants are provided: re-declaring an inherited
Set/Get with an identical macro-generated body duplicates the
base. Write the function by hand if a real override is needed.
Co-Authored-By: Niels Dekker <N.Dekker@lumc.nl>
016b685 to
2766eb5
Compare
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
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.
Add
itkVirtual/itkFinal/itkNonVirtualvariants for allitkSet*/itkGet*macros. Legacy macros unchanged — zero behavioral/ABI change. Silences 611-Wunnecessary-virtual-specifierwarnings in ANTs (issue #6232, PR #2785).Backward-compatibility guarantee
100 % backward-compatible. Every legacy macro name continues to expand to the
virtualvariant — byte-identical generated code, identical vtable layout, identical ABI. No deprecation gate, noITK_FUTURE_LEGACY_REMOVEchange, no symbol churn, no API removed/renamed/relocated. Downstream code that never typesitkVirtual*/itkFinal*/itkNonVirtual*sees no behavioral or compilation difference. The PR only adds new optional spellings; no existing class is migrated.What this PR does
For each member-function macro family in
itkMacro.h:*Implhelper parametrized by(virtualKeyword, finalKeyword)— the DRY shared body.itkVirtual*Macro→virtual(matches today's behavior)itkFinal*Macro→final(seals further overriding)itkNonVirtual*Macro→ no virtual-specifier (new methods infinalleaf classes)itkVirtual*Macrovariant.No
itkOverride*variants are provided: re-declaring an inherited Set/Get in a subclass with the exact same macro-generated body just duplicates the base-class implementation. If an override is genuinely needed (different behavior, extra validation, side effects), write the function by hand. The pre-existingitkOverrideGetNameOfClassMacrois unchanged.Minimal non-breaking foundation for PR #2785 and issue #6232.
Why this matters — concrete ANTs evidence
ANTs has shipped
finalleaf classes (command-line option types, filter wrappers, statistical containers, image-curvature analyzers, etc.) since at least 2019. For that entire period the legacy macros offered no mechanism to silence Clang's-Wunnecessary-virtual-specifieron the resultingvirtualaccessors — the only workaround was abandoning the macros and losingitkDebugMacro/Modified()/ float-equal-suppression standardization.Clean ANTs
masterbuild against this PR:final-marked headers.itkNonVirtual*at 167 call sites in those 14 headers: 15 remaining (residue is hand-writtenvirtualkeywords, not macro-generated).Design: the DRY pattern
At most one of
virtualKeywordandfinalKeywordis non-empty;itkNonVirtual*passes both empty.Local verification
pre-commit run --all-filespasses at the pushed HEAD.ITKCommonand module header tests build clean.masterbuild against this PR yields the 626→15 warning delta above.