Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

use CoordinateType #674

Closed

Conversation

hjmjohnson
Copy link
Contributor

  • STYLE: Add itkVirtualGetNameOfClassMacro + itkOverrideGetNameOfClassMacro
  • STYLE: Rename ITK_DISALLOW_COPY_AND_ASSIGN to ITK_DISALLOW_COPY_AND_MOVE
  • STYLE: Replace itkStaticConstMacro with static constexpr
  • STYLE: CoordRepType -> CoordinateType code readability

…acro

Added two new macro's, intended to replace the old 'itkTypeMacro' and
'itkTypeMacroNoParent'.

The main aim is to be clearer about what those macro's do: add a virtual
'GetNameOfClass()' member function and override it. Unlike 'itkTypeMacro',
'itkOverrideGetNameOfClassMacro' does not have a 'superclass' parameter, as it
was not used anyway.

Note that originally 'itkTypeMacro' did not use its 'superclass' parameter
either, looking at commit 699b66cb04d410e555656828e8892107add38ccb, Will
Schroeder, June 27, 2001:
https://github.com/InsightSoftwareConsortium/ITK/blob/699b66cb04d410e555656828e8892107add38ccb/Code/Common/itkMacro.h#L331-L337
Clarifies that the macro does not just disallow copy and assign, but
also move operations. Note that in this context, the term 'move' refers
to both move-construct and move-assign.

With this commit, the old macro name will remain available, as long as
'ITK_FUTURE_LEGACY_REMOVE = OFF' (which is the default).
Use static constexpr directly now that C++11 conformance
is required by all compilers.

:%s/itkStaticConstMacro *( *\([^,]*\),[ \_s]*\([^,]*\),\_s*\([^)]*\)) */static constexpr \2 \1 = \3/ge

'itkStaticConstMacro(name, type, value)' became unconditionally
identical to 'static constexpr type name = value' with ITK commit
aec95193ab00e1322039911e1032da00f3a103b6 "ENH: Update compiler macros (#810)",
maekclena, 7 May 2019.

'itkGetStaticConstMacro(name)' became unconditionally identical to
'(Self::name)' with ITK commit 84e490b81e3f3c2b0edb89ae7b9de53bfc52f2b2
"Removing some outdated compiler conditionals", Hans Johnson, 31 July
2010.

Most 'itkStaticConstMacro' calls were removed by ITK commit 5c14741e1e063a132ea7e7ee69c5bd0a4e49af74
For the sake of code readability, a new 'CoordinateType' alias is added for
each nested 'CoordRepType' alias. The old 'CoordRepType' aliases will still be
available with ITK 6.0, but it is recommended to use 'CoordinateType' instead.
The 'CoordRepType' aliases will be removed when 'ITK_FUTURE_LEGACY_REMOVE' is
enabled. Similarly, 'InputCoordinateType', 'OutputCoordinateType', and
'ImagePointCoordinateType' replace 'InputCoordRepType', 'OutputCoordRepType',
and 'ImagePointCoordRepType', respectively.
Copy link
Collaborator

@SimonRit SimonRit left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for this! Actuallly, commits 7643a07 and 448bb26 are wrongly implemented in my opinion (see inline comments). Either you don't mind changing them or remove them from the PR and I'll redo it myself.

@@ -2176,7 +2176,7 @@ PREDEFINED = "itkNotUsed(x)=" \
"ITK_NULLPTR= nullptr " \
"ITK_NOEXCEPT= noexcept " \
"ITK_DISALLOW_COPY_AND_MOVE(type)=" \
"ITK_DISALLOW_COPY_AND_ASSIGN(type)=" \
"ITK_DISALLOW_COPY_AND_MOVE(type)=" \
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This line should either be deleted or the change cancelled I guess.

@@ -155,7 +155,7 @@ class ITK_TEMPLATE_EXPORT ADMMTotalVariationConeBeamReconstructionFilter
#ifdef itkOverrideGetNameOfClassMacro
itkOverrideGetNameOfClassMacro(ADMMTotalVariationConeBeamReconstructionFilter);
#else
itkTypeMacro(ADMMTotalVariationConeBeamReconstructionFilter, IterativeConeBeamReconstructionFilter);
itkOverrideGetNameOfClassMacro(ADMMTotalVariationConeBeamReconstructionFilter);
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is not correct. The ifdef are here for backward compatibility. This may be outdated (as we are only trying to be backward compatible with the latest release) but then the ifdef should be deleted and only the new version kept

@@ -2106,7 +2106,7 @@ INCLUDE_FILE_PATTERNS =
# This tag requires that the tag ENABLE_PREPROCESSING is set to YES.

PREDEFINED = "itkNotUsed(x)=" \
"itkStaticConstMacro(name,type,value)=static constexpr type name = value" \
"static constexpr type name = value=static constexpr type name = value" \
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This looks wrong...

@SimonRit
Copy link
Collaborator

Superseded by #675

@SimonRit SimonRit closed this Jan 27, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants