Skip to content

STYLE: Remove 6 no-op dynamic_casts (casting T* to T*) from Modules/Core#2360

Merged
dzenanz merged 1 commit intoInsightSoftwareConsortium:masterfrom
N-Dekker:Remove-no-op-dynamic_cast-from-Core
Mar 4, 2021
Merged

STYLE: Remove 6 no-op dynamic_casts (casting T* to T*) from Modules/Core#2360
dzenanz merged 1 commit intoInsightSoftwareConsortium:masterfrom
N-Dekker:Remove-no-op-dynamic_cast-from-Core

Conversation

@N-Dekker
Copy link
Copy Markdown
Contributor

@N-Dekker N-Dekker commented Mar 4, 2021

Removed six "no-op" dynamic_casts that just cast pointers to their own compile-time type from Modules/Core.

Removed six "no-op" dynamic_casts that just cast pointers to their own compile-time type from Modules/Core.
Copy link
Copy Markdown
Member

@dzenanz dzenanz left a comment

Choose a reason for hiding this comment

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

How did you identify these instances?

@N-Dekker
Copy link
Copy Markdown
Contributor Author

N-Dekker commented Mar 4, 2021

How did you identify these instances?

@dzenanz Thanks for asking! And thanks to you both for approving the pull request! We're hunting dynamic_casts as they do occasionally appear to cause troubles (segfaults) for ITKElastix on MacOS. I found these particular "no-op" cases by doing a #define dynamic_cast MyDynamicCast, with the following definition of MyDynamicCast:

#include <type_traits>

template<class TPointer, class U>
TPointer
MyDynamicCast(U* source)
{
  using T = typename std::remove_pointer<TPointer>::type;
  static_assert(!std::is_same<typename std::remove_cv<T>::type, typename std::remove_cv<U>::type>::value,
      "no-op dynamic_cast<T*>!");
  return dynamic_cast<T*>(source);
}

template <class T, class U>
T &
MyDynamicCast(U& source)
{
  static_assert(!std::is_same<typename std::remove_cv<T>::type, typename std::remove_cv<U>::type>::value,
      "no-op dynamic_cast<T&>!");
  return dynamic_cast<T &>(source);
}

This redefinition of dynamic_cast triggers a compilation error whenever trying to cast to its own compile-time type. Disclaimer: #define dynamic_cast is non-Standard, as far as I know. So it could do anything. 😸

I must say I'm not against all uses of dynamic_cast, but in cases where they can be avoided easily, I would rather have them removed.

@dzenanz dzenanz merged commit 5b3fc71 into InsightSoftwareConsortium:master Mar 4, 2021
hjmjohnson added a commit to hjmjohnson/ITK that referenced this pull request Apr 27, 2026
Make the MGHIO module a shared library like the
other IO modules.

ctest -R MGH
 1/14 Test InsightSoftwareConsortium#810: ITKIOMGHInDoxygenGroup ...........   Passed    0.05 sec
 2/14 Test InsightSoftwareConsortium#811: MGHFactoryCreationTest ...........   Passed    0.09 sec
 3/14 Test InsightSoftwareConsortium#812: MGHReadImagesTest_mgz ............   Passed    4.23 sec
 4/14 Test InsightSoftwareConsortium#813: MGHReadImagesTest_mgh ............   Passed    0.68 sec
 5/14 Test InsightSoftwareConsortium#814: MGHReadImagesTest_mgh.gz .........   Passed    4.11 sec
 6/14 Test InsightSoftwareConsortium#815: itkITKIOMGHInternalTests .........   Passed    0.08 sec
 7/14 Test InsightSoftwareConsortium#816: itkITKIOMGHOriginTest ............   Passed    4.16 sec
 8/14 Test InsightSoftwareConsortium#2356: MGHIOInDoxygenGroup ..............   Passed    0.04 sec
 9/14 Test InsightSoftwareConsortium#2357: MGHFactoryCreationTest ...........   Passed    0.07 sec
10/14 Test InsightSoftwareConsortium#2358: MGHReadImagesTest_mgz ............   Passed    4.23 sec
11/14 Test InsightSoftwareConsortium#2359: MGHReadImagesTest_mgh ............   Passed    0.68 sec
12/14 Test InsightSoftwareConsortium#2360: MGHReadImagesTest_mgh.gz .........   Passed    4.13 sec
13/14 Test InsightSoftwareConsortium#2361: itkMGHIOInternalTests ............   Passed    0.07 sec
14/14 Test InsightSoftwareConsortium#2362: itkMGHIOOriginTest ...............   Passed    4.13 sec
100% tests passed, 0 tests failed out of 14
hjmjohnson added a commit to hjmjohnson/ITK that referenced this pull request Apr 27, 2026
Make the MGHIO module a shared library like the
other IO modules.

ctest -R MGH
 1/14 Test InsightSoftwareConsortium#810: ITKIOMGHInDoxygenGroup ...........   Passed    0.05 sec
 2/14 Test InsightSoftwareConsortium#811: MGHFactoryCreationTest ...........   Passed    0.09 sec
 3/14 Test InsightSoftwareConsortium#812: MGHReadImagesTest_mgz ............   Passed    4.23 sec
 4/14 Test InsightSoftwareConsortium#813: MGHReadImagesTest_mgh ............   Passed    0.68 sec
 5/14 Test InsightSoftwareConsortium#814: MGHReadImagesTest_mgh.gz .........   Passed    4.11 sec
 6/14 Test InsightSoftwareConsortium#815: itkITKIOMGHInternalTests .........   Passed    0.08 sec
 7/14 Test InsightSoftwareConsortium#816: itkITKIOMGHOriginTest ............   Passed    4.16 sec
 8/14 Test InsightSoftwareConsortium#2356: MGHIOInDoxygenGroup ..............   Passed    0.04 sec
 9/14 Test InsightSoftwareConsortium#2357: MGHFactoryCreationTest ...........   Passed    0.07 sec
10/14 Test InsightSoftwareConsortium#2358: MGHReadImagesTest_mgz ............   Passed    4.23 sec
11/14 Test InsightSoftwareConsortium#2359: MGHReadImagesTest_mgh ............   Passed    0.68 sec
12/14 Test InsightSoftwareConsortium#2360: MGHReadImagesTest_mgh.gz .........   Passed    4.13 sec
13/14 Test InsightSoftwareConsortium#2361: itkMGHIOInternalTests ............   Passed    0.07 sec
14/14 Test InsightSoftwareConsortium#2362: itkMGHIOOriginTest ...............   Passed    4.13 sec
100% tests passed, 0 tests failed out of 14
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