Skip to content

Commit 8a9ba71

Browse files
committed
STYLE: Replace declare-then-assign with initialization at declaration
Merge variable declaration and subsequent assignment into a single initialization-at-declaration statement across Modules/Core/Common. Fixes forward-reference hazards in itkPyImageFilter.hxx (args used before declared) and itkImage.hxx (offset table read before computed). Preserves two-line form where copy/move assignment semantics are intentionally tested. Add const to immutable variables on changed lines.
1 parent 4c44a9f commit 8a9ba71

23 files changed

+118
-108
lines changed

Documentation/AI/compiler-cautions.md

Lines changed: 30 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -357,6 +357,35 @@ class Foo {
357357
};
358358
```
359359
360+
### 6c. `-Wunused-but-set-variable` — GCC (declare-then-assign refactoring)
361+
362+
After converting `T x; x = func();` to `const T x = func();`, GCC warns if
363+
`x` is never read downstream. Apple Clang and MSVC typically don't fire this.
364+
This is the most common warning introduced by STYLE: declare-then-assign
365+
refactoring commits.
366+
367+
```cpp
368+
// Triggers warning — computed but never tested:
369+
const MatrixType inverse = matrix.GetInverse();
370+
371+
// Fix for test code — add a meaningful assertion:
372+
const MatrixType inverse = matrix.GetInverse();
373+
if (itk::Math::NotExactlyEquals(inverse(0, 0), expected))
374+
{
375+
std::cerr << "Problem with GetInverse()" << std::endl;
376+
return EXIT_FAILURE;
377+
}
378+
379+
// Fix for production code — don't capture if unneeded:
380+
matrix.GetInverse(); // called for side effect only
381+
```
382+
383+
In test files, **always prefer adding a test assertion** over suppressing
384+
with `[[maybe_unused]]` or `static_cast<void>()`. Tests that compute values
385+
without checking them are a missed coverage opportunity.
386+
387+
**References:** PR #6044, CDash build 11198750.
388+
360389
---
361390

362391
## 7. MSVC-Specific Warnings
@@ -537,6 +566,7 @@ When refactoring existing code, verify each item:
537566
- [ ] `std::vector` replacing `new[]` — guard `data()` / `operator[]` calls on potentially empty vectors
538567
- [ ] Lambda captures — no unused captures; `constexpr` variables don't need capture
539568
- [ ] Loop variables initialized at declaration
569+
- [ ] Declare-then-assign conversions: if `const T x = func()` is never read downstream, add a test assertion (in test code) or restructure (in production code) — GCC `-Wunused-but-set-variable` fires on captured-but-unread return values
540570
- [ ] `bool` not used with `|=` for exit-status accumulation — use `int`
541571
- [ ] Third-party header includes wrapped with appropriate `ITK_CLANG_SUPPRESS_*` macros
542572
- [ ] Explicit template instantiations in shared-build modules marked `ITK_TEMPLATE_EXPORT`

Modules/Core/Common/include/itkConceptChecking.h

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -775,9 +775,8 @@ struct HasZero
775775
void
776776
constraints()
777777
{
778-
T a;
778+
T a = T{};
779779

780-
a = T{};
781780
Detail::IgnoreUnusedVariable(a);
782781
}
783782
};

Modules/Core/Common/include/itkImage.hxx

Lines changed: 1 addition & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -38,10 +38,8 @@ template <typename TPixel, unsigned int VImageDimension>
3838
void
3939
Image<TPixel, VImageDimension>::Allocate(bool initializePixels)
4040
{
41-
SizeValueType num;
42-
4341
this->ComputeOffsetTable();
44-
num = static_cast<SizeValueType>(this->GetOffsetTable()[VImageDimension]);
42+
SizeValueType num = static_cast<SizeValueType>(this->GetOffsetTable()[VImageDimension]);
4543

4644
m_Buffer->Reserve(num, initializePixels);
4745
}

Modules/Core/Common/include/itkPyImageFilter.hxx

Lines changed: 3 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -151,10 +151,8 @@ PyImageFilter<TInputImage, TOutputImage>::GenerateOutputInformation()
151151
// make sure that the CommandCallable is in fact callable
152152
if (PyCallable_Check(this->m_GenerateOutputInformationCallable))
153153
{
154-
PyObject * result;
155-
156154
PyObject * args = PyTuple_Pack(1, this->m_Self);
157-
result = PyObject_Call(this->m_GenerateOutputInformationCallable, args, (PyObject *)NULL);
155+
PyObject * result = PyObject_Call(this->m_GenerateOutputInformationCallable, args, (PyObject *)NULL);
158156
SWIG_Py_DECREF(args);
159157

160158
if (result)
@@ -214,10 +212,8 @@ PyImageFilter<TInputImage, TOutputImage>::GenerateInputRequestedRegion()
214212
// make sure that the CommandCallable is in fact callable
215213
if (PyCallable_Check(this->m_GenerateInputRequestedRegionCallable))
216214
{
217-
PyObject * result;
218-
219215
PyObject * args = PyTuple_Pack(1, this->m_Self);
220-
result = PyObject_Call(this->m_GenerateInputRequestedRegionCallable, args, (PyObject *)NULL);
216+
PyObject * result = PyObject_Call(this->m_GenerateInputRequestedRegionCallable, args, (PyObject *)NULL);
221217
SWIG_Py_DECREF(args);
222218

223219
if (result)
@@ -249,10 +245,8 @@ PyImageFilter<TInputImage, TOutputImage>::GenerateData()
249245
}
250246
else
251247
{
252-
PyObject * result;
253-
254248
PyObject * args = PyTuple_Pack(1, this->m_Self);
255-
result = PyObject_Call(this->m_GenerateDataCallable, args, (PyObject *)NULL);
249+
PyObject * result = PyObject_Call(this->m_GenerateDataCallable, args, (PyObject *)NULL);
256250
SWIG_Py_DECREF(args);
257251

258252
if (result)

Modules/Core/Common/src/itkObjectFactoryBase.cxx

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -316,7 +316,7 @@ ObjectFactoryBase::LoadDynamicFactories()
316316
static std::string
317317
CreateFullPath(const char * path, const char * file)
318318
{
319-
std::string ret;
319+
std::string ret = path;
320320

321321
# ifdef _WIN32
322322
const char sep = '\\';
@@ -326,7 +326,6 @@ CreateFullPath(const char * path, const char * file)
326326
/**
327327
* make sure the end of path is a separator
328328
*/
329-
ret = path;
330329
if (!ret.empty() && ret.back() != sep)
331330
{
332331
ret += sep;

Modules/Core/Common/test/itkAnnulusOperatorGTest.cxx

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -115,8 +115,7 @@ TEST(AnnulusOperator, CreateAndInspect)
115115

116116
annulus.CreateOperator();
117117

118-
OperatorType::SizeType annulusSize;
119-
annulusSize = annulus.GetSize();
118+
OperatorType::SizeType annulusSize = annulus.GetSize();
120119
std::cout << ", N = " << annulusSize << ", r = " << annulus.GetInnerRadius() << ", t = " << annulus.GetThickness()
121120
<< ", i = " << annulus.GetInteriorValue() << ", a = " << annulus.GetAnnulusValue()
122121
<< ", e = " << annulus.GetExteriorValue() << std::endl;

Modules/Core/Common/test/itkBoundaryConditionGTest.cxx

Lines changed: 3 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -102,14 +102,12 @@ TEST(BoundaryCondition, ConstantBoundaryAtImageEdge)
102102
cbc.SetConstant(0.0f);
103103
it2d.OverrideBoundaryCondition(&cbc);
104104

105-
SmartIteratorType::NeighborhoodType tempN;
106-
SmartIteratorType::NeighborhoodType temp2N;
107-
temp2N = it2d.GetNeighborhood(); // initialize
105+
SmartIteratorType::NeighborhoodType temp2N = it2d.GetNeighborhood();
108106

109107
it2d.GoToEnd();
110108
--it2d;
111-
tempN = it2d.GetNeighborhood();
112109

110+
SmartIteratorType::NeighborhoodType tempN = it2d.GetNeighborhood();
113111
printn(tempN.GetBufferReference(), tempN.GetSize());
114112

115113
// The 2D image is 30x15, filled with 100*j + i.
@@ -169,8 +167,7 @@ TEST(BoundaryCondition, ZeroFluxNeumannBoundaryTraversal)
169167
cbc.SetConstant(0.0f);
170168
it2d.OverrideBoundaryCondition(&cbc);
171169

172-
SmartIteratorType::NeighborhoodType temp2N;
173-
temp2N = it2d.GetNeighborhood(); // initialize
170+
SmartIteratorType::NeighborhoodType temp2N = it2d.GetNeighborhood();
174171

175172
itk::ZeroFluxNeumannBoundaryCondition<ImageType2D> neumann;
176173
for (int yak = 0; yak < 2; ++yak)

Modules/Core/Common/test/itkConstNeighborhoodIteratorWithOnlyIndexTest.cxx

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -208,10 +208,9 @@ itkConstNeighborhoodIteratorWithOnlyIndexTestRun()
208208

209209
std::cout << "Creating ConstNeighborhoodIterator" << std::endl;
210210
ConstNeighborhoodIteratorType ra_it(radius, ra_img, ra_img->GetRequestedRegion());
211-
ConstNeighborhoodIteratorType copy_it;
211+
ConstNeighborhoodIteratorType copy_it = ra_it;
212212

213213
std::cout << "Test copying." << std::endl;
214-
copy_it = ra_it;
215214
if (copy_it != ra_it || !(copy_it == ra_it))
216215
{
217216
std::cerr << "Failure with copying or equality comparison." << std::endl;

Modules/Core/Common/test/itkExceptionObjectTest.cxx

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -159,8 +159,7 @@ itkExceptionObjectTest(int, char *[])
159159
Se.SetLocation("SE LOCATION");
160160
Se.SetDescription("SE DESCRIPTION");
161161
itk::SampleError Sf(Se);
162-
itk::SampleError Sg;
163-
Sg = Sf;
162+
itk::SampleError Sg = Sf;
164163
std::cout << Sg << std::endl;
165164
166165

Modules/Core/Common/test/itkExtractImageTest.cxx

Lines changed: 7 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -133,8 +133,8 @@ itkExtractImageTest(int, char *[])
133133
std::cout << "Input Image: " << if2 << std::endl;
134134

135135
// Create a filter
136-
itk::ExtractImageFilter<ShortImage, ShortImage>::Pointer extract;
137-
extract = itk::ExtractImageFilter<ShortImage, ShortImage>::New();
136+
const itk::ExtractImageFilter<ShortImage, ShortImage>::Pointer extract =
137+
itk::ExtractImageFilter<ShortImage, ShortImage>::New();
138138
extract->SetInput(if2);
139139

140140
// fill in an image
@@ -218,8 +218,8 @@ itkExtractImageTest(int, char *[])
218218
extract->SetExtractionRegion(extractRegion);
219219

220220
// Create a stream
221-
itk::StreamingImageFilter<ShortImage, ShortImage>::Pointer stream;
222-
stream = itk::StreamingImageFilter<ShortImage, ShortImage>::New();
221+
const itk::StreamingImageFilter<ShortImage, ShortImage>::Pointer stream =
222+
itk::StreamingImageFilter<ShortImage, ShortImage>::New();
223223
stream->SetInput(extract->GetOutput());
224224
stream->SetNumberOfStreamDivisions(2);
225225

@@ -289,8 +289,8 @@ itkExtractImageTest(int, char *[])
289289
}
290290

291291
// Case 3: Try extracting a single row
292-
itk::ExtractImageFilter<ShortImage, LineImage>::Pointer lineExtract;
293-
lineExtract = itk::ExtractImageFilter<ShortImage, LineImage>::New();
292+
const itk::ExtractImageFilter<ShortImage, LineImage>::Pointer lineExtract =
293+
itk::ExtractImageFilter<ShortImage, LineImage>::New();
294294
lineExtract->SetDirectionCollapseToGuess();
295295
lineExtract->SetInput(if2);
296296

@@ -307,9 +307,8 @@ itkExtractImageTest(int, char *[])
307307
std::cout << "After 1D extraction. " << std::endl;
308308

309309
// test the dimension collapse
310-
LineImage::RegionType requestedLineRegion;
310+
LineImage::RegionType requestedLineRegion = lineExtract->GetOutput()->GetRequestedRegion();
311311

312-
requestedLineRegion = lineExtract->GetOutput()->GetRequestedRegion();
313312

314313
itk::ImageRegionIterator<LineImage> iteratorLineIn(lineExtract->GetOutput(), requestedLineRegion);
315314

0 commit comments

Comments
 (0)