STYLE: Use macros to Set/Get ivars in Common operators

[jhlegarreta]

Use macros to Set/Get ivars in Common operators.

Take advantage of the commit to leave the documentation of the ivars only
in their corresponding Set/Get methods, as dictated by the ITK SW Guide.

PR Checklist

• No API changes were made (or the changes have been approved)
• No major design changes were made (or the changes have been approved)

[jhlegarreta]

Not sure about what the errors imply:
https://open.cdash.org/viewBuildError.php?buildid=7260537

Suggestions are welcome.

[jhlegarreta]

Push forced to see the error reports back and see where we were at this level.

[N-Dekker]

I'm sorry but I still don't like to have a virtual keyword added to those Get and Set member functions. Especially when they were originally non-virtual, and when they worked just fine without a virtual keyword. The only reason for adding virtual to an existing member function should be that some user actually needs to override it.

C++ Core Guidelines C.132: Don’t make a function virtual without reason says on this topic:

Redundant virtual increases run-time and object-code size. A virtual function can be overridden and is thus open to mistakes in a derived class...

We already spent a lot of time in the last few years, removing virtual from the ITK code base, and we're still not finished. Removal of virtual might break legacy user code, even while it is an improvement to new user code, which is why Bradley (@blowekamp) introduced a ITK_ITERATOR_VIRTUAL macro, for legacy support.

ITK/Modules/Core/Common/include/itkMacro.h

Lines 1314 to 1326 in 6e3d31d

	#if defined(ITKV4_COMPATIBILITY)
	# define ITK_ITERATOR_VIRTUAL virtual
	# define ITK_ITERATOR_OVERRIDE override
	# define ITK_ITERATOR_FINAL
	#elif !defined(ITK_LEGACY_REMOVE)
	# define ITK_ITERATOR_VIRTUAL virtual
	# define ITK_ITERATOR_OVERRIDE override
	# define ITK_ITERATOR_FINAL final
	#else
	# define ITK_ITERATOR_VIRTUAL
	# define ITK_ITERATOR_OVERRIDE
	# define ITK_ITERATOR_FINAL
	#endif

So please only make a member function virtual when it is actually needed!

[jhlegarreta]

Thanks for the comment @N-Dekker . Then, as pointed in this comment of yours I'll mark it to status:Blocked until the proposed itkGetConstNonVirtualMacro gets added to the ITK code base.

[jhlegarreta]

10fce06 might go somehow unnoticed inside the PR and unless one takes a look at the commit history, but did not want to push a PR that added code that was not exercised. Also, I am not sure if all these would get extensive use with time. Open to suggestions.

[dzenanz]

Niels' PR is more comprehensive, so I would wait with this PR until #2785 is resolved. Then rebase this on top of that, which would make this PR somewhat smaller.

[stale]

This issue has been automatically marked as stale because it has not had recent activity. Thank you for your contributions.

[hjmjohnson]

@jhlegarreta Is this still something that needs to be worked on? If so, could you please rebase and make a game plan for what needs to be done?

[jhlegarreta]

@jhlegarreta Is this still something that needs to be worked on? If so, could you please rebase and make a game plan for what needs to be done?

Thanks for the heads-up Hans. Still on my ToDo list; have not had the time too look at my ITK-related ToDo list in a while. I do think this still deserves having a go. Not sure when I'll be able to take it up, though.

Also, wisest might be to wait until what is mentioned here is resolved/done: #2568 (comment)

[hjmjohnson]

@jhlegarreta I rebased on top of master. Is there more work that needs to be done to this PR? My reading is that all the issues have been dealt with.

CI identifies spelling of "overridden"

[jhlegarreta]

@N-Dekker expressed concerns about this, but going over the files I do not see any of these members being virtual, and inspecting the commits I do not see where they were being declared as virtual. Sorry @hjmjohnson. @N-Dekker do you think that we should still wait till you have time to resolve #2785 to merge this?

[hjmjohnson]

Per the request to consolidate the scattered virtual / override / final design conversation around ITK Get/Set macros, I've opened #6232 (Master tracking — virtual / override / final semantics on Get/Set macros for ITK 6.x) as a single venue.

It pulls in the 2018 Discourse #814 precedent (the same final-as-stress-test technique and the @phcerdan / @N-Dekker 18–24% HoughTransform performance gain), the 2026-05-07 final-keyword audit (17 base methods overridden across 19 files in upstream ITK), and a draft additive itkVirtualSetMacro family on hjmjohnson:macro-optionally-virtual @ ffc70ed0b9. Surfaces 7 binary/short-list questions — Q1 (default-emit virtual?) is the load-bearing one this PR is blocked on.

Suggest holding further design comments on this thread until #6232 has a maintainer-quorum answer on Q1; otherwise the conversation re-fragments. This thread can stay open as the implementation venue once direction is settled.

[jhlegarreta]

Closing after offline discussion with @hjmjohnson so that work can make progress without this being a roadblock.