Skip to content

ENH: Default override VNL 1D FFT class destructors#2815

Merged
thewtex merged 2 commits intoInsightSoftwareConsortium:masterfrom
jhlegarreta:DefaultOverrideVnl1DFFTClassDestructors
Oct 15, 2021
Merged

ENH: Default override VNL 1D FFT class destructors#2815
thewtex merged 2 commits intoInsightSoftwareConsortium:masterfrom
jhlegarreta:DefaultOverrideVnl1DFFTClassDestructors

Conversation

@jhlegarreta
Copy link
Copy Markdown
Member

  • COMP: Mark class destructors with override
  • STYLE: Prefer = default to explicitly trivial implementations

PR Checklist

Mark class destructors with `override`.

Fixes:
```

In file included from /Users/builder/externalModules/Filtering/FFT/test/itkComplexToComplex1DFFTImageFilterTest.cxx:27:
In file included from /Users/builder/externalModules/Filtering/FFT/include/itkComplexToComplex1DFFTImageFilter.h:122:
In file included from /Users/builder/externalModules/Filtering/FFT/include/itkComplexToComplex1DFFTImageFilter.hxx:23:
[CTest: warning matched] /Users/builder/externalModules/Filtering/FFT/include/itkVnlComplexToComplex1DFFTImageFilter.h:61:11:
warning: '~VnlComplexToComplex1DFFTImageFilter' overrides a destructor but is not marked 'override' [-Winconsistent-missing-destructor-override]
  virtual ~VnlComplexToComplex1DFFTImageFilter() {}
          ^
```

and
```
In file included from /Users/builder/externalModules/Filtering/FFT/test/itkFFT1DImageFilterTest.cxx:25:
In file included from /Users/builder/externalModules/Filtering/FFT/test/itkForward1DFFTImageFilterTest.cxx:27:

In file included from /Users/builder/externalModules/Filtering/FFT/include/itkForward1DFFTImageFilter.h:101:
In file included from /Users/builder/externalModules/Filtering/FFT/include/itkForward1DFFTImageFilter.hxx:23:
[CTest: warning matched] /Users/builder/externalModules/Filtering/FFT/include/itkVnlForward1DFFTImageFilter.h:62:11:
warning: '~VnlForward1DFFTImageFilter' overrides a destructor but is not marked 'override' [-Winconsistent-missing-destructor-override]
  virtual ~VnlForward1DFFTImageFilter() {}
          ^
```

raised for example at:
https://open.cdash.org/viewBuildError.php?type=1&onlydeltap&buildid=7512728
This check replaces default bodies of special member functions with
= default;. The explicitly defaulted function declarations enable more
opportunities in optimization, because the compiler might treat
explicitly defaulted functions as trivial.

Additionally, the C++11 use of = default more clearly expreses the
intent for the special member functions.
@jhlegarreta
Copy link
Copy Markdown
Member Author

Left behind in #2800. Really sorry @seanm.

@github-actions github-actions Bot added area:Filtering Issues affecting the Filtering module type:Enhancement Improvement of existing methods or implementation labels Oct 14, 2021
Copy link
Copy Markdown
Contributor

@tbirdso tbirdso left a comment

Choose a reason for hiding this comment

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

While we're at it we should fix similar constructor/destructors in the FFTW equivalent classes (itkFFTWComplexToComplex1DFFTImageFilter, itkFFTWForward1DFFTImageFilter, itkFFTWInverse1DFFTImageFilter). @jhlegarreta would you prefer that I add this in a separate PR?

@jhlegarreta
Copy link
Copy Markdown
Member Author

While we're at it we should fix similar constructor/destructors in the FFTW equivalent classes (itkFFTWComplexToComplex1DFFTImageFilter, itkFFTWForward1DFFTImageFilter, itkFFTWInverse1DFFTImageFilter). @jhlegarreta would you prefer that I add this in a separate PR?

Seems reasonable. I think best is to amend the commits in this PR. Please go ahead.

Not sure if it was me that again missed them or whether they are not raising warnings.

@seanm
Copy link
Copy Markdown
Contributor

seanm commented Oct 14, 2021

lgtm

@tbirdso
Copy link
Copy Markdown
Contributor

tbirdso commented Oct 14, 2021

@jhlegarreta it looks like I don't have permission to amend your branch, I'll just make the changes in another PR. On second review FFTW classes are acting a little differently anyway with non-default constructors and destructors.

Copy link
Copy Markdown
Member

@thewtex thewtex left a comment

Choose a reason for hiding this comment

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

@jhlegarreta thanks!

@thewtex thewtex merged commit b1a0a91 into InsightSoftwareConsortium:master Oct 15, 2021
@jhlegarreta jhlegarreta deleted the DefaultOverrideVnl1DFFTClassDestructors branch October 15, 2021 01:46
hjmjohnson pushed a commit to hjmjohnson/ITK that referenced this pull request May 6, 2026
…faultOverrideVnl1DFFTClassDestructors

ENH: Default override VNL 1D FFT class destructors
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

area:Filtering Issues affecting the Filtering module type:Enhancement Improvement of existing methods or implementation

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants