Skip to content

COMP: Move inline = default destructors to .cxx for 30 exported classes#6041

Merged
hjmjohnson merged 1 commit intoInsightSoftwareConsortium:release-5.4from
thewtex:backport-inline-default
Apr 11, 2026
Merged

COMP: Move inline = default destructors to .cxx for 30 exported classes#6041
hjmjohnson merged 1 commit intoInsightSoftwareConsortium:release-5.4from
thewtex:backport-inline-default

Conversation

@thewtex
Copy link
Copy Markdown
Member

@thewtex thewtex commented Apr 10, 2026

Non-template exported classes with ~ClassName() override = default; inline in the header have hidden D1Ev/D0Ev destructor thunks under -fvisibility-inlines-hidden, even when the class vtable and typeinfo remain exported. This breaks dynamic_cast across DSO boundaries in shared library builds.

Moving the destructor definition out of line to the .cxx file ensures it is compiled in exactly one translation unit, giving it default (exported) visibility and correctly anchoring all destructor thunks.

Affected modules: ITKCommon (3), ITKIOImageBase (3), ITKFEM (1), ITKOptimizers (11), ITKStatistics (8), ITKWatersheds (2), ITKVideoCore (1).

See: #6000

This is a backport to @hjmjohnson 's patch to ITK 5.

Non-template exported classes with `~ClassName() override = default;`
inline in the header have hidden D1Ev/D0Ev destructor thunks under
-fvisibility-inlines-hidden, even when the class vtable and typeinfo
remain exported.  This breaks dynamic_cast across DSO boundaries in
shared library builds.

Moving the destructor definition out of line to the .cxx file ensures
it is compiled in exactly one translation unit, giving it default
(exported) visibility and correctly anchoring all destructor thunks.

Affected modules: ITKCommon (3), ITKIOImageBase (3), ITKFEM (1),
ITKOptimizers (11), ITKStatistics (8), ITKWatersheds (2), ITKVideoCore (1).

See: InsightSoftwareConsortium#6000
@github-actions github-actions bot added type:Compiler Compiler support or related warnings area:Core Issues affecting the Core module area:IO Issues affecting the IO module area:Segmentation Issues affecting the Segmentation module area:Video Issues affecting the Video module area:Numerics Issues affecting the Numerics module labels Apr 10, 2026
@greptile-apps
Copy link
Copy Markdown
Contributor

greptile-apps bot commented Apr 10, 2026

Greptile Summary

This PR moves 30 exported non-template class destructors from inline = default in headers to out-of-line = default in their corresponding .cxx files, fixing hidden D1Ev/D0Ev destructor thunks under -fvisibility-inlines-hidden that break dynamic_cast across DSO boundaries in shared-library builds. The change is a mechanical, consistent backport of the fix from the main branch to release-5.4, covering 7 modules (ITKCommon, ITKIOImageBase, ITKFEM, ITKOptimizers, ITKStatistics, ITKWatersheds, ITKVideoCore).

The PR description states "ITKOptimizers (11)" but the changeset includes 12 optimizer classes (itkVersorTransformOptimizer is the 12th). This is a documentation-only discrepancy with no code impact.

Confidence Score: 5/5

This PR is safe to merge — all 30 destructor moves are mechanically correct and consistently applied across all 60 changed files.

Every header/source pair follows the same correct pattern: declaration-only ~ClassName() override; in the header, out-of-line ClassName::~ClassName() = default; in the .cxx. No logic is changed, no API is broken, and the fix directly addresses the documented ABI issue with -fvisibility-inlines-hidden. The only finding is a minor off-by-one in the PR description ('11' optimizers vs. 12 actual), which has no code impact.

No files require special attention — all changes are uniform and correct.

Important Files Changed

Filename Overview
Modules/Core/Common/include/itkEquivalencyTable.h Removes inline = default destructor; now declares ~EquivalencyTable() override; only — correct pattern
Modules/Core/Common/src/itkEquivalencyTable.cxx Adds out-of-line EquivalencyTable::~EquivalencyTable() = default; — anchors destructor thunks correctly
Modules/Core/Common/include/itkLoggerManager.h Inline = default destructor removed; declaration-only ~LoggerManager() override; retained — correct
Modules/Core/Common/src/itkLoggerManager.cxx Out-of-line destructor added at top of translation unit — correct placement
Modules/Numerics/Optimizers/include/itkOptimizer.h Declaration-only destructor ~Optimizer() override; — correct, consistent with the rest of the PR
Modules/Numerics/Optimizers/src/itkOptimizer.cxx Out-of-line Optimizer::~Optimizer() = default; added — correctly anchors vtable and destructor thunks
Modules/Numerics/FEM/include/itkFEMLightObject.h Declaration-only destructor left in header — correct fix for DSO visibility
Modules/Numerics/FEM/src/itkFEMLightObject.cxx Out-of-line FEMLightObject::~FEMLightObject() = default; added inside nested itk::fem namespace — correct
Modules/Numerics/Statistics/src/itkChiSquareDistribution.cxx Out-of-line destructor placed correctly before the constructor definition — matches established pattern
Modules/Video/Core/src/itkTemporalProcessObject.cxx Out-of-line TemporalProcessObject::~TemporalProcessObject() = default; added — correct
Modules/Segmentation/Watersheds/src/itkOneWayEquivalencyTable.cxx Out-of-line destructor definition added at top of namespace block — correct and consistent

Flowchart

%%{init: {'theme': 'neutral'}}%%
flowchart TD
    A["Class declared with ITK*_EXPORT macro\n(shared library build)"]
    A --> B{Destructor placement?}
    B -- "Before (inline = default in header)" --> C["Compiled into every TU that includes header\nfvisibility-inlines-hidden hides D1Ev/D0Ev thunks\nBreaks dynamic_cast across DSO boundaries"]
    B -- "After (out-of-line = default in .cxx)" --> D["Compiled into exactly one TU\nDestructor has default (exported) visibility\nAll destructor thunks anchored and exported"]
    C --> E["❌ dynamic_cast fails across DSO boundaries\n(shared library builds)"]
    D --> F["✅ dynamic_cast works correctly\nacross DSO boundaries"]
Loading

Reviews (1): Last reviewed commit: "COMP: Move inline = default destructors ..." | Re-trigger Greptile

Copy link
Copy Markdown
Member

@hjmjohnson hjmjohnson left a comment

Choose a reason for hiding this comment

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

Thanks Matt!

@hjmjohnson hjmjohnson merged commit 7330169 into InsightSoftwareConsortium:release-5.4 Apr 11, 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:IO Issues affecting the IO module area:Numerics Issues affecting the Numerics module area:Segmentation Issues affecting the Segmentation module area:Video Issues affecting the Video module type:Compiler Compiler support or related warnings

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants