Skip to content

STYLE: Replace declare-then-assign with initialization at declaration#6044

Open
hjmjohnson wants to merge 3 commits intoInsightSoftwareConsortium:mainfrom
hjmjohnson:style-declare-then-assign-common
Open

STYLE: Replace declare-then-assign with initialization at declaration#6044
hjmjohnson wants to merge 3 commits intoInsightSoftwareConsortium:mainfrom
hjmjohnson:style-declare-then-assign-common

Conversation

@hjmjohnson
Copy link
Copy Markdown
Member

Merge variable declaration and subsequent assignment into single initialization-at-declaration across Modules/Core/Common (22 files). Adds const where clang-tidy misc-const-correctness identifies immutable variables on changed lines.

Changes by category

Forward-reference fixes (bugs in prior declare-then-init output):

  • itkPyImageFilter.hxxresult = PyObject_Call(..., args, ...) was moved above args = PyTuple_Pack(...), creating use-before-declaration. Fixed by preserving correct ordering.
  • itkImage.hxxnum = GetOffsetTable()[...] was moved above ComputeOffsetTable(), reading stale data. Fixed by keeping compute before read.

Dead code revival:

  • itkVectorGeometryTest.cxx — uncommented a test block with invalid C++ (VectorType vv = 0, 2, 4;) and rewrote with correct brace-initialization (VectorType vv{ { 0, 2, 4 } }).

Mechanical changes (declare-then-assign → init at declaration):

  • 19 additional files in Modules/Core/Common/{include,src,test}/
Validation
  • clang-tidy-diff.py (LLVM 22.1.2) run on changed lines with misc-const-correctness
  • difit local review (all changes inspected in GitHub-style UI)
  • One malformed clang-tidy auto-fix caught and corrected (const Type & constconst Type)

@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) labels Apr 13, 2026
@hjmjohnson hjmjohnson force-pushed the style-declare-then-assign-common branch 2 times, most recently from f526ece to 0456324 Compare April 13, 2026 01:05
@hjmjohnson
Copy link
Copy Markdown
Member Author

Do: test this please

@kwrobot-v1
Copy link
Copy Markdown

kwrobot-v1 bot commented Apr 13, 2026

Errors:

  • While processing the test command: the command is not supported

@hjmjohnson
Copy link
Copy Markdown
Member Author

@greptileai review this draft before I make it official

@greptile-apps

This comment was marked as resolved.

@github-actions github-actions bot removed the type:Infrastructure Infrastructure/ecosystem related changes, such as CMake or buildbots label Apr 13, 2026
@hjmjohnson hjmjohnson force-pushed the style-declare-then-assign-common branch from 4daba1b to 037539c Compare April 13, 2026 10:45
@github-actions github-actions bot added the type:Infrastructure Infrastructure/ecosystem related changes, such as CMake or buildbots label Apr 13, 2026
@hjmjohnson hjmjohnson force-pushed the style-declare-then-assign-common branch from 037539c to f01c8e2 Compare April 13, 2026 11:30
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.

Mostly looks good.

@hjmjohnson hjmjohnson force-pushed the style-declare-then-assign-common branch 2 times, most recently from 6d789c9 to 8a9ba71 Compare April 13, 2026 15:09
@github-actions github-actions bot added the area:Documentation Issues affecting the Documentation module label Apr 13, 2026
@hjmjohnson hjmjohnson force-pushed the style-declare-then-assign-common branch from 8a9ba71 to be7d9e3 Compare April 13, 2026 15:53
Mechanical conversion of two-line `T x; x = expr;` patterns to
single-line `T x = expr;` (or `const T x = expr;` where possible)
in Modules/Core/Common source and test files.

No behavioral change. Tests exercise the same operations as before.
@hjmjohnson hjmjohnson force-pushed the style-declare-then-assign-common branch from be7d9e3 to 4ca2f8f Compare April 13, 2026 16:05
@hjmjohnson hjmjohnson marked this pull request as ready for review April 13, 2026 16:07
Add verification of previously untested return values:
- itkMatrixTest: assert GetInverse(), GetTranspose(), and operator*
  produce expected results (eliminates -Wunused-but-set-variable)
- itkVectorGeometryTest: enable the commented-out VectorType brace
  initialization test and verify element values
Add section 6c covering the GCC -Wunused-but-set-variable warning
that fires after declare-then-assign refactoring converts `T x; x =
func();` to `const T x = func();` when the return value is never read.

Also add a checklist item in the refactoring quick-reference section.
Prefer adding test assertions in test code over suppressing with
[[maybe_unused]] or static_cast<void>().

References: PR InsightSoftwareConsortium#6044, CDash build 11198750.
@hjmjohnson hjmjohnson force-pushed the style-declare-then-assign-common branch from 4ca2f8f to 35a1b23 Compare April 13, 2026 16:53
@hjmjohnson hjmjohnson requested a review from dzenanz April 13, 2026 20:18
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 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