Skip to content

STYLE: Remove dead branches for compilers ITK cannot build with - #7121

Merged
hjmjohnson merged 2 commits into
InsightSoftwareConsortium:mainfrom
hjmjohnson:style-remove-dead-compiler-branches
Nov 5, 2026
Merged

hjmjohnson merged 2 commits into
InsightSoftwareConsortium:mainfrom
hjmjohnson:style-remove-dead-compiler-branches

Conversation

@hjmjohnson

@hjmjohnson hjmjohnson commented Nov 5, 2026 •

Copy link
Copy Markdown
Member

Remove preprocessor and CMake branches for compilers that cannot build ITK, plus one unused legacy macro. First cleanup from the survey in #7120; no behavior changes on any supported compiler.

Each compiler specific test was redundant with the "C++17" minimum standard which fails before those tests are reached.

What is removed

ITK requires C++17 and GCC 7, Clang 5, AppleClang 10, MSVC 19.20, or Intel 19.1.

  • itkMacro.h: the #error guards for SunPro, Borland, MetroWerks, and SGI MIPSpro. None of those compilers supports C++17. The guards for the supported compiler families and for Cygwin stay.
  • itkNumericTraits.h: the SunPro 32-bit PrintType = double workaround for long double.
  • itkMemoryUsageObserver.h and .cxx: the __SUNPRO_CC half of __SUNPRO_CC || __sun__. The Solaris platform test __sun__ stays.
  • itkTestDriver.cxx: the SGI MIPS and Cygwin library-path branches.
  • CMake/ITKModuleMacros.cmake: the BORLAND branch of itk_module_warnings_disable.
  • Modules/Core/Common/src/CMakeLists.txt: the probe for -fno-associative-math ("GCC 4.3 onwards"). GCC now always gets that flag for itkCompensatedSummation.cxx.
  • Modules/Numerics/Statistics/test/CMakeLists.txt: if(NOT MSVC90) around itkSampleToHistogramFilterTest4 (Visual Studio 2008).
  • itkConfigure.h.in: ITK_HAS_CPP11_TYPETRAITS, defined only without ITK_LEGACY_REMOVE and read by nothing in ITK. A GitHub code search finds it only in vendored copies of old ITK releases. A second commit notes the removal in the migration guide.
Left alone on purpose
Local verification (macOS arm64, Homebrew LLVM, C++17)
  • ITKCommon, itkTestDriver, and the three ITKCommon test drivers build with no compiler warnings; the 8 related tests pass (numeric traits, memory usage and probes, compensated summation, time probes).
  • pre-commit run --all-files passes.
  • Not verified here: the -fno-associative-math change, because this compiler is Clang and the branch is GCC-only; the itkSampleToHistogramFilterTest4 registration, because the Statistics tests are not in this build tree; and Windows. CI covers all three.
AI assistance

Claude Code (Fable 5.1) ran the survey, drafted the change, and ran the local checks. The author reviewed and is responsible for the change.

@github-actions github-actions Bot added type:Infrastructure Infrastructure/ecosystem related changes, such as CMake or buildbots type:Testing Ensure that the purpose of a class is met/the results on a wide set of test cases are correct area:Core Issues affecting the Core module type:Style Style changes: no logic impact (indentation, comments, naming) area:Numerics Issues affecting the Numerics module area:Documentation Issues affecting the Documentation module labels Nov 5, 2026
@hjmjohnson
hjmjohnson marked this pull request as ready for review November 5, 2026 18:24
@greptile-apps

greptile-apps Bot commented Nov 5, 2026 •

Copy link
Copy Markdown
Contributor

RetriggerView in GreptileConfidence Score: 5/5

[Medium impact] The PR appears safe to merge; no actionable issues were found.

Summary

Removes obsolete compiler branches and documents one legacy macro removal.

  • Common headers keep Solaris detection separate from unsupported compiler paths.
  • Build and test setup no longer uses branches for compilers ITK cannot support.
  • GCC builds set the compensated-summation flag directly.
  • ITK no longer defines the unused type-traits macro.

Intentional compatibility change: hjmjohnson explicitly identified removal of ITK_HAS_CPP11_TYPETRAITS as intentional because ITK does not read it. The migration guide tells downstream users how to update their checks.

Reviews (1) · Last reviewed commit: "DOC: Note the removal of ITK_HAS_CPP11_T..." · Reviewed by Greptile

ITK requires C++17 and GCC 7, Clang 5, AppleClang 10, MSVC 19.20, or
Intel 19.1. Branches for SunPro, Borland, MetroWerks, SGI MIPSpro,
Visual Studio 2008, and GCC older than 4.3 can never be taken, and
Cygwin is rejected by the guard that stays in itkMacro.h.

ITK_HAS_CPP11_TYPETRAITS was defined for legacy builds and read by
nothing in ITK.
The macro was public in legacy builds, so a project that tests it needs
to know which branch to keep.
@hjmjohnson

Copy link
Copy Markdown
Member Author

Force-push 54916e0 -> a2a4375 is a plain rebase on main (no content change): #7119 also appended to the end of the ITK 6 migration guide, so both sections are kept, in merge order.

@hjmjohnson
hjmjohnson force-pushed the style-remove-dead-compiler-branches branch from 54916e0 to a2a4375 Compare November 5, 2026 23:50
@hjmjohnson
hjmjohnson merged commit 4902ee9 into InsightSoftwareConsortium:main Nov 5, 2026
12 of 15 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

area:Core Issues affecting the Core module area:Documentation Issues affecting the Documentation module area:Numerics Issues affecting the Numerics module type:Infrastructure Infrastructure/ecosystem related changes, such as CMake or buildbots type:Style Style changes: no logic impact (indentation, comments, naming) type:Testing Ensure that the purpose of a class is met/the results on a wide set of test cases are correct

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants