Skip to content

COMP: Fix Mac nightly compile errors (VTI test + ImageAlgorithm CTAD)#6219

Merged
dzenanz merged 3 commits intoInsightSoftwareConsortium:mainfrom
hjmjohnson:fix-cdash-2026-05-05
May 6, 2026
Merged

COMP: Fix Mac nightly compile errors (VTI test + ImageAlgorithm CTAD)#6219
dzenanz merged 3 commits intoInsightSoftwareConsortium:mainfrom
hjmjohnson:fix-cdash-2026-05-05

Conversation

@hjmjohnson
Copy link
Copy Markdown
Member

@hjmjohnson hjmjohnson commented May 6, 2026

Fix two compile-error clusters first observed on the 2026-05-05 nightly dashboard: a const-correctness slip in itkVTIImageIOReadWriteTest.cxx that surfaces only when ITK_FUTURE_LEGACY_REMOVE=ON (Mac dbg-Universal builds), and a CTAD deduction failure in itkImageAlgorithm.hxx that surfaces only on AppleClang's stricter deduction (Mac Python + Mac remote-modules builds). Both fixes are one-/two-line changes with no behavioral effect at runtime.

A third commit adds a GoogleTest regression covering ImageAlgorithm::Copy with RLEImage source/destination — the CTAD bug was discovered while ingesting ITKMorphologicalContourInterpolation (PR #6209), where it caused SIGABRT on gcc-11 and a compile error on AppleClang 17.

Failing CDash buildgroups and counts

Source: https://open.cdash.org/index.php?project=Insight&date=2026-05-05

Buildgroup Build name Site err tfail Cause
Expected Nightly Mac14.x-AppleClang-dbg-Universal RogueResearch25 4 0 #1 (VTI)
Expected Nightly Mac13.x-AppleClang-dbg-Universal RogueResearch24 4 0 #1 (VTI)
Expected Nightly Mac10.15-AppleClang-dbg RogueResearch17 4 0 #1 (VTI)
Nightly Mac26.x-ClangMain-dbg-arm64 RogueResearch27 4 0 #1 (VTI)
Nightly Mac26.x-AppleClang-dbg-TSan RogueResearch27 4 0 #1 (VTI)
Nightly Mac15.x-AppleClang-dbg-ASanUBSan RogueResearch26 4 0 #1 (VTI)
Nightly Mac11.x-AppleClang-dbg RogueResearch20 4 0 #1 (VTI)
Nightly Mac12.x-AppleClang-dbg-arm64 RogueResearch19 1 0 #1 (VTI partial)
Expected Nightly Python Wrapping MacOS-Rel-Python ZukicM1.kitware 12 2 #2 (CTAD)
Expected Nightly Remote Modules MacOS-Rel-MorphologicalContourInterpolation ZukicM1.kitware 12 0 #2 (CTAD)

Total: 8 sites × 4 errors (#1) + 2 sites × 12 errors (#2) = 56 compile errors cleared.

Fix #1 — `itkVTIImageIOReadWriteTest.cxx` const-correctness under `ITK_FUTURE_LEGACY_REMOVE=ON`

Commit: COMP: Use GetModifiableImageIO() in VTI test for ITK_FUTURE_LEGACY_REMOVE builds

Failing line (introduced by PR #6032 merged 2026-05-01):

auto imageIO = reader->GetImageIO();    // line 92
imageIO->SetFileName(inputImage);        // line 93 — fails
ITK_TRY_EXPECT_NO_EXCEPTION(imageIO->ReadImageInformation()); // line 95 — fails
return internalMain<2>(inputImage, outputImage, imageIO, compress);  // line 104 — fails

Diagnostic (representative of all 4 errors):

itkVTIImageIOReadWriteTest.cxx:93:12: error: no matching member function for call to 'SetFileName'
itkVTIImageIOReadWriteTest.cxx:95:31: error: 'this' argument to member function 'ReadImageInformation'
                                            has type 'const ImageIOBase', but function is not marked const
itkVTIImageIOReadWriteTest.cxx:104:14: error: no matching function for call to 'internalMain'
itkVTIImageIOReadWriteTest.cxx:106:14: error: no matching function for call to 'internalMain'
4 errors generated.

Root cause: itkGetModifiableObjectMacro(ImageIO, ImageIOBase) expands differently based on ITK_FUTURE_LEGACY_REMOVE:

  • OFF (default) — emits all three: GetModifiableImageIO() (non-const), GetImageIO() const (returning const ImageIOBase *), and a legacy-fallback non-const GetImageIO(). The bare reader->GetImageIO() resolves to the non-const fallback; the test compiles.
  • ON (strict-future mode) — emits only GetModifiableImageIO() and GetImageIO() const; the bare reader->GetImageIO() returns const ImageIOBase *. auto deduces const, every subsequent call needing a non-const path errors out.

The PR-time CI green-lit the test because no per-PR builder configures ITK_FUTURE_LEGACY_REMOVE=ON. The Rogue Research Mac dbg-Universal nightly pipelines do, exposing the regression.

Fix: change the bare-name call to the modifiable getter — works under both legacy and strict-future expansions.

-  auto imageIO = reader->GetImageIO();
+  auto imageIO = reader->GetModifiableImageIO();
Fix #2 — `itkImageAlgorithm.hxx` CTAD deduction on AppleClang

Commit: COMP: Spell template args on ImageRegion iterators in DispatchedCopy

Failing lines (in ImageAlgorithm::DispatchedCopy):

ImageRegionConstIterator it(inImage, inRegion);    // line 56
ImageRegionIterator      ot(outImage, outRegion);  // line 57

Diagnostic:

itkImageAlgorithm.hxx:56:28: error: no viable constructor or deduction guide for deduction
                                    of template arguments of 'ImageRegionConstIterator'
itkImageAlgorithm.hxx:57:28: error: no viable constructor or deduction guide for deduction
                                    of template arguments of 'ImageRegionIterator'

Root cause: class-template-argument-deduction (CTAD) used without an explicit deduction guide. ImageScanlineConstIterator / ImageScanlineIterator (a few lines above, lines 39-40) ship deduction guides and compile cleanly; ImageRegionConstIterator / ImageRegionIterator do not. AppleClang's CTAD on the failing builds is stricter than gcc/MSVC, exposing the asymmetry.

The two surfaces affected (MacOS-Rel-Python ZukicM1.kitware, MacOS-Rel-MorphologicalContourInterpolation ZukicM1.kitware) trigger the same 12-error count because every consumer of ImageAlgorithm.hxx from those buildgroups instantiates the path through DispatchedCopy.

Fix: spell the template argument explicitly. Compiles on every toolchain, removes the deduction-guide dependency. Could alternatively add a deduction guide to itkImageRegionConstIterator.h / itkImageRegionIterator.h for parity with the Scanline iterators — that's a separate cleanup PR.

-  ImageRegionConstIterator it(inImage, inRegion);
-  ImageRegionIterator      ot(outImage, outRegion);
+  ImageRegionConstIterator<InputImageType> it(inImage, inRegion);
+  ImageRegionIterator<OutputImageType>     ot(outImage, outRegion);
Fix #3 — RLEImage GTest regression for ImageAlgorithm::Copy iterator deduction

Commit: ENH: Add RLEImage GTest covering ImageAlgorithm::Copy iterator deduction

The CTAD bug (fix #2) was first identified while ingesting ITKMorphologicalContourInterpolation (PR #6209). RLEImage is a partially-specialized Image-like template whose iterator types lack visible deduction guides at the itkImageAlgorithm.hxx parse site. On AppleClang 17 this produced 12 compile errors; on gcc-11 it silently selected the primary-template iterator and caused SIGABRT at runtime.

Two GoogleTest cases added to Modules/Filtering/RLEImage/test/:

  • RLEImageAlgorithmCopy.FromRLEToImage — copies a filled RLEImage into a standard Image; verifies all pixels via ImageRegionConstIterator.
  • RLEImageAlgorithmCopy.FromImageToRLE — copies a filled standard Image into an RLEImage; verifies all pixels via ImageRegionConstIterator<RLEType>.

Both were verified locally with Apple-clang 17 in ~/src/ITK-mci/build before this push.

What this PR does not address

Other 2026-05-05 dashboard symptoms have separate root causes and are out of scope here:

Each of those merits a separate fix PR if persistent.

Suggested follow-up

The recurring pattern — test passes per-PR, fails on nightly because nightly enables strict modes — argues for adding a Pixi-Cxx-Strict or equivalent GHA job that builds with -DITK_FUTURE_LEGACY_REMOVE=ON. That would have caught fix #1 at PR time and saved a 24-hour nightly cycle. Comparable rationale for an AppleClang-CTAD job for fix #2, though the case for that one is weaker (Apple-only, low frequency).

@github-actions github-actions Bot added type:Compiler Compiler support or related warnings 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 area:IO Issues affecting the IO module labels May 6, 2026
@hjmjohnson
Copy link
Copy Markdown
Member Author

@greptileai review this draft before I make it official

@greptile-apps

This comment was marked as resolved.

@hjmjohnson hjmjohnson marked this pull request as ready for review May 6, 2026 11:11
@hjmjohnson hjmjohnson force-pushed the fix-cdash-2026-05-05 branch from b808bf5 to 74f423e Compare May 6, 2026 11:14
hjmjohnson added 3 commits May 6, 2026 07:41
itkVTIImageIOReadWriteTest at line 92 took the bare-name 'GetImageIO()'
overload, which the 'itkGetModifiableObjectMacro(ImageIO, ImageIOBase)'
declaration only emits when 'ITK_FUTURE_LEGACY_REMOVE' is OFF (the
historical default).  When the future-strict mode is ON, the macro
emits only 'GetModifiableImageIO()' and 'const ImageIOBase * GetImageIO()
const'; the bare 'GetImageIO()' returns a const pointer and the test
fails to compile with four cascading errors:

error: no matching member function for call to 'SetFileName'
error: 'this' argument to 'ReadImageInformation' has type 'const
ImageIOBase', but function is not marked const
error: no matching function for call to 'internalMain' (line 104)
error: no matching function for call to 'internalMain' (line 106)

The PR-time CI green-lit the test because no per-PR builder configures
'ITK_FUTURE_LEGACY_REMOVE=ON'; the failure surfaced only after the
nightly Mac AppleClang dbg-Universal pipelines (Rogue Research 17/19/
20/24/25/26/27) ran, where the strict-future flag is set.  Switching
to 'GetModifiableImageIO()' compiles cleanly under both legacy and
strict-future macro expansions.

Observed on the 2026-05-05 nightly across every Mac dbg-Universal /
ASanUBSan / TSan / arm64 build site (5+ buildgroups, 4 errors per
site).  No behavioral change for runtime users — the call site needed
a non-const ImageIOBase pointer all along.
itkImageAlgorithm::DispatchedCopy(...) at lines 56-57 declares the
input/output region iterators via class-template-argument-deduction
('ImageRegionConstIterator it(inImage, inRegion);').  AppleClang in
the dbg-Universal builds with strict CTAD checking refuses the
deduction with 'no viable constructor or deduction guide for deduction
of template arguments of "ImageRegionConstIterator"' / "ImageRegionIterator".

The neighboring 'ImageScanline*Iterator' declarations a few lines above
ship their own deduction guides and are accepted on the same compiler;
the Region iterators do not.  Spelling out the template arguments
explicitly compiles cleanly on every supported toolchain and removes
the dependence on a deduction guide that AppleClang's CTAD does not
synthesize.

Observed on the 2026-05-05 nightly:

  - MacOS-Rel-Python (12 errors)
  - MacOS-Rel-MorphologicalContourInterpolation (12 errors)

Both buildgroups configure with stricter CTAD than the per-PR Linux
and Windows builders, which is why the issue surfaced only at nightly
time.  No behavioral or runtime change.
Locks in correct iterator type deduction for itk::ImageAlgorithm::Copy
when the source or destination is a partially-specialized Image-like
template (RLEImage). Discovered while ingesting the
MorphologicalContourInterpolation remote module: the prior CTAD form
in itkImageAlgorithm.hxx silently selected the primary-template
iterator for RLEImage on gcc (runtime SIGABRT) while AppleClang 17
refused to compile.  The companion fix in this branch spells the
template arguments explicitly; this regression test exercises both
copy directions so a future CTAD re-introduction is caught at test
time on every supported toolchain. The same trap can affect any
future user of itk::ImageAlgorithm::Copy with a partially-specialized
Image-like template.
@hjmjohnson hjmjohnson force-pushed the fix-cdash-2026-05-05 branch from 74f423e to 020f37d Compare May 6, 2026 12:48
@github-actions github-actions Bot added type:Infrastructure Infrastructure/ecosystem related changes, such as CMake or buildbots area:Python wrapping Python bindings for a class area:Filtering Issues affecting the Filtering module labels May 6, 2026
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.

Thanks for follow-up.

@dzenanz
Copy link
Copy Markdown
Member

dzenanz commented May 6, 2026

I could not write an inline comment the normal way. In the first commit, I have a question. I feel like reader->GetImageIO() is quite widespread. Why is this the only place where reader->GetModifiableImageIO() is needed?

@hjmjohnson
Copy link
Copy Markdown
Member Author

@dzenanz Good question. I grep'd every GetImageIO() consumer in the tree (33 hits across 14 files) and classified each by what it does with the returned pointer.

The VTI test is the only call site that needs a non-const ImageIOBase *. Every other consumer is one of:

  • Pure-read — calls only const methods (GetSpacing, GetOrigin, GetPixelType, GetNumberOfDimensions, GetMetaDataDictionary const overload, Print const, etc.). A const ImageIOBase * is sufficient. (25 hits across 9 files: itkFileListVideoIOTest.cxx, itkTestDriverInclude.cxx, itkBinaryFillholeImageFilterTest1.cxx, itkSLICImageFilterTest.cxx, itkImageSeriesReader.hxx, itkLSMImageIOTest.cxx, itkGDCMImageIOTest2.cxx, itkBMPImageIOTest.cxx, itkBMPImageIOTest2.cxx)
  • Already typed const ImageIOBase * — e.g. LinearAnisotropicDiffusionCommandLine.h:103,113 and itkVTIImageIODirectionGTest.cxx:72,85 (the ExpectDirectionMatchesOblique helper takes const itk::ImageIOBase *).
  • Comparison-onlyITK_TEST_SET_GET_VALUE(...) macro reads the pointer for value comparison; no mutation. (itkImageSeriesWriterTest.cxx:113, itkGDCMImageReadSeriesWriteTest.cxx:83)

The VTI test (line 92) is unique because it (a) uses auto, (b) calls the non-const SetFileName() and ReadImageInformation() on the returned pointer, and (c) passes the pointer to internalMain<N> which declares its parameter as itk::ImageIOBase::Pointer (non-const smart pointer; a const ImageIOBase * can't bind).

So GetModifiableImageIO() is genuinely needed only here. A tree-wide migration would obscure that the rest of the codebase is intentionally read-only against the IO. The const-correctness rule: read-only → GetImageIO(); mutate or pass to non-const-pointer function → GetModifiableImageIO().

Separately, the recurring "test passes per-PR, fails on nightly because nightly enables strict modes" pattern argues for a Pixi-Cxx-Strict GHA job with -DITK_FUTURE_LEGACY_REMOVE=ON so the next instance is caught at PR time. Happy to follow up with that as a separate PR.

@dzenanz
Copy link
Copy Markdown
Member

dzenanz commented May 6, 2026

Thanks for the investigation. Good to merge when CI comes back green.

@dzenanz dzenanz merged commit 450baaa into InsightSoftwareConsortium:main May 6, 2026
19 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:Filtering Issues affecting the Filtering module area:IO Issues affecting the IO module area:Python wrapping Python bindings for a class type:Compiler Compiler support or related warnings 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

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants