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

COMP: Use modern macro for name of class #126

Open
wants to merge 5 commits into
base: main
Choose a base branch
from

Conversation

hjmjohnson
Copy link
Contributor

When preparing for the future with ITK by setting
ITK_FUTURE_LEGACY_REMOVE:BOOL=ON
ITK_LEGACY_REMOVEBOOL=ON

The future preferred macro should be used
│ - itkTypeMacro
│ + itkOverrideGetNameOfClassMacro

When preparing for the future with ITK by setting
ITK_FUTURE_LEGACY_REMOVE:BOOL=ON
ITK_LEGACY_REMOVEBOOL=ON

The future preferred macro should be used
│ -  itkTypeMacro
│ +  itkOverrideGetNameOfClassMacro
@hjmjohnson
Copy link
Contributor Author

@dzenanz This is part of an effort to create a v5.4.2.1 release that allows Slicer to build with ITK_LEGACY_REMOVE and ITK_FUTURE_LEGACY_REMOVE turned on.

It is an intermediate refactoring that is upstream compatible but allows Slicer to catch up more slowly.

@hjmjohnson
Copy link
Contributor Author

@dzenanz Part of the future updates needed to build Slicer with future code recommendationsl

@dzenanz
Copy link
Member

dzenanz commented Jan 28, 2025

This sounds like WASM part needs to be updated to use a newer base image:

In file included from /work/wasm/morphological-contour-interpolation.cxx:24:
/work/include/itkMorphologicalContourInterpolator.h:83:3: error: unknown type name 'itkOverrideGetNameOfClassMacro'
   83 |   itkOverrideGetNameOfClassMacro(MorphologicalContourInterpolator);
      |   ^
1 error generated.

@dzenanz dzenanz requested a review from thewtex January 28, 2025 16:31
@thewtex
Copy link
Collaborator

thewtex commented Jan 28, 2025

Is itkOverrideGetNameOfClassMacro in the release-5.4 branch?

@dzenanz
Copy link
Member

dzenanz commented Jan 28, 2025

Yes, it was added in Dec 2023.

@dzenanz
Copy link
Member

dzenanz commented Jan 28, 2025

It is there already in v5.4.0:
https://github.com/InsightSoftwareConsortium/ITK/blob/v5.4.0/Modules/Core/Common/include/itkMacro.h#L463

@thewtex
Copy link
Collaborator

thewtex commented Jan 30, 2025

I will rebase, follow-up.

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.

3 participants