Skip to content

COMP: Spell out Montage AllPeakInterpolationMethods return type#6653

Merged
hjmjohnson merged 1 commit into
InsightSoftwareConsortium:mainfrom
hjmjohnson:comp-montage-wrap-return-type
Jul 16, 2026
Merged

COMP: Spell out Montage AllPeakInterpolationMethods return type#6653
hjmjohnson merged 1 commit into
InsightSoftwareConsortium:mainfrom
hjmjohnson:comp-montage-wrap-return-type

Conversation

@hjmjohnson

@hjmjohnson hjmjohnson commented Jul 16, 2026

Copy link
Copy Markdown
Member

igenerator cannot resolve a deduced return type: it emits the literal ?unknown? into itkPhaseCorrelationOptimizer.i, which swig then rejects, failing the itkTileMontage wrapping target on all three Python builds. Spelling out the std::array type restores wrapping and keeps the -Winit-list-lifetime fix from 6f0a280 intact.

Companion to #6652: the two together account for every build error on the current nightly dashboard (this one the 3 Python builds, #6652 the 6 macOS dbg builds). They are independent — no merge ordering required.

Failing builds (Nightly 20260716-0100)
Build Errors
MacOS-Rel-Python 4
Ubuntu-22.04-gcc11.4-Rel-Python 4
Windows11-VS22x64-Release-Python 2
FAILED: Wrapping/Modules/Montage/itkTileMontagePython.cpp
swig -c++ -python ... -Werror ... .../Wrapping/Typedefs/itkTileMontage.i
.../Wrapping/Typedefs/itkPhaseCorrelationOptimizer.i:249: Error: Syntax error in input(3).

Reproduces identically on macOS, Linux and Windows, so it is source rather than toolchain. The failure appeared with the first nightly after 6f0a280 (2026-07-15) landed.

Root cause

6f0a280 fixed a genuine -Winit-list-lifetime defect by returning a std::array by value instead of a std::initializer_list whose backing array did not outlive the return. In doing so it also changed the return type to a deduced one:

static constexpr auto
AllPeakInterpolationMethods()
{
  return std::array{ /* ... */ };
}

igenerator cannot name that type, and emits it literally. Line 249 of the generated itkPhaseCorrelationOptimizer.i:

class itkPhaseCorrelationOptimizerEnums {
  public:
    enum class PeakInterpolationMethod: uint8_t {  None,  Parabolic,  Cosine,  WeightedMeanPhase };

    static ?unknown? AllPeakInterpolationMethods();

swig fails on the ?. Naming the type makes igenerator emit a declaration it can wrap:

static std::array<itkPhaseCorrelationOptimizerEnums::PeakInterpolationMethod, 4> AllPeakInterpolationMethods();

The lifetime fix is unaffected — the array is still returned by value. Only the return type is spelled out.

Local validation

Generated the real .i (castxml + igenerator) and ran the exact swig invocation and version CI uses (4.4.1, fetched via ITK_SWIG_VERSION), with CI's full flag set including -Werror:

  • Fail-before: reproduced CI's message byte-for-byte — itkPhaseCorrelationOptimizer.i:249: Error: Syntax error in input(3).
  • Pass-after: ?unknown? is gone; swig exits 0 on itkPhaseCorrelationOptimizer.i and, more importantly, on itkTileMontage.i — the target CI actually fails on.
  • itkPhaseCorrelationOptimizer.cxx.o compiles with 0 errors and 0 warnings, so the -Winit-list-lifetime fix still holds.

pre-commit run --all-files exits 0.

Note on wrapping-visible signatures

A deduced return type on a wrapped public API silently degrades to ?unknown? rather than failing loudly at the point of authorship, and swig only rejects it later while wrapping an unrelated class that imports the header. AllPeakInterpolationMethods() currently has no live callers — the three references in itkMontageTestHelper.hxx and itkPairwiseTestHelper.hxx are commented out — so the construct was not exercised by anything except the wrapping pass.

AI assistance
  • Tool: Claude Code (claude-opus-4-8)
  • Role: CDash triage and root-cause analysis
  • Contribution: generated the real .i to identify the ?unknown? emission after an initial hypothesis (swig cannot parse constexpr auto) failed to reproduce the reported syntax error against swig 4.4.1
  • All code was reviewed, built, and tested locally before committing

igenerator cannot resolve a deduced return type and emits the literal
"?unknown?" into itkPhaseCorrelationOptimizer.i, which swig rejects
with a syntax error, failing the itkTileMontage wrapping target on
every Python build.

Naming the std::array type keeps the initializer_list lifetime fix
and lets igenerator emit a wrappable declaration.
@github-actions github-actions Bot added type:Compiler Compiler support or related warnings area:Registration Issues affecting the Registration module labels Jul 16, 2026
@hjmjohnson
hjmjohnson marked this pull request as ready for review July 16, 2026 14:21

@hjmjohnson hjmjohnson left a comment

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

I over optimized to use "auto" so that future changes were easier (remove magic parameter 4), but swig got confused. This is still an easy fix as compiler error will aleart if the return array has a different size than 4, so a quickly exposed issue.

@greptile-apps

greptile-apps Bot commented Jul 16, 2026

Copy link
Copy Markdown
Contributor

Greptile Summary

This PR makes the peak-interpolation helper visible to the wrapping pipeline. The main changes are:

  • Spells out the four-element std::array return type.
  • Preserves constexpr value-return semantics.
  • Documents the igenerator constraint.

Confidence Score: 5/5

This looks safe to merge.

No blocking issues found in the changed code. The explicit type matches the four returned enum values, and the function still returns its array by value.

T-Rex T-Rex Logs

What T-Rex did

  • Real parent interface generation was completed through CastXML and igenerator.
  • The remaining work is to allow the head target to finish, inspect its generated interface, and then run the dependent Wrapping/Modules/Montage/itkTileMontagePython.cpp SWIG target.
  • The attempted parent TileMontage target was stopped because it would pull in 480 compilation steps rather than immediately reaching SWIG.

View all artifacts

T-Rex Ran code and verified through T-Rex

Important Files Changed

Filename Overview
Modules/Registration/Montage/include/itkPhaseCorrelationOptimizer.h Replaces the deduced return type with the equivalent explicit std::array type for wrapper generation.

Reviews (1): Last reviewed commit: "COMP: Spell out Montage AllPeakInterpola..." | Re-trigger Greptile

@hjmjohnson
hjmjohnson merged commit becca0f into InsightSoftwareConsortium:main Jul 16, 2026
20 of 21 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

area:Registration Issues affecting the Registration module type:Compiler Compiler support or related warnings

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants