Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
30 changes: 30 additions & 0 deletions Documentation/AI/compiler-cautions.md
Original file line number Diff line number Diff line change
Expand Up @@ -357,6 +357,35 @@ class Foo {
};
```

### 6c. `-Wunused-but-set-variable` — GCC (declare-then-assign refactoring)

After converting `T x; x = func();` to `const T x = func();`, GCC warns if
`x` is never read downstream. Apple Clang and MSVC typically don't fire this.
This is the most common warning introduced by STYLE: declare-then-assign
refactoring commits.

```cpp
// Triggers warning — computed but never tested:
const MatrixType inverse = matrix.GetInverse();

// Fix for test code — add a meaningful assertion:
const MatrixType inverse = matrix.GetInverse();
if (itk::Math::NotExactlyEquals(inverse(0, 0), expected))
{
std::cerr << "Problem with GetInverse()" << std::endl;
return EXIT_FAILURE;
}

// Fix for production code — don't capture if unneeded:
matrix.GetInverse(); // called for side effect only
```

In test files, **always prefer adding a test assertion** over suppressing
with `[[maybe_unused]]` or `static_cast<void>()`. Tests that compute values
without checking them are a missed coverage opportunity.

**References:** PR #6044, CDash build 11198750.

---

## 7. MSVC-Specific Warnings
Expand Down Expand Up @@ -537,6 +566,7 @@ When refactoring existing code, verify each item:
- [ ] `std::vector` replacing `new[]` — guard `data()` / `operator[]` calls on potentially empty vectors
- [ ] Lambda captures — no unused captures; `constexpr` variables don't need capture
- [ ] Loop variables initialized at declaration
- [ ] 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
- [ ] `bool` not used with `|=` for exit-status accumulation — use `int`
- [ ] Third-party header includes wrapped with appropriate `ITK_CLANG_SUPPRESS_*` macros
- [ ] Explicit template instantiations in shared-build modules marked `ITK_TEMPLATE_EXPORT`
Expand Down
3 changes: 1 addition & 2 deletions Modules/Core/Common/include/itkConceptChecking.h
Original file line number Diff line number Diff line change
Expand Up @@ -775,9 +775,8 @@ struct HasZero
void
constraints()
{
T a;
T a = T{};

a = T{};
Detail::IgnoreUnusedVariable(a);
}
};
Expand Down
4 changes: 1 addition & 3 deletions Modules/Core/Common/include/itkImage.hxx
Original file line number Diff line number Diff line change
Expand Up @@ -38,10 +38,8 @@ template <typename TPixel, unsigned int VImageDimension>
void
Image<TPixel, VImageDimension>::Allocate(bool initializePixels)
{
SizeValueType num;

this->ComputeOffsetTable();
num = static_cast<SizeValueType>(this->GetOffsetTable()[VImageDimension]);
SizeValueType num = static_cast<SizeValueType>(this->GetOffsetTable()[VImageDimension]);

m_Buffer->Reserve(num, initializePixels);
}
Expand Down
12 changes: 3 additions & 9 deletions Modules/Core/Common/include/itkPyImageFilter.hxx
Original file line number Diff line number Diff line change
Expand Up @@ -151,10 +151,8 @@ PyImageFilter<TInputImage, TOutputImage>::GenerateOutputInformation()
// make sure that the CommandCallable is in fact callable
if (PyCallable_Check(this->m_GenerateOutputInformationCallable))
{
PyObject * result;

PyObject * args = PyTuple_Pack(1, this->m_Self);
result = PyObject_Call(this->m_GenerateOutputInformationCallable, args, (PyObject *)NULL);
PyObject * result = PyObject_Call(this->m_GenerateOutputInformationCallable, args, (PyObject *)NULL);
SWIG_Py_DECREF(args);

if (result)
Expand Down Expand Up @@ -214,10 +212,8 @@ PyImageFilter<TInputImage, TOutputImage>::GenerateInputRequestedRegion()
// make sure that the CommandCallable is in fact callable
if (PyCallable_Check(this->m_GenerateInputRequestedRegionCallable))
{
PyObject * result;

PyObject * args = PyTuple_Pack(1, this->m_Self);
result = PyObject_Call(this->m_GenerateInputRequestedRegionCallable, args, (PyObject *)NULL);
PyObject * result = PyObject_Call(this->m_GenerateInputRequestedRegionCallable, args, (PyObject *)NULL);
SWIG_Py_DECREF(args);

if (result)
Expand Down Expand Up @@ -249,10 +245,8 @@ PyImageFilter<TInputImage, TOutputImage>::GenerateData()
}
else
{
PyObject * result;

PyObject * args = PyTuple_Pack(1, this->m_Self);
result = PyObject_Call(this->m_GenerateDataCallable, args, (PyObject *)NULL);
PyObject * result = PyObject_Call(this->m_GenerateDataCallable, args, (PyObject *)NULL);
SWIG_Py_DECREF(args);

if (result)
Expand Down
3 changes: 1 addition & 2 deletions Modules/Core/Common/src/itkObjectFactoryBase.cxx
Original file line number Diff line number Diff line change
Expand Up @@ -316,7 +316,7 @@ ObjectFactoryBase::LoadDynamicFactories()
static std::string
CreateFullPath(const char * path, const char * file)
{
std::string ret;
std::string ret = path;

# ifdef _WIN32
const char sep = '\\';
Expand All @@ -326,7 +326,6 @@ CreateFullPath(const char * path, const char * file)
/**
* make sure the end of path is a separator
*/
ret = path;
if (!ret.empty() && ret.back() != sep)
{
ret += sep;
Expand Down
3 changes: 1 addition & 2 deletions Modules/Core/Common/test/itkAnnulusOperatorGTest.cxx
Original file line number Diff line number Diff line change
Expand Up @@ -115,8 +115,7 @@ TEST(AnnulusOperator, CreateAndInspect)

annulus.CreateOperator();

OperatorType::SizeType annulusSize;
annulusSize = annulus.GetSize();
OperatorType::SizeType annulusSize = annulus.GetSize();
std::cout << ", N = " << annulusSize << ", r = " << annulus.GetInnerRadius() << ", t = " << annulus.GetThickness()
<< ", i = " << annulus.GetInteriorValue() << ", a = " << annulus.GetAnnulusValue()
<< ", e = " << annulus.GetExteriorValue() << std::endl;
Expand Down
9 changes: 3 additions & 6 deletions Modules/Core/Common/test/itkBoundaryConditionGTest.cxx
Original file line number Diff line number Diff line change
Expand Up @@ -102,14 +102,12 @@ TEST(BoundaryCondition, ConstantBoundaryAtImageEdge)
cbc.SetConstant(0.0f);
it2d.OverrideBoundaryCondition(&cbc);

SmartIteratorType::NeighborhoodType tempN;
SmartIteratorType::NeighborhoodType temp2N;
temp2N = it2d.GetNeighborhood(); // initialize
SmartIteratorType::NeighborhoodType temp2N = it2d.GetNeighborhood();

it2d.GoToEnd();
--it2d;
tempN = it2d.GetNeighborhood();

SmartIteratorType::NeighborhoodType tempN = it2d.GetNeighborhood();
printn(tempN.GetBufferReference(), tempN.GetSize());

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

SmartIteratorType::NeighborhoodType temp2N;
temp2N = it2d.GetNeighborhood(); // initialize
SmartIteratorType::NeighborhoodType temp2N = it2d.GetNeighborhood();

itk::ZeroFluxNeumannBoundaryCondition<ImageType2D> neumann;
for (int yak = 0; yak < 2; ++yak)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -208,10 +208,9 @@ itkConstNeighborhoodIteratorWithOnlyIndexTestRun()

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

std::cout << "Test copying." << std::endl;
copy_it = ra_it;
if (copy_it != ra_it || !(copy_it == ra_it))
{
std::cerr << "Failure with copying or equality comparison." << std::endl;
Expand Down
3 changes: 1 addition & 2 deletions Modules/Core/Common/test/itkExceptionObjectTest.cxx
Original file line number Diff line number Diff line change
Expand Up @@ -159,8 +159,7 @@ itkExceptionObjectTest(int, char *[])
Se.SetLocation("SE LOCATION");
Se.SetDescription("SE DESCRIPTION");
itk::SampleError Sf(Se);
itk::SampleError Sg;
Sg = Sf;
itk::SampleError Sg = Sf;
std::cout << Sg << std::endl;


Expand Down
15 changes: 7 additions & 8 deletions Modules/Core/Common/test/itkExtractImageTest.cxx
Original file line number Diff line number Diff line change
Expand Up @@ -133,8 +133,8 @@ itkExtractImageTest(int, char *[])
std::cout << "Input Image: " << if2 << std::endl;

// Create a filter
itk::ExtractImageFilter<ShortImage, ShortImage>::Pointer extract;
extract = itk::ExtractImageFilter<ShortImage, ShortImage>::New();
const itk::ExtractImageFilter<ShortImage, ShortImage>::Pointer extract =
itk::ExtractImageFilter<ShortImage, ShortImage>::New();
extract->SetInput(if2);

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

// Create a stream
itk::StreamingImageFilter<ShortImage, ShortImage>::Pointer stream;
stream = itk::StreamingImageFilter<ShortImage, ShortImage>::New();
const itk::StreamingImageFilter<ShortImage, ShortImage>::Pointer stream =
itk::StreamingImageFilter<ShortImage, ShortImage>::New();
stream->SetInput(extract->GetOutput());
stream->SetNumberOfStreamDivisions(2);

Expand Down Expand Up @@ -289,8 +289,8 @@ itkExtractImageTest(int, char *[])
}

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

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

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

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

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

Expand Down
3 changes: 2 additions & 1 deletion Modules/Core/Common/test/itkImageIteratorTest.cxx
Original file line number Diff line number Diff line change
Expand Up @@ -96,7 +96,8 @@ itkImageIteratorTest(int, char *[])
// Exercise copy constructor
const VectorImageIterator itr3(itr1);

// Exercise assignment operator
// Exercise assignment operator — intentionally two-line; do NOT merge
// into `VectorImageIterator itr4 = itr1` which invokes copy constructor.
VectorImageIterator itr4;
itr4 = itr1;

Expand Down
6 changes: 2 additions & 4 deletions Modules/Core/Common/test/itkImageLinearIteratorTest.cxx
Original file line number Diff line number Diff line change
Expand Up @@ -231,8 +231,7 @@ itkImageLinearIteratorTest(int, char *[])
bot.SetDirection(0); // 0=x, 1=y, 2=z
bot.GoToBegin();

ImageType::IndexType testIndex;
testIndex = start;
ImageType::IndexType testIndex = start;
testIndex[1] += 2; // advance two lines in Y

bot.NextLine(); // advance two lines in Y
Expand Down Expand Up @@ -274,8 +273,7 @@ itkImageLinearIteratorTest(int, char *[])
cbot.SetDirection(0); // 0=x, 1=y, 2=z
cbot.GoToBegin();

ImageType::IndexType testIndex;
testIndex = start;
ImageType::IndexType testIndex = start;
testIndex[1] += 2; // advance two lines in Y

cbot.NextLine(); // advance two lines in Y
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -122,9 +122,8 @@ class itkImageRegionConstIteratorWithOnlyIndexTestIteratorTester
}

// Test iterating fwd by line
IndexType index;
it.GoToBegin();
index = it.GetIndex();
IndexType index = it.GetIndex();
for (unsigned int i = 0; i < region.GetSize()[0]; ++i)
{
++it;
Expand Down
41 changes: 29 additions & 12 deletions Modules/Core/Common/test/itkMatrixTest.cxx
Original file line number Diff line number Diff line change
Expand Up @@ -76,14 +76,34 @@ itkMatrixTest(int, char *[])
matrix2.SetIdentity();
matrix2.GetVnlMatrix()(0, 0) = 10;

MatrixType matrixProduct;
matrixProduct = matrix * matrix2;
MatrixType matrixProduct = matrix * matrix2;

MatrixType matrix3;
matrix3 = matrix.GetInverse();
// identity * matrix2 should equal matrix2
if (itk::Math::NotExactlyEquals(matrixProduct(0, 0), 10) || itk::Math::NotExactlyEquals(matrixProduct(1, 1), 1) ||
Comment thread
dzenanz marked this conversation as resolved.
itk::Math::NotExactlyEquals(matrixProduct(2, 2), 1) || itk::Math::NotExactlyEquals(matrixProduct(0, 1), 0) ||
itk::Math::NotExactlyEquals(matrixProduct(1, 0), 0))
{
std::cerr << "Problem with matrix product" << std::endl;
return EXIT_FAILURE;
}

const MatrixType matrix3 = matrix.GetInverse();
// Inverse of identity should be identity
if (itk::Math::NotExactlyEquals(matrix3(0, 0), 1) || itk::Math::NotExactlyEquals(matrix3(1, 1), 1) ||
itk::Math::NotExactlyEquals(matrix3(2, 2), 1) || itk::Math::NotExactlyEquals(matrix3(0, 1), 0))
{
std::cerr << "Problem with GetInverse()" << std::endl;
return EXIT_FAILURE;
}

MatrixType matrix4;
matrix4 = matrix.GetTranspose();
const MatrixType matrix4 = matrix.GetTranspose();
// Transpose of identity should be identity
if (itk::Math::NotExactlyEquals(matrix4(0, 0), 1) || itk::Math::NotExactlyEquals(matrix4(1, 1), 1) ||
itk::Math::NotExactlyEquals(matrix4(2, 2), 1) || itk::Math::NotExactlyEquals(matrix4(0, 1), 0))
{
std::cerr << "Problem with GetTranspose()" << std::endl;
return EXIT_FAILURE;
}

auto matrix5 = itk::MakeFilled<MatrixType>(1.7);

Expand Down Expand Up @@ -217,11 +237,9 @@ itkMatrixTest(int, char *[])
}
}

AddSubtractMatrixType m3;
m3 = m1 + m2;
AddSubtractMatrixType m3 = m1 + m2;

AddSubtractMatrixType m4;
m4 = m1 - m2;
AddSubtractMatrixType m4 = m1 - m2;

std::cout << "Results of ITK matrix addition" << std::endl;
std::cout << "M1 = " << std::endl << m1 << std::endl;
Expand Down Expand Up @@ -311,8 +329,7 @@ itkMatrixTest(int, char *[])
}
}

MatrixType matrixC;
matrixC = vnlMatrixA; // Test assignment
MatrixType matrixC = vnlMatrixA;

{ // verify values
constexpr double tolerance{ 1e-7 };
Expand Down
3 changes: 1 addition & 2 deletions Modules/Core/Common/test/itkNeighborhoodAlgorithmTest.cxx
Original file line number Diff line number Diff line change
Expand Up @@ -29,9 +29,8 @@ ImageBoundaryFaceCalculatorTest(TImage * image,
using FaceCalculatorType = itk::NeighborhoodAlgorithm::ImageBoundaryFacesCalculator<TImage>;
using FaceListType = typename FaceCalculatorType::FaceListType;
FaceCalculatorType faceCalculator;
FaceListType faceList;
FaceListType faceList = faceCalculator(image, region, radius);

faceList = faceCalculator(image, region, radius);
for (auto fit = faceList.begin(); fit != faceList.end(); ++fit)
{
std::cout << "Number of pixels : " << fit->GetNumberOfPixels() << std::endl;
Expand Down
4 changes: 4 additions & 0 deletions Modules/Core/Common/test/itkRangeGTestUtilities.h
Original file line number Diff line number Diff line change
Expand Up @@ -82,6 +82,8 @@ class RangeGTestUtilities
static void
ExpectCopyAssignedRangeHasSameIteratorsAsOriginal(const TRange & originalRange)
{
// Intentionally two-line; do NOT merge into `TRange copyAssignedRange = originalRange`
// which invokes copy constructor instead of operator=.
TRange copyAssignedRange;
copyAssignedRange = originalRange;

Expand All @@ -106,6 +108,8 @@ class RangeGTestUtilities
{
const TRange originalRangeBeforeMove = originalRange;

// Intentionally two-line; do NOT merge into `TRange moveAssignedRange = std::move(...)`
// which invokes move constructor instead of operator=.
TRange moveAssignedRange;
moveAssignedRange = std::move(originalRange);

Expand Down
3 changes: 1 addition & 2 deletions Modules/Core/Common/test/itkSliceIteratorTest.cxx
Original file line number Diff line number Diff line change
Expand Up @@ -25,13 +25,12 @@ template <typename TPixelType, unsigned int VDimension>
void
FillRegionSequential(itk::SmartPointer<itk::Image<TPixelType, VDimension>> I)
{
itk::Size<VDimension> Index;
itk::Size<VDimension> Index = (I->GetRequestedRegion()).GetSize();
unsigned long Location[VDimension];


itk::ImageRegionIterator<itk::Image<TPixelType, VDimension>> data(I, I->GetRequestedRegion());

Index = (I->GetRequestedRegion()).GetSize();

unsigned int ArrayLength = 1;
for (unsigned int iDim = 0; iDim < VDimension; ++iDim)
Expand Down
Loading
Loading