Skip to content

STYLE: Add constexpr to variables initialized by MakeFilled #4930

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 1 commit into from
Nov 5, 2024
Merged

STYLE: Add constexpr to variables initialized by MakeFilled #4930

merged 1 commit into from
Nov 5, 2024

Conversation

@N-Dekker
Copy link
Contributor

@N-Dekker N-Dekker commented Nov 4, 2024

Suggests evaluating its value at compile-time, rather than at runtime.

Found by regular expressions:

^( [ ]+)(auto \w+[ ]+= MakeFilled<.+>\(-?\d+\);)
^( [ ]+)(auto \w+[ ]+= MakeFilled<.+>\(\d.0\);)
^( [ ]+)(auto \w+[ ]+= MakeFilled<.+>\(false\);)
^( [ ]+)(auto \w+[ ]+= MakeFilled<.+>\(true\);)

Inspired by a comment by Hans Johnson (@hjmjohnson) at
#4924 (comment)

@N-Dekker
STYLE: Add constexpr to variables initialized by MakeFilled
12251ef 
Suggests evaluating its value at compile-time, rather than at runtime.

Found by regular expressions:

    ^( [ ]+)(auto \w+[ ]+= MakeFilled<.+>\(-?\d+\);)
    ^( [ ]+)(auto \w+[ ]+= MakeFilled<.+>\(\d.0\);)
    ^( [ ]+)(auto \w+[ ]+= MakeFilled<.+>\(false\);)
    ^( [ ]+)(auto \w+[ ]+= MakeFilled<.+>\(true\);)

Inspired by a comment by Hans Johnson at
InsightSoftwareConsortium#4924 (comment)
@github-actions github-actions bot added area:Core Issues affecting the Core module area:Filtering Issues affecting the Filtering module area:Registration Issues affecting the Registration module area:Segmentation Issues affecting the Segmentation module type:Style Style changes: no logic impact (indentation, comments, naming) area:Numerics Issues affecting the Numerics module labels Nov 4, 2024
hjmjohnson
hjmjohnson approved these changes Nov 4, 2024
View reviewed changes
Copy link
Member

@hjmjohnson hjmjohnson left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Love It !!!

N-Dekker
N-Dekker commented Nov 4, 2024
View reviewed changes
Modules/Core/Common/include/itkShapedFloodFilledFunctionConditionalConstIterator.hxx

// Build and setup the neighborhood iterator
auto radius = MakeFilled<typename NeighborhoodIteratorType::RadiusType>(1);
constexpr auto radius = MakeFilled<typename NeighborhoodIteratorType::RadiusType>(1);
Copy link
Contributor Author

@N-Dekker N-Dekker Nov 4, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We could even declare those variables static constexpr, instead of just non-static constexpr. I don't know if it matters much, in these cases 🤷 Some people prefer to always declare local variables static constexpr if possible.

@N-Dekker
Copy link
Contributor Author

N-Dekker commented Nov 4, 2024

FYI This PR has excluded variables that could not be declared constexpr, either because they are modified later on, or because their MakeFilled call cannot be evaluated at compile-time.

@N-Dekker N-Dekker marked this pull request as ready for review November 4, 2024 20:47
@N-Dekker
Copy link
Contributor Author

N-Dekker commented Nov 4, 2024

/azp run ITK.Linux

@hjmjohnson hjmjohnson merged commit fd14482 into InsightSoftwareConsortium:master Nov 5, 2024
15 checks passed
N-Dekker added a commit to N-Dekker/ITK that referenced this pull request Nov 9, 2024
@N-Dekker
STYLE: Add constexpr to variables initialized by T::Filled(x)
a0f327c 
Found by the regular expression `^( [ ]+)(auto \w+[ ]+= .+::Filled\(\d+\);)`

Manually excluded cases that did not compile, in

    itkLabelMapContourOverlayImageFilter.hxx
    itkBSplineControlPointImageFilter.hxx
    itkBSplineScatteredDataPointSetToImageFilter.hxx
    itkTileImageFilter.hxx
    itkFFTDiscreteGaussianImageFilter.hxx
    itkSLICImageFilter.hxx

 - Follow-up to pull request InsightSoftwareConsortium#4930
commit fd14482
"STYLE: Add constexpr to variables initialized by MakeFilled"
N-Dekker added a commit to N-Dekker/ITK that referenced this pull request Nov 9, 2024
@N-Dekker
STYLE: Add constexpr to variables initialized by T::Filled(int-literal)
80bc26c 
Found by the regular expression `^( [ ]+)(auto \w+[ ]+= .+::Filled\(\d+\);)`

Manually excluded cases that did not compile, in

    itkLabelMapContourOverlayImageFilter.hxx
    itkBSplineControlPointImageFilter.hxx
    itkBSplineScatteredDataPointSetToImageFilter.hxx
    itkTileImageFilter.hxx
    itkFFTDiscreteGaussianImageFilter.hxx
    itkSLICImageFilter.hxx

 - Follow-up to pull request InsightSoftwareConsortium#4930
commit fd14482
"STYLE: Add constexpr to variables initialized by MakeFilled"
N-Dekker added a commit to N-Dekker/ITK that referenced this pull request Nov 9, 2024
@N-Dekker
STYLE: Add constexpr to variables initialized by T::Filled(int-literal)
c0b08ad 
Using Notepad++, Replace in Files, doing:

    Find what: ^( [ ]+)(auto \w+[ ]+= .+::Filled\(\d+\);)
    Replace with: $1constexpr $2
    Filters: itk*.* !+\test
    Directory: D:\src\ITK\Modules
    [v] Match case
    (*) Regular expression

Manually excluded cases that did not compile, in

    itkLabelMapContourOverlayImageFilter.hxx
    itkBSplineControlPointImageFilter.hxx
    itkBSplineScatteredDataPointSetToImageFilter.hxx
    itkTileImageFilter.hxx
    itkFFTDiscreteGaussianImageFilter.hxx
    itkSLICImageFilter.hxx

 - Follow-up to pull request InsightSoftwareConsortium#4930
commit fd14482
"STYLE: Add constexpr to variables initialized by MakeFilled"
@N-Dekker N-Dekker mentioned this pull request Nov 9, 2024
Merged
hjmjohnson pushed a commit that referenced this pull request Nov 11, 2024
@N-Dekker @hjmjohnson
STYLE: Add constexpr to variables initialized by T::Filled(int-literal)
c77f9f8 
Using Notepad++, Replace in Files, doing:

    Find what: ^( [ ]+)(auto \w+[ ]+= .+::Filled\(\d+\);)
    Replace with: $1constexpr $2
    Filters: itk*.* !+\test
    Directory: D:\src\ITK\Modules
    [v] Match case
    (*) Regular expression

Manually excluded cases that did not compile, in

    itkLabelMapContourOverlayImageFilter.hxx
    itkBSplineControlPointImageFilter.hxx
    itkBSplineScatteredDataPointSetToImageFilter.hxx
    itkTileImageFilter.hxx
    itkFFTDiscreteGaussianImageFilter.hxx
    itkSLICImageFilter.hxx

 - Follow-up to pull request #4930
commit fd14482
"STYLE: Add constexpr to variables initialized by MakeFilled"
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@hjmjohnson hjmjohnson hjmjohnson approved these changes

@dzenanz dzenanz dzenanz approved these changes

Assignees

No one assigned

Labels

area:Core Issues affecting the Core module area:Filtering Issues affecting the Filtering module area:Numerics Issues affecting the Numerics module area:Registration Issues affecting the Registration module area:Segmentation Issues affecting the Segmentation module type:Style Style changes: no logic impact (indentation, comments, naming)

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

@N-Dekker @hjmjohnson @dzenanz