Skip to content

STYLE: Avoid repeating parent aliases in operator classes#2567

Merged
dzenanz merged 3 commits intoInsightSoftwareConsortium:masterfrom
jhlegarreta:AvoidRepeatingParentAliasesInCommonOperators
Sep 21, 2021
Merged

STYLE: Avoid repeating parent aliases in operator classes#2567
dzenanz merged 3 commits intoInsightSoftwareConsortium:masterfrom
jhlegarreta:AvoidRepeatingParentAliasesInCommonOperators

Conversation

@jhlegarreta
Copy link
Copy Markdown
Member

Avoid repeating parent aliases in Common operator classes.

PR Checklist

@github-actions github-actions Bot added area:Core Issues affecting the Core module type:Style Style changes: no logic impact (indentation, comments, naming) labels Jun 2, 2021
@jhlegarreta jhlegarreta force-pushed the AvoidRepeatingParentAliasesInCommonOperators branch 3 times, most recently from 08a6e45 to 830c693 Compare June 2, 2021 03:42
Comment thread Modules/Core/Common/include/itkNeighborhoodOperator.h Outdated
Comment thread Modules/Core/Common/include/itkNeighborhoodOperator.h Outdated
Comment thread Modules/Core/Common/include/itkGaussianDerivativeOperator.h
@jhlegarreta jhlegarreta force-pushed the AvoidRepeatingParentAliasesInCommonOperators branch 4 times, most recently from cfcf52b to cfa8825 Compare June 2, 2021 23:14
Comment thread Modules/Core/Common/include/itkDerivativeOperator.h Outdated
@jhlegarreta jhlegarreta force-pushed the AvoidRepeatingParentAliasesInCommonOperators branch 3 times, most recently from 45217ee to abe65f2 Compare June 2, 2021 23:45
Copy link
Copy Markdown
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.

This is a good start. Please cherry-pick the last two commits from https://github.com/hjmjohnson/ITK/tree/AvoidRepeatingParentAliasesInCommonOperators. does a more complete refactoring to keep the style and recommendations consistent.

Avoid repeating parent aliases in `Common` operator classes whenever
possible.

Use the preferred `using typename Superclass::{TypeAlias}` aliasing
form for the relevant type.

Make the `CoefficientVector` type be public in `itk::NeighborhoodOperator`
so that child classes can have access to it.
@jhlegarreta jhlegarreta force-pushed the AvoidRepeatingParentAliasesInCommonOperators branch from abe65f2 to 201537c Compare September 11, 2021 23:45
@github-actions github-actions Bot added area:Examples Demonstration of the use of classes area:Filtering Issues affecting the Filtering module area:IO Issues affecting the IO module area:Numerics Issues affecting the Numerics module labels Sep 11, 2021
@jhlegarreta jhlegarreta force-pushed the AvoidRepeatingParentAliasesInCommonOperators branch from 201537c to 0069817 Compare September 18, 2021 15:58
@dzenanz dzenanz merged commit e64795c into InsightSoftwareConsortium:master Sep 21, 2021
@jhlegarreta jhlegarreta deleted the AvoidRepeatingParentAliasesInCommonOperators branch September 21, 2021 13:37
N-Dekker added a commit to SuperElastix/elastix that referenced this pull request Sep 21, 2021
Following ITK pull request InsightSoftwareConsortium/ITK#2567 "STYLE: Avoid repeating parent aliases..." commit
InsightSoftwareConsortium/ITK@babece9 and InsightSoftwareConsortium/ITK@4f30980

Using Visual Studio 2019 Find and Replace regular expressions:

    typedef typename Superclass::(\w+)[ ]+\1;
    using typename Superclass::$1;
N-Dekker added a commit to SuperElastix/elastix that referenced this pull request Sep 21, 2021
Following ITK pull request InsightSoftwareConsortium/ITK#2567 "STYLE: Avoid repeating parent aliases..." commit
InsightSoftwareConsortium/ITK@babece9 and InsightSoftwareConsortium/ITK@4f30980

Using Visual Studio 2019 Find and Replace regular expressions:

    typedef typename Superclass::(\w+)[ ]+\1;
    using typename Superclass::$1;
N-Dekker added a commit to SuperElastix/elastix that referenced this pull request Sep 21, 2021
Following ITK pull request InsightSoftwareConsortium/ITK#2567 "STYLE: Avoid repeating parent aliases..." commit
InsightSoftwareConsortium/ITK@babece9 and InsightSoftwareConsortium/ITK@4f30980

Using Visual Studio 2019 Find and Replace regular expressions, like:

    typedef typename Superclass::(\w+)[ ]+\1;
    using typename Superclass::$1;
N-Dekker added a commit to SuperElastix/elastix that referenced this pull request Sep 21, 2021
Following ITK pull request InsightSoftwareConsortium/ITK#2567 "STYLE: Avoid repeating parent aliases..." commit
InsightSoftwareConsortium/ITK@babece9 and InsightSoftwareConsortium/ITK@4f30980

Using Visual Studio 2019 Find and Replace regular expressions, like:

    typedef Superclass::(\w+)[ ]+\1;
    using Superclass::$1;
N-Dekker added a commit to SuperElastix/elastix that referenced this pull request Sep 21, 2021
Following ITK pull request InsightSoftwareConsortium/ITK#2567 "STYLE: Avoid repeating parent aliases..." commit
InsightSoftwareConsortium/ITK@babece9 and InsightSoftwareConsortium/ITK@4f30980

Using Visual Studio 2019 Find and Replace regular expressions, like:

    typedef typename Superclass::(\w+)[ ]+\1;
    using typename Superclass::$1;
N-Dekker added a commit to SuperElastix/elastix that referenced this pull request Sep 22, 2021
Following ITK pull request InsightSoftwareConsortium/ITK#2567 "STYLE: Avoid repeating parent aliases..." commit
InsightSoftwareConsortium/ITK@babece9 and InsightSoftwareConsortium/ITK@4f30980

Using Visual Studio 2019 Find and Replace regular expressions, like:

    typedef typename Superclass::(\w+)[ ]+\1;
    using typename Superclass::$1;
N-Dekker added a commit to SuperElastix/elastix that referenced this pull request Sep 22, 2021
Following ITK pull request InsightSoftwareConsortium/ITK#2567 "STYLE: Avoid repeating parent aliases..." commit
InsightSoftwareConsortium/ITK@babece9 and InsightSoftwareConsortium/ITK@4f30980

Using Visual Studio 2019 Find and Replace regular expressions, like:

    typedef typename Superclass::(\w+)[ ]+\1;
    using typename Superclass::$1;
@jhlegarreta
Copy link
Copy Markdown
Member Author

jhlegarreta commented Sep 22, 2021

The CDash builds are not happy with this change 😟.

As for the failure in itkMultiphaseDenseFiniteDifferenceImageFilterTest.cxx, I think the superclass alias is missing the last template parameter ( TIdCell) here. Which seems to be a bug that surfaced thanks to the PR at least 🛠️.

I will submit a patch set for it later today.

The rest of the failures to the reported 50 require more investigation.

@dzenanz
Copy link
Copy Markdown
Member

dzenanz commented Sep 22, 2021

Oh, I just now see this. That's what happens when there are 30 new emails in the morning 😄

@jhlegarreta
Copy link
Copy Markdown
Member Author

@hjmjohnson I had a quick look yesterday to the rest of the compilation errors, but was not able to come up with a solution. Can you please have a look? Thanks.

@dzenanz
Copy link
Copy Markdown
Member

dzenanz commented Sep 23, 2021

@seanm just submitted #2753.

@jhlegarreta
Copy link
Copy Markdown
Member Author

jhlegarreta commented Sep 23, 2021

@seanm just submitted #2753.

The fix for one of the files has already been merged in #2751. I missed the other file. Sorry and thanks @seanm.

Not sure why the conflict with the master branch has not triggered.

@dzenanz Unfortunately, the rest of the failures to the listed 50 dwell in some other files.

@seanm
Copy link
Copy Markdown
Contributor

seanm commented Sep 23, 2021

I've just rebased #2753.

Does the github-based CI have Module_ITKReview enabled anywhere? I'm guessing not... it probably should...

@jhlegarreta
Copy link
Copy Markdown
Member Author

I've just rebased #2753.

👍

Does the github-based CI have Module_ITKReview enabled anywhere? I'm guessing not... it probably should...

Yep, I agree. I think it has been brought a few times before, e.g. #2653 (comment). But maybe there are reasons that are unknown to me that make enabling the Review module for the CI builds not paying off.

@dzenanz
Copy link
Copy Markdown
Member

dzenanz commented Sep 23, 2021

Review module is fairly small now. I think we can enable it for the CI. PR incoming.

N-Dekker added a commit to SuperElastix/elastix that referenced this pull request Sep 23, 2021
Following ITK pull request InsightSoftwareConsortium/ITK#2567 "STYLE: Avoid repeating parent aliases..." commit
InsightSoftwareConsortium/ITK@babece9 and InsightSoftwareConsortium/ITK@4f30980

Using Visual Studio 2019 Find and Replace regular expressions, like:

    typedef typename Superclass::(\w+)[ ]+\1;
    using typename Superclass::$1;
N-Dekker added a commit to SuperElastix/elastix that referenced this pull request Sep 24, 2021
Following ITK pull request InsightSoftwareConsortium/ITK#2567 "STYLE: Avoid repeating parent aliases..." commit
InsightSoftwareConsortium/ITK@babece9 and InsightSoftwareConsortium/ITK@4f30980

Using Visual Studio 2019 Find and Replace regular expressions, like:

    typedef typename Superclass::(\w+)[ ]+\1;
    using typename Superclass::$1;
N-Dekker added a commit to N-Dekker/ITK that referenced this pull request Oct 5, 2021
Partially reverted pull request InsightSoftwareConsortium#2567
commit 4f30980 "STYLE: Avoid repeating parent aliases"
because of Visual Studio 2017 compile errors reported by Dženan Zukić at
InsightSoftwareConsortium#2759 "COMP: making it compile on VS2017"

Worked around the following VS2017 compile errors:

> error C2653: '...': is not a class or namespace name
> error C2886: '...': symbol cannot be used in a member using-declaration

The compiler bug that caused these errors is reported here:
"Compile error when using "using declaration" referencing a base class type that refers to itself"
EssentiaX - Reported March 12, 2019 [Fixed in version 16.2]
https://developercommunity.visualstudio.com/t/486683
dzenanz pushed a commit that referenced this pull request Oct 5, 2021
Partially reverted pull request #2567
commit 4f30980 "STYLE: Avoid repeating parent aliases"
because of Visual Studio 2017 compile errors reported by Dženan Zukić at
#2759 "COMP: making it compile on VS2017"

Worked around the following VS2017 compile errors:

> error C2653: '...': is not a class or namespace name
> error C2886: '...': symbol cannot be used in a member using-declaration

The compiler bug that caused these errors is reported here:
"Compile error when using "using declaration" referencing a base class type that refers to itself"
EssentiaX - Reported March 12, 2019 [Fixed in version 16.2]
https://developercommunity.visualstudio.com/t/486683
hjmjohnson pushed a commit to hjmjohnson/ITK that referenced this pull request May 6, 2026
Partially reverted pull request InsightSoftwareConsortium#2567
commit 77006be "STYLE: Avoid repeating parent aliases"
because of Visual Studio 2017 compile errors reported by Dženan Zukić at
InsightSoftwareConsortium#2759 "COMP: making it compile on VS2017"

Worked around the following VS2017 compile errors:

> error C2653: '...': is not a class or namespace name
> error C2886: '...': symbol cannot be used in a member using-declaration

The compiler bug that caused these errors is reported here:
"Compile error when using "using declaration" referencing a base class type that refers to itself"
EssentiaX - Reported March 12, 2019 [Fixed in version 16.2]
https://developercommunity.visualstudio.com/t/486683
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 area:Examples Demonstration of the use of classes area:Filtering Issues affecting the Filtering module area:IO Issues affecting the IO module area:Numerics Issues affecting the Numerics module area:Python wrapping Python bindings for a class area:Registration Issues affecting the Registration module area:Segmentation Issues affecting the Segmentation module area:Video Issues affecting the Video module type:Style Style changes: no logic impact (indentation, comments, naming) 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.

5 participants