Skip to content

Commit

Permalink
ENH: Let x::New() initialize the created object by doing new x()
Browse files Browse the repository at this point in the history
Replace `new x` with `new x()`, in the definitions of `itkSimpleNewMacro` and
`itkFactorylessNewMacro`. This ensures that objects created by `x::New()` are
"value-initialized". In practice, this will zero-initialize data members that
might otherwise be left uninitialized.

Added GTests to check that `x::New()` does indeed properly initialize the
created object.
  • Loading branch information
N-Dekker authored and hjmjohnson committed Mar 1, 2024
1 parent 69626af commit 1986b54
Show file tree
Hide file tree
Showing 3 changed files with 85 additions and 2 deletions.
4 changes: 2 additions & 2 deletions Modules/Core/Common/include/itkMacro.h
Expand Up @@ -312,7 +312,7 @@ namespace itk
Pointer smartPtr = ::itk::ObjectFactory<x>::Create(); \
if (smartPtr == nullptr) \
{ \
smartPtr = new x; \
smartPtr = new x(); \
} \
smartPtr->UnRegister(); \
return smartPtr; \
Expand Down Expand Up @@ -369,7 +369,7 @@ namespace itk
#define itkFactorylessNewMacro(x) \
static Pointer New() \
{ \
x * rawPtr = new x; \
x * rawPtr = new x(); \
Pointer smartPtr = rawPtr; \
rawPtr->UnRegister(); \
return smartPtr; \
Expand Down
1 change: 1 addition & 0 deletions Modules/Core/Common/test/CMakeLists.txt
Expand Up @@ -1734,6 +1734,7 @@ set(ITKCommonGTests
itkImageRegionGTest.cxx
itkIndexGTest.cxx
itkIndexRangeGTest.cxx
itkLightObjectGTest.cxx
itkMakeUniqueForOverwriteGTest.cxx
itkMatrixGTest.cxx
itkMersenneTwisterRandomVariateGeneratorGTest.cxx
Expand Down
82 changes: 82 additions & 0 deletions Modules/Core/Common/test/itkLightObjectGTest.cxx
@@ -0,0 +1,82 @@
/*=========================================================================
*
* Copyright NumFOCUS
*
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
* You may obtain a copy of the License at
*
* https://www.apache.org/licenses/LICENSE-2.0.txt
*
* Unless required by applicable law or agreed to in writing, software
* distributed under the License is distributed on an "AS IS" BASIS,
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
* See the License for the specific language governing permissions and
* limitations under the License.
*
*=========================================================================*/

// First include the header file to be tested:
#include "itkLightObject.h"
#include "itkObjectFactory.h"
#include <gtest/gtest.h>


// Tests that New() does initialize the data of a derived class, even when the derived class has a defaulted
// default-constructor *and* its data has no {} initializer. Uses itkSimpleNewMacro to define New().
TEST(LightObject, SimpleNewInitializesDataOfDerivedClass)
{
class DerivedClass : public itk::LightObject
{
public:
ITK_DISALLOW_COPY_AND_MOVE(DerivedClass);

using Self = DerivedClass;
using Pointer = itk::SmartPointer<Self>;

itkSimpleNewMacro(DerivedClass);
itkGetConstMacro(Data, int);

protected:
// Defaulted default-constructor, essential to this test.
DerivedClass() = default;

// Defaulted destructor.
~DerivedClass() override = default;

private:
int m_Data; // Without {} initializer, for the purpose of this test.
};

EXPECT_EQ(DerivedClass::New()->GetData(), 0);
}


// Tests that New() does initialize the data of a derived class, even when the derived class has a defaulted
// default-constructor *and* its data has no {} initializer. Uses itkFactorylessNewMacro to define New().
TEST(LightObject, FactorylessNewInitializesDataOfDerivedClass)
{
class DerivedClass : public itk::LightObject
{
public:
ITK_DISALLOW_COPY_AND_MOVE(DerivedClass);

using Self = DerivedClass;
using Pointer = itk::SmartPointer<Self>;

itkFactorylessNewMacro(DerivedClass);
itkGetConstMacro(Data, int);

protected:
// Defaulted default-constructor, essential to this test.
DerivedClass() = default;

// Defaulted destructor.
~DerivedClass() override = default;

private:
int m_Data; // Without {} initializer, for the purpose of this test.
};

EXPECT_EQ(DerivedClass::New()->GetData(), 0);
}

0 comments on commit 1986b54

Please sign in to comment.