Handling `GetNameOfClass` for upcast instances
Motivation
ITK tests rely on the ITK_EXERCISE_BASIC_OBJECT_METHODS macro for unit testing individual classes, including checking that class and superclass names match expectations. Investigation in #3003 reveals that upcast instances returned from object factory instantiation may have inconsistent naming which may require more detailed test handling than existing macros allow.
Details
FFT classes are a concrete example of how the ITK object factory can be used to dynamically swap in different backends for a given class interface at runtime. itk::ComplexToComplexFFTImageFilter is a base FFT class which defines an interface and relies on the object factory to implement a backend such as Vnl or FFTW for FFT computation. The class hierarchy looks like this:
itk::ImageToImageFilter->itk::ComplexToComplexFFTImageFilter->itk::VnlComplexToComplexFFTImageFilter
If a filter is instantiated through the object factory:
itk::ComplexToComplexFFTImageFilter::Pointer inverseComplexFilter = itk::ComplexToComplexFFTImageFilter::New();
Then several methods are available to determine the name of the class and its superclass:
inverseComplexFilter->GetNameOfClass() --> VnlComplexToComplexFFTImageFilter
inverseComplexFilter->Self::GetNameOfClass() --> ComplexToComplexFFTImageFilter
inverseComplexFilter->Superclass::GetNameOfClass() --> ImageToImageFilter
This demonstrates that GetNameOfClass is overridden even though the filter object is otherwise referenced as an instance of the itk::ComplexToComplexFFTImageFilter base class inline.
ITK_EXERCISE_BASIC_OBJECT_METHODS internally calls GetNameOfClass on the instance directly and fails if the returned value does not match the string passed into the macro. Generally this is useful for validating that the object has been constructed within its expected hierarchy, however object factory instantiation intentionally violates the standard hierarchy by providing a subclass instance that has been upcast to the base class.
Potential Resolutions
- Avoid overriding the class name directly: it seems a little strange to allow the base class knowledge of its subclass, but is only a weak coupling as it currently exists and would require nontrivial effort to rearchitect. Would not favor this option.
- Update
ITK_EXERCISE_BASIC_OBJECT_METHODSto replaceGetNameOfClass()withSelf::GetNameOfClass(). This would result in the base class's name always being checked rather than the subclass's name. The change would remove the macro's ability to detect unexpected subclass names, but I'm doubtful that this is a particularly useful sensitivity as it currently exists. - Add a new
ITK_EXERCISE_BASIC_OBJECT_METHODS_SELFmacro replacingGetNameOfClass()withSelf::GetNameOfClass(). This would avoid weakening theITK_EXERCISE_BASIC_OBJECT_METHODSmacro for current checks and would be used only in cases where an upcast class is expected, such as objects instantiated from the object factory. However, this adds additional overhead.
Not sure if the comment in https://github.com/InsightSoftwareConsortium/ITK/pull/2286#discussion_r575685116 stemmed also from the underlying reason here, and whether the cases in itkPolygonGroupSpatialObjectXMLFileTest reported in https://github.com/InsightSoftwareConsortium/ITK/pull/2286#issuecomment-778559383 are also related.
If they are, maybe applying the proposed fix in #3010 may be appropriate.
@tbirdso @jhlegarreta did #3010 address this?
Unfortunately, it didn't fully address the issue Matt. The observed behavior is weird/inconsistent: the fix works on Windows/MSVC, but it does not on Linux and macOS.
#3022 provides a temporary fix in ITK_EXERCISE_BASIC_OBJECT_METHODS removing the strcmp so that tests compiled with GCC do not fail here, however this is not sufficient to close this issue. The problem remains as to how object->Self and object->Superclass must be written so that GCC successfully references the base and parent classes. @Leengit offered good suggestions in #3022 in regards to templates that can be explored.
There appears to be nothing wrong with this test, but it fails. Declaration and test invocation both look fine to me. Is this another instance of this issue?
In case the test output link rots, here is the tail:
Name of Class = PointSetToImageFilter
Name of Superclass = PointSetToImageFilter
Class name provided does not match object's NameOfClass
I like the approach of creating and using ITK_EXERCISE_BASIC_OBJECT_METHODS_SELF. If some day we figure out how to merge it into ITK_EXERCISE_BASIC_OBJECT_METHODS then we can quickly find and modify all the uses of the former to be the latter. And at that time, we can redefine the deprecated macro to call the merged macro and also issue a warning that it is deprecated.