Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Default default-constructor of ConstantBoundaryCondition and other Core/Common class templates #3929

Conversation

N-Dekker
Copy link
Contributor

Follow-up to pull request #3878 "STYLE: Default default-constructors of class templates in Core/Common"

@github-actions github-actions bot added the area:Core Issues affecting the Core module label Feb 16, 2023
@N-Dekker N-Dekker force-pushed the Default-default-constructors-Core-Common branch from b70c5a2 to 32a7b06 Compare February 16, 2023 17:09
@N-Dekker N-Dekker force-pushed the Default-default-constructors-Core-Common branch 4 times, most recently from 17f33b4 to d1a5d36 Compare February 17, 2023 00:22
This warning appeared on ITK.macOS Azure.Mac-1676593025693/Darwin-Build9696,
Apple clang version 13.0.0 (clang-1300.0.29.30), just after _defaulting_ the
default-constructor of `itk::CompensatedSummation`.
Defaulted the default-constructors of five SpatialFunction class templates:

 - BinaryThresholdSpatialFunction
 - EllipsoidInteriorExteriorSpatialFunction
 - GaussianDerivativeSpatialFunction
 - GaussianSpatialFunction
 - SphereSpatialFunction
Default the default-constructor of four Image class templates:

 - Image
 - SparseImage
 - SpecialCoordinatesImage
 - VectorImage
Defaulted the default-constructor of ImportImageContainer and ImportImageFilter
Also defaulted the default-constructor of MinPriorityQueueElementWrapper, and
added in-class initializers to its data members.
@N-Dekker N-Dekker force-pushed the Default-default-constructors-Core-Common branch from d1a5d36 to d5fa8ae Compare February 17, 2023 10:41
@github-actions github-actions bot added the type:Testing Ensure that the purpose of a class is met/the results on a wide set of test cases are correct label Feb 17, 2023
@N-Dekker N-Dekker marked this pull request as ready for review February 17, 2023 12:25
Copy link
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.

I love this simplification.

@dzenanz dzenanz merged commit 1b5da45 into InsightSoftwareConsortium:master Feb 23, 2023
N-Dekker added a commit to N-Dekker/ITK that referenced this pull request Sep 1, 2023
The constructors of `ImageRandomConstIteratorWithIndex` and
`ImageRandomConstIteratorWithOnlyIndex` that support two arguments (image and
region) accidentally still assigned their data members, while they were already
initialized by in-class default member initialization. This in-class default
member initialization was added as part of pull request
InsightSoftwareConsortium#3929 commit
4e46cb6 "STYLE: Default default-constructor of
ImageRandom ConstIterator classes", merged on February 24, 2023 and included
with tag ITK v5.4rc01.

This caused extra `MersenneTwisterRandomVariateGenerator::New()` calls, which
changed the seeds of random number generation. These changes can possibly cause
regression failures in unit tests of client applications, including elastix.

This commit removes all data member assignments from the bodies of these
constructors.
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 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