Skip to content

STYLE: Replace declare-then-assign with const auto for FixedArray-based types#6014

Merged
hjmjohnson merged 1 commit intoInsightSoftwareConsortium:mainfrom
hjmjohnson:style-sizetype-filled-assign
Apr 9, 2026
Merged

STYLE: Replace declare-then-assign with const auto for FixedArray-based types#6014
hjmjohnson merged 1 commit intoInsightSoftwareConsortium:mainfrom
hjmjohnson:style-sizetype-filled-assign

Conversation

@hjmjohnson
Copy link
Copy Markdown
Member

Summary

Replaces the two-line declare-then-assign pattern with a single const auto declaration for local variables of FixedArray-based types (SizeType, IndexType, PointType, VectorType) where the variable is not modified after initialization.

Before:
```cpp
InputImageType::SizeType extentSize;
extentSize = reader->GetOutput()->GetLargestPossibleRegion().GetSize();
```

After:
```cpp
const auto extentSize = reader->GetOutput()->GetLargestPossibleRegion().GetSize();
```

Scope

5 conversions across 5 test files. Cases where the variable was reassigned or mutated after the initial assignment (e.g. index[1] += 2, tempIndex -= offset) were intentionally left unchanged.

Also removes two now-unused using IndexType = ... type aliases in the Statistics subsample tests, which were only referenced by the replaced variable declarations.

Motivation

Companion to #6010 (merged) which handled the Type v; v.Fill(literal)constexpr auto v = Type::Filled(literal) pattern. This PR addresses the Type v; v = expr;const auto v = expr; pattern for runtime-expression RHS.

Unlike ::Filled() which allows constexpr, const auto is used here since the RHS is a runtime expression.

🤖 Generated with Claude Code

@github-actions github-actions bot added 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 type:Style Style changes: no logic impact (indentation, comments, naming) area:Numerics Issues affecting the Numerics module labels Apr 5, 2026
@greptile-apps
Copy link
Copy Markdown
Contributor

greptile-apps bot commented Apr 5, 2026

Greptile Summary

This PR is a pure style refactoring across 42 test files that replaces the two-line declare-then-assign pattern (Type v; v = expr;) with the single-line const auto v = expr; idiom for local variables of FixedArray-based types (SizeType, IndexType, PointType, VectorType) that are not mutated after initialization. It also removes two now-unused using IndexType = ... type aliases in the Statistics subsample tests that were only referenced by the replaced variable declarations.

Confidence Score: 5/5

This PR is safe to merge — all changes are pure style refactoring with no logic modifications.

All 42 files contain only the mechanical replacement of two-line declare-then-assign patterns with single-line const auto declarations for locally immutable FixedArray-based variables. Cases where variables are mutated after initialization were correctly left unchanged. The two removed IndexType aliases are genuinely unused after the conversion. No logic, algorithms, or public API are affected. All findings are at most P2.

No files require special attention.

Important Files Changed

