COMP: Remove in-class {} member initializers of unique_ptr by SimonRit · Pull Request #3877 · InsightSoftwareConsortium/ITK

SimonRit · 2023-01-20T14:10:07Z

Errors showed up in (e.g.) https://my.cdash.org/viewBuildError.php?buildid=2273357

Patch 5e2c49f causes the following errors on GCC 7.4.1:
/usr/include/c++/7/bits/unique_ptr.h:76:22: error: invalid application of ‘sizeof’ to incomplete type ‘itk::NiftiImageIO::NiftiImageProxy’ /usr/include/c++/7/bits/unique_ptr.h:76:22: error: invalid application of ‘sizeof’ to incomplete type ‘itk::LBFGSBOptimizerHelper’ /usr/include/c++/7/bits/unique_ptr.h:76:22: error: invalid application of ‘sizeof’ to incomplete type ‘gdcm::SerieHelper’ /usr/include/c++/7/bits/unique_ptr.h:76:22: error: invalid application of ‘sizeof’ to incomplete type ‘itk::NiftiImageIO::NiftiImageProxy’ /usr/include/c++/7/bits/unique_ptr.h:76:22: error: invalid application of ‘sizeof’ to incomplete type ‘itk::GiftiMeshIO::GiftiImageProxy’ /usr/include/c++/7/bits/unique_ptr.h:76:22: error: invalid application of ‘sizeof’ to incomplete type ‘itk::GiftiMeshIO::GiftiImageProxy’

The issue has already been fixed in the past, see
d7c0128

All instances have been removed with the script:
for i in $(find Modules -type f)
do
sed -i "s/(unique_ptr.*) *{}/\1/g" $i
done

PR Checklist

No API changes were made (or the changes have been approved)
No major design changes were made (or the changes have been approved)
Added test (or behavior not changed)
Updated API documentation (or API not changed)
Added license to new files (if any)
Added Python wrapping to new files (if any) as described in ITK Software Guide Section 9.5
Added ITK examples for all new major features (if any)

Refer to the ITK Software Guide for
further development details if necessary.

N-Dekker · 2023-01-20T14:17:01Z

Wow, thanks @SimonRit I knew about an in-class {nullptr} being problematic because of this unique_ptr bug, but I did not know that even an empty {} is still problematic for this old GCC version.

But I see now, at https://godbolt.org/z/8Y5vMMYfG

#include <memory>

class ForwardDeclaredClass;

class Object
{
  std::unique_ptr<ForwardDeclaredClass> m_Data{};
};

Still produces:

/opt/compiler-explorer/gcc-9.1.0/include/c++/9.1.0/bits/unique_ptr.h:79:16: error: invalid application of 'sizeof' to incomplete type 'ForwardDeclaredClass'
79 | static_assert(sizeof(_Tp)>0,

Apparently, this bug is fixed with GCC 9.2.

Update March 24, 2023: For the record, this was indeed a GCC bug, and indeed, fixed with GCC 9.2: Bug 85552 - Adding curly braces to the declaration of a std::unique_ptr to a forward declared class breaks compilation, reported by Parker Coates, 2018-04-27.

dzenanz · 2023-01-20T14:34:46Z

Modules/Core/ImageFunction/include/itkBSplineInterpolateImageFunction.h

-  std::unique_ptr<vnl_matrix<long>[]>   m_ThreadedEvaluateIndex {};
-  std::unique_ptr<vnl_matrix<double>[]> m_ThreadedWeights {};
-  std::unique_ptr<vnl_matrix<double>[]> m_ThreadedWeightsDerivative {};
+  std::unique_ptr<vnl_matrix<long>[]>   m_ThreadedEvaluateIndex ;


Remove space before semicolon. linter complains

These spaces were actually introduced automatically by Clang-Format (with my PR #3851), when adding the {}! I think it's a Clang-Format bug!

Just force pushed a slightly modified version of the regexp to cope with this, linter is happy now!

Patch 5e2c49f causes the following errors on GCC 7.4.1: /usr/include/c++/7/bits/unique_ptr.h:76:22: error: invalid application of ‘sizeof’ to incomplete type ‘itk::NiftiImageIO::NiftiImageProxy’ /usr/include/c++/7/bits/unique_ptr.h:76:22: error: invalid application of ‘sizeof’ to incomplete type ‘itk::LBFGSBOptimizerHelper’ /usr/include/c++/7/bits/unique_ptr.h:76:22: error: invalid application of ‘sizeof’ to incomplete type ‘gdcm::SerieHelper’ /usr/include/c++/7/bits/unique_ptr.h:76:22: error: invalid application of ‘sizeof’ to incomplete type ‘itk::NiftiImageIO::NiftiImageProxy’ /usr/include/c++/7/bits/unique_ptr.h:76:22: error: invalid application of ‘sizeof’ to incomplete type ‘itk::GiftiMeshIO::GiftiImageProxy’ /usr/include/c++/7/bits/unique_ptr.h:76:22: error: invalid application of ‘sizeof’ to incomplete type ‘itk::GiftiMeshIO::GiftiImageProxy’ The issue has already been fixed in the past, see d7c0128 All instances have been removed with the script: for i in $(find Modules -type f) do sed -i "s/$unique_ptr.*$$ *{}$/\1/g" $i done

N-Dekker

As far as I can see, the old GCC errors only occur for unique_ptr<T> data members whose type T is a forward-declared class. So it should not affect all unique_ptr data members.

Anyway, I see that it's easiest to just remove the in-class {} initializers from all of them. Approved.

…ration GCC 7.4.1 raised the following errors on Linux: Modules/Core/Common/include/itkSmartPointer.h:214:18: error: invalid use of incomplete type ‘using ObjectType = class itk::MultiThreaderBase {aka class itk::MultiThreaderBase}’ This is a compiler bug (as discussed in InsightSoftwareConsortium#3877 (comment) and the same issue has been fixed for unique_ptr by 719c3ce.

GCC 7.4.1 raised the following errors on Linux: Modules/Core/Common/include/itkSmartPointer.h:214:18: error: invalid use of incomplete type ‘using ObjectType = class itk::MultiThreaderBase {aka class itk::MultiThreaderBase}’ This is a compiler bug (as discussed in InsightSoftwareConsortium#3877 (comment) and the same issue has been fixed for unique_ptr by 719c3ce.

GCC 7.4.1 raised the following errors on Linux: Modules/Core/Common/include/itkSmartPointer.h:214:18: error: invalid use of incomplete type ‘using ObjectType = class itk::MultiThreaderBase {aka class itk::MultiThreaderBase}’ This is a compiler bug (as discussed in InsightSoftwareConsortium#3877 (comment)) and the same issue has been fixed for unique_ptr by 719c3ce.

GCC 7.4.1 raised the following errors on Linux: Modules/Core/Common/include/itkSmartPointer.h:214:18: error: invalid use of incomplete type ‘using ObjectType = class itk::MultiThreaderBase {aka class itk::MultiThreaderBase}’ This is a compiler bug (as discussed in #3877 (comment)) and the same issue has been fixed for unique_ptr by 719c3ce.

Excluded padding data, `std::unique_ptr`, `itk::SmartPointer` and low level utility classes from the guideline that says that "all member variables must be initialized when they are declared". `std::unique_ptr` and `itk::SmartPointer` are excluded because of some GCC compile errors, which were addressed in ITK by Simon Rit: - pull request InsightSoftwareConsortium/ITK#3877 commit InsightSoftwareConsortium/ITK@eac289d "COMP: Remove in-class {} member initializers of unique_ptr" - pull request InsightSoftwareConsortium/ITK#3927 commit InsightSoftwareConsortium/ITK@f5f8367 "COMP: Remove in class init of SmartPointer of forward declaration"

github-actions bot added area:Core Issues affecting the Core module area:IO Issues affecting the IO module area:Numerics Issues affecting the Numerics module area:Registration Issues affecting the Registration module type:Compiler Compiler support or related warnings labels Jan 20, 2023

dzenanz approved these changes Jan 20, 2023

View reviewed changes

SimonRit force-pushed the UniquePtrInit branch from d84575e to 719c3ce Compare January 20, 2023 14:39

N-Dekker approved these changes Jan 20, 2023

View reviewed changes

dzenanz merged commit eac289d into InsightSoftwareConsortium:master Jan 20, 2023

SimonRit mentioned this pull request Feb 15, 2023

COMP: Remove in class init of SmartPointer of forward declaration #3927

Merged

7 tasks

N-Dekker mentioned this pull request Mar 1, 2023

DOC: Adjust initialization rule for padding data, smart pointers, etc. InsightSoftwareConsortium/ITKSoftwareGuide#192

Merged

SimonRit deleted the UniquePtrInit branch January 19, 2024 09:30

N-Dekker mentioned this pull request Mar 31, 2026

COMP: Apply C++ Core Guidelines smart pointer fixes #5993

Closed

5 tasks

This was referenced Mar 31, 2026

COMP: Replace raw pointers with std::unique_ptr in VoxBoCUBImageIO #5997

Merged

DOC: Add compiler-cautions.md — compiler-specific pitfalls from 10 years of COMP: commits #5999

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

COMP: Remove in-class {} member initializers of unique_ptr#3877

COMP: Remove in-class {} member initializers of unique_ptr#3877
dzenanz merged 1 commit intoInsightSoftwareConsortium:masterfrom
SimonRit:UniquePtrInit

SimonRit commented Jan 20, 2023 •

edited

Loading

Uh oh!

N-Dekker commented Jan 20, 2023 •

edited

Loading

Uh oh!

dzenanz Jan 20, 2023

Uh oh!

N-Dekker Jan 20, 2023 •

edited

Loading

Uh oh!

SimonRit Jan 20, 2023

Uh oh!

N-Dekker left a comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

Uh oh!

Conversation

SimonRit commented Jan 20, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

PR Checklist

Uh oh!

N-Dekker commented Jan 20, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

dzenanz Jan 20, 2023

Choose a reason for hiding this comment

Uh oh!

N-Dekker Jan 20, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

SimonRit Jan 20, 2023

Choose a reason for hiding this comment

Uh oh!

N-Dekker left a comment

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

SimonRit commented Jan 20, 2023 •

edited

Loading

N-Dekker commented Jan 20, 2023 •

edited

Loading

N-Dekker Jan 20, 2023 •

edited

Loading