Two C++ refactoring hazards recur in ITK review and are not written down anywhere. Both cause silent behavior changes that compile cleanly, and both have already produced review findings.
Hazard 1 — T x = expr; is construction, not assignment
Converting
T x;
x = expr; // exercises operator=
to
T x = expr; // copy-initialization: exercises a CONSTRUCTOR
changes which special member function runs. For most types the observable
result is identical, but a test whose purpose is to exercise operator= is
silently no longer doing so.
Such tests must keep the two-line form, and should carry a short comment
saying why, so a later mechanical sweep does not re-merge them.
Hazard 2 — adding const can change overload resolution
When const and non-const overloads return different types, adding const
to a variable silently selects a different function with different semantics.
ITK has this exact shape today — verified on upstream/main:
// itkPoint.h:179,183 itkPoint.hxx:118-131
vnl_vector_ref<T> Point<T,N>::GetVnlVector(); // :120 — aliasing VIEW
vnl_vector<T> Point<T,N>::GetVnlVector() const; // :130 — deep COPY
The non-const overload returns a reference wrapper that aliases the point's
storage; the const overload returns an independent copy. Writing const auto v = p.GetVnlVector(); therefore yields a copy, and subsequent writes through
it no longer affect p.
The same const/non-const split exists in itkVector.hxx:142/149 and
itkCovariantVector.hxx:171/178.
Correction to an earlier note: this does not apply to itk::Array, which
derives from vnl_vector and has no GetVnlVector member.
Proposed work
Add both hazards to the C++ guidance under Documentation/ — most naturally
alongside the existing style/modernization material — each as a short entry
with the minimal example above. Two paragraphs; no code changes.
Two C++ refactoring hazards recur in ITK review and are not written down anywhere. Both cause silent behavior changes that compile cleanly, and both have already produced review findings.
Hazard 1 —
T x = expr;is construction, not assignmentConverting
T x; x = expr; // exercises operator=to
T x = expr; // copy-initialization: exercises a CONSTRUCTORchanges which special member function runs. For most types the observable
result is identical, but a test whose purpose is to exercise
operator=issilently no longer doing so.
Such tests must keep the two-line form, and should carry a short comment
saying why, so a later mechanical sweep does not re-merge them.
Hazard 2 — adding
constcan change overload resolutionWhen const and non-const overloads return different types, adding
constto a variable silently selects a different function with different semantics.
ITK has this exact shape today — verified on
upstream/main:The non-const overload returns a reference wrapper that aliases the point's
storage; the const overload returns an independent copy. Writing
const auto v = p.GetVnlVector();therefore yields a copy, and subsequent writes throughit no longer affect
p.The same const/non-const split exists in
itkVector.hxx:142/149anditkCovariantVector.hxx:171/178.Correction to an earlier note: this does not apply to
itk::Array, whichderives from
vnl_vectorand has noGetVnlVectormember.Proposed work
Add both hazards to the C++ guidance under
Documentation/— most naturallyalongside the existing style/modernization material — each as a short entry
with the minimal example above. Two paragraphs; no code changes.