Filename Overview
Modules/Core/Common/test/itkAnnulusOperatorGTest.cxx Replaces two-line declare-then-assign with const auto for AnnulusOperator size type
Modules/Core/Common/test/itkBSplineInterpolationWeightFunctionTest.cxx Applies const auto pattern to BSpline interpolation weight function test declarations
Modules/Core/Common/test/itkImageRandomNonRepeatingIteratorWithIndexGTest.cxx Applies const auto pattern to iterator index variable in GTest
Modules/Core/Common/test/itkVersorTest.cxx Applies const auto pattern to Versor test FixedArray-based declarations
Modules/Core/ImageFunction/test/itkBSplineInterpolateImageFunctionTest.cxx Applies const auto pattern to BSpline image interpolation function test
Modules/Core/Transform/test/itkBSplineDeformableTransformTest2.cxx Applies const auto pattern to BSpline deformable transform test 2
Modules/Core/Transform/test/itkBSplineDeformableTransformTest3.cxx Applies const auto pattern to BSpline deformable transform test 3
Modules/Core/Transform/test/itkBSplineTransformInitializerTest1.cxx Applies const auto pattern to BSpline transform initializer test
Modules/Core/Transform/test/itkBSplineTransformTest2.cxx Applies const auto pattern to BSpline transform test 2
Modules/Core/Transform/test/itkBSplineTransformTest3.cxx Applies const auto pattern to BSpline transform test 3
Modules/Core/Transform/test/itkComposeScaleSkewVersor3DTransformTest.cxx Applies const auto pattern to ComposeScaleSkewVersor3D transform test
Modules/Core/Transform/test/itkEuler2DTransformTest.cxx Applies const auto pattern to Euler2D transform test
Modules/Core/Transform/test/itkQuaternionRigidTransformTest.cxx Applies const auto pattern to quaternion rigid transform test
Modules/Core/Transform/test/itkRigid2DTransformTest.cxx Applies const auto pattern to Rigid2D transform test
Modules/Core/Transform/test/itkRigid3DTransformTest.cxx Applies const auto pattern to Rigid3D transform test
Modules/Core/Transform/test/itkScaleSkewVersor3DTransformTest.cxx Applies const auto pattern to ScaleSkewVersor3D transform test
Modules/Core/Transform/test/itkScaleVersor3DTransformTest.cxx Applies const auto pattern to ScaleVersor3D transform test
Modules/Core/Transform/test/itkSimilarity2DTransformTest.cxx Applies const auto pattern to Similarity2D transform test
Modules/Core/Transform/test/itkSimilarity3DTransformTest.cxx Applies const auto pattern to Similarity3D transform test
Modules/Core/Transform/test/itkVersorRigid3DTransformTest.cxx Applies const auto pattern to VersorRigid3D transform test
Modules/Core/Transform/test/itkVersorTransformTest.cxx Applies const auto pattern to Versor transform test
Modules/Filtering/LabelMap/test/itkLabelMapTest.cxx Applies const auto pattern to label map test declarations
Modules/IO/DCMTK/test/itkDCMTKImageIONoPreambleTest.cxx Applies const auto pattern to DCMTK IO no-preamble test
Modules/IO/GDCM/test/itkGDCMImagePositionPatientTest.cxx Applies const auto pattern to GDCM image position patient test
Modules/Numerics/Statistics/test/itkGaussianRandomSpatialNeighborSubsamplerTest.cxx Removes unused IndexType alias; uses const auto for index variable
Modules/Numerics/Statistics/test/itkImageToHistogramFilterTest.cxx Applies const auto pattern to image-to-histogram filter test
Modules/Numerics/Statistics/test/itkUniformRandomSpatialNeighborSubsamplerTest.cxx Removes unused IndexType alias; uses const auto for index variable
Modules/Registration/Metricsv4/test/itkEuclideanDistancePointSetMetricTest2.cxx Applies const auto pattern to Euclidean distance point-set metric test
Modules/Registration/PDEDeformable/test/itkDemonsRegistrationFilterTest.cxx Applies const auto pattern to Demons registration filter test
Modules/Registration/PDEDeformable/test/itkDiffeomorphicDemonsRegistrationFilterTest.cxx Applies const auto pattern to Diffeomorphic Demons registration test
Modules/Registration/PDEDeformable/test/itkFastSymmetricForcesDemonsRegistrationFilterTest.cxx Applies const auto pattern to Fast Symmetric Forces Demons test
Modules/Registration/PDEDeformable/test/itkLevelSetMotionRegistrationFilterTest.cxx Applies const auto pattern to level-set motion registration test
Modules/Registration/PDEDeformable/test/itkSymmetricForcesDemonsRegistrationFilterTest.cxx Applies const auto pattern to Symmetric Forces Demons registration test
Modules/Segmentation/LevelSets/test/itkCollidingFrontsImageFilterTest.cxx Applies const auto pattern to colliding fronts image filter test
Modules/Segmentation/LevelSets/test/itkShapePriorMAPCostFunctionTest.cxx Applies const auto pattern to ShapePriorMAP cost function test
Modules/Segmentation/LevelSets/test/itkShapePriorSegmentationLevelSetFunctionTest.cxx Applies const auto pattern to shape prior segmentation level-set test
Modules/Segmentation/LevelSetsv4/test/itkMultiLevelSetChanAndVeseInternalTermTest.cxx Applies const auto pattern to multi-level-set ChanAndVese internal term test
Modules/Segmentation/LevelSetsv4/test/itkMultiLevelSetDenseImageTest.cxx Applies const auto pattern to multi-level-set dense image test
Modules/Segmentation/LevelSetsv4/test/itkMultiLevelSetEvolutionTest.cxx Applies const auto pattern to multi-level-set evolution test
Modules/Segmentation/LevelSetsv4/test/itkTwoLevelSetMalcolmImage2DTest.cxx Applies const auto pattern to two-level-set Malcolm 2D test
Modules/Segmentation/LevelSetsv4/test/itkTwoLevelSetShiImage2DTest.cxx Applies const auto pattern to two-level-set Shi 2D test
Modules/Segmentation/LevelSetsv4/test/itkTwoLevelSetWhitakerImage2DTest.cxx Applies const auto pattern to two-level-set Whitaker 2D test

Flowchart

%%{init: {'theme': 'neutral'}}%%
flowchart TD
    A["Two-line declare-then-assign pattern
    (e.g. SizeType sz; sz = img->GetSize())"] --> B{"Variable mutated
after assignment?
(e.g. sz[0] += 2)"}
    B -- Yes --> C["Left unchanged"]
    B -- No --> D["Replace with const auto
    const auto sz = img->GetSize()"]
    D --> E["Remove now-unused
type aliases
(using IndexType = ...)"]
Loading

Reviews (2): Last reviewed commit: "STYLE: Use explicit VectorType in itkVer..." | Re-trigger Greptile

@hjmjohnson hjmjohnson marked this pull request as draft April 5, 2026 14:02
@hjmjohnson
Copy link
Copy Markdown
Member Author

Local build + test verification on style-sizetype-filled-assign @ a06070a57f:

  • Host: macOS, Apple Clang, RelWithDebInfo, Ninja
  • Rebuilt test drivers: ITKCommon2TestDriver, ITKIOGDCMTestDriver, ITKStatisticsTestDriver
  • Build: 7 compile/link steps, zero warnings, zero errors
  • Tests run:
    • itkVersorTest — Passed (0.46s)
    • itkGDCMImagePositionPatientTest — Passed (0.39s)
    • itkGaussianRandomSpatialNeighborSubsamplerTest — Passed (0.38s)
    • itkUniformRandomSpatialNeighborSubsamplerTest — Passed (0.04s)
  • itkDCMTKImageIONoPreambleTest is in the PR but the DCMTK module is disabled in this local build config; not verified locally. CI will cover it.

All 4 locally-verifiable changes compile cleanly, produce no new warnings, and pass their ctest targets.

@hjmjohnson
Copy link
Copy Markdown
Member Author

Added one more conversion and verified:

  • Modules/Core/Common/test/itkBSplineInterpolationWeightFunctionTest.cxx — merge WeightsType weights; (line 258) with weights = function->Evaluate(position); (line 261) into a single initialized declaration. This case uses the explicit type (not const auto) because weights is subsequently passed by non-const reference to the 3-argument Evaluate(position, weights, startIndex) overload, so const would be invalid.

  • Commit: bfe9e5a9a2

Why this case was missed in the earlier scan: the previous regex-based scanner looked only at the immediately following non-blank line, which was an unrelated IndexType startIndex; declaration, not the target assignment. A hybrid clang-query + Python scanner that finds decl sites via the AST matcher varDecl(hasInitializer(cxxConstructExpr(argumentCountIs(0)))) and then scans forward for the first matching var = expr; correctly identifies it.

Local verification:

  • Build: ninja ITKCommon2TestDriver — 1659 steps, zero warnings, zero errors
  • Test: itkBSplineInterpolationWeightFunctionTest — Passed (0.62s)

@github-actions github-actions bot added the area:Registration Issues affecting the Registration module label Apr 5, 2026
@hjmjohnson
Copy link
Copy Markdown
Member Author

Added 7 more scope-shrink conversions discovered by a codebase-wide clang-query scan. Each was individually verified via whole-function grep to confirm the variable has no references outside its target loop body.

Commit: b306876026

Files changed (7):

  • Modules/Core/Transform/test/itkBSplineDeformableTransformTest2.cxxindex → loop-body const auto
  • Modules/Core/Transform/test/itkBSplineDeformableTransformTest3.cxxindex
  • Modules/Core/Transform/test/itkBSplineTransformTest2.cxxindex
  • Modules/Core/Transform/test/itkBSplineTransformTest3.cxxindex
  • Modules/Core/Transform/test/itkBSplineTransformInitializerTest1.cxxindex, displacement
  • Modules/Numerics/Statistics/test/itkImageToHistogramFilterTest.cxxindex
  • Modules/Registration/Metricsv4/test/itkEuclideanDistancePointSetMetricTest2.cxxtransformedPoint

Build + test verification:

  • ninja ITKTransformTestDriver ITKStatisticsTestDriver ITKMetricsv4TestDriver — 826 steps, zero warnings, zero errors
  • ctest9/9 tests pass: itkEuclideanDistancePointSetMetricTest2, itkImageToHistogramFilterTest, itkImageToHistogramFilterTest2, itkImageToHistogramFilterTest2_Auto, itkBSplineDeformableTransformTest2, itkBSplineDeformableTransformTest3, itkBSplineTransformTest2, itkBSplineTransformTest3, itkBSplineTransformInitializerTest1

Discovery method: Hybrid clang-query + Python scanner. The clang-query AST matcher varDecl(hasInitializer(cxxConstructExpr(argumentCountIs(0)))) finds every default-constructed local class-type variable (class types with default ctors aren't caught by cppcoreguidelines-init-variables, which is POD/scalar-only). The codebase-wide scan on 459 pre-filtered files found 48 candidates; after the whole-function-scope check, only 8 were true positives (~18%), matching the pattern shown here plus the earlier weights conversion in itkBSplineInterpolationWeightFunctionTest.cxx.

False positives rejected by the whole-function-scope check (7 candidates): iterator and path tests where a single IndexType index0; or IndexType pixelIndex; is reused across several sequential loops or mutated after the initial loop. The whole-function grep for the variable name is the critical filter — a "next-statement" check is insufficient because the later uses can be 100+ lines away.

@github-actions github-actions bot added the area:Segmentation Issues affecting the Segmentation module label Apr 5, 2026
@hjmjohnson
Copy link
Copy Markdown
Member Author

Wave 2 — 17 additional conversions (32/32 tests pass)

Added commit 781d5c8da9 with 17 more files converted across 5 test drivers.

New conversions

Sub-case A — const auto (never mutated):

File Variable Pattern
itkAnnulusOperatorGTest.cxx normalizedAnnulusSize simple merge
itkImageRandomNonRepeatingIteratorWithIndexGTest.cxx indexFill0, indexConstMatch scope-shrink into loop
itkBSplineInterpolateImageFunctionTest.cxx value (CovariantVectorType) simple merge inside if block
itkDemonsRegistrationFilterTest.cxx index scope-shrink into while loop
itkDiffeomorphicDemonsRegistrationFilterTest.cxx index scope-shrink into for loop
itkFastSymmetricForcesDemonsRegistrationFilterTest.cxx index scope-shrink into for loop
itkLevelSetMotionRegistrationFilterTest.cxx index scope-shrink into while loop
itkSymmetricForcesDemonsRegistrationFilterTest.cxx index scope-shrink into while loop
itkShapePriorMAPCostFunctionTest.cxx index already in loop body
itkShapePriorSegmentationLevelSetFunctionTest.cxx index ×2 already in loop body
itkMultiLevelSetDenseImageTest.cxx idx scope-shrink into loop
itkMultiLevelSetChanAndVeseInternalTermTest.cxx idx scope-shrink into loop
itkMultiLevelSetEvolutionTest.cxx idx scope-shrink into loop
itkTwoLevelSetMalcolmImage2DTest.cxx idx scope-shrink into loop
itkTwoLevelSetShiImage2DTest.cxx idx scope-shrink into loop
itkTwoLevelSetWhitakerImage2DTest.cxx idx scope-shrink into loop

Sub-case B — explicit type merge (mutable after init):

File Variable Reason
itkCollidingFrontsImageFilterTest.cxx tempIndex mutated via tempIndex -= offset

Build verification: 5 drivers rebuilt (ITKCommonGTestDriver, ITKImageFunctionTestDriver, ITKPDEDeformableRegistrationTestDriver, ITKLevelSetsTestDriver, ITKLevelSetsv4TestDriver), 0 warnings, 0 errors, 32/32 ctest targets passed.

PR summary (4 commits, 30 files total)

  • a06070a57f — 5 files (BSpline Metricsv4, Numerics/Statistics)
  • bfe9e5a9a2 — 1 file (BSpline interpolation weights, merge-without-const)
  • b306876026 — 7 files (BSpline transform tests, histogram, EuclideanDistance)
  • 781d5c8da9 — 17 files (PDEDeformable, LevelSets, LevelSetsv4, ImageFunction, Common)

@github-actions github-actions bot added the area:Filtering Issues affecting the Filtering module label Apr 5, 2026
@hjmjohnson hjmjohnson force-pushed the style-sizetype-filled-assign branch from f6a9bdd to c1f1cf8 Compare April 5, 2026 17:32
@hjmjohnson hjmjohnson force-pushed the style-sizetype-filled-assign branch from c1f1cf8 to a190f69 Compare April 5, 2026 19:21
hjmjohnson added a commit to hjmjohnson/ITK that referenced this pull request Apr 5, 2026
Replace `const auto xb = xa * sinangle` with `const VectorType xb`
so the type is immediately clear at the point of use, per N-Dekker's
review comment on PR InsightSoftwareConsortium#6014.
@hjmjohnson hjmjohnson marked this pull request as ready for review April 5, 2026 19:21
@hjmjohnson hjmjohnson requested a review from N-Dekker April 6, 2026 19:18
@hjmjohnson
Copy link
Copy Markdown
Member Author

Addressed in commit b781291. All auto replaced with explicit types across 39 files (126 replacements).

auto is now only used where the type is obvious from the RHS:

  • C-style cast: auto * h_idata = (TElement *)malloc(bytes);

For all method return types (TransformPoint(), TransformVector(), ComputeIndex(), GetSize(), EvaluateDerivativeAtContinuousIndex(), etc.), the explicit type is now stated. This follows the clang-tidy modernize-use-auto criteria (iterators, new, casts only) rather than AAA.

Merge the two-line pattern:

  Type var;
  var = expr;

into the single-line form:

  const Type var = expr;

across 49 files in Modules/ and Examples/. Variables that are never
modified after initialization are marked const. Explicit types are
used throughout -- auto is NOT used unless the type is blatantly
obvious from the RHS (cast or T::New()).

This follows the clang-tidy modernize-use-auto criteria (iterators,
new, casts only) rather than AAA ("auto almost always"), per review
feedback from N-Dekker and dzenanz.

Scope-shrink: where a variable was declared at function scope but
only used inside a loop body, the declaration is moved into the
loop to reduce its scope.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
@hjmjohnson hjmjohnson force-pushed the style-sizetype-filled-assign branch from b781291 to c79c308 Compare April 9, 2026 02:33
@hjmjohnson hjmjohnson requested review from N-Dekker and dzenanz April 9, 2026 03:03
Copy link
Copy Markdown
Contributor

@N-Dekker N-Dekker left a comment

Choose a reason for hiding this comment

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

Looks fine to me! Thank you for addressing my comments!

@N-Dekker
Copy link
Copy Markdown
Contributor

N-Dekker commented Apr 9, 2026

Replaces the two-line declare-then-assign pattern with a single const auto declaration for local variables of FixedArray-based types (SizeType, IndexType, PointType, VectorType)

For what it's worth, SizeType and IndexType may be FixedArray-like, but they are not "FixedArray-based" 🤓

No problem, of course, the term "FixedArray-based" is not in the proposed commit 👍

@hjmjohnson hjmjohnson merged commit fe96138 into InsightSoftwareConsortium:main Apr 9, 2026
18 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:Numerics Issues affecting the Numerics module area:Registration Issues affecting the Registration module area:Segmentation Issues affecting the Segmentation module 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.

3 participants