DOC: Document two silent refactoring hazards — declare-then-init construction and const-addition overload changes

Open Beginner friendly
#6,752 0 comments 0 reactions 0 assignees View on GitHub

Nobody has claimed this yet.

Assessment

Difficulty
2/5
Estimated time
1-3 hours
Newbie friendliness
76/100
Issue type
Documentation
Clarity
Mostly clear
Activity status
Quiet
Tech stack
cpp
Domain
documentation

Research direction

Start in Documentation/ by locating the existing C++ style or modernization guidance, then review the cited itkPoint.h, itkPoint.hxx, itkVector.hxx, and itkCovariantVector.hxx references for context. Add two short entries covering declare-then-init construction and const-dependent overload resolution, including the minimal examples and the itk::Array correction; done means documentation only, with no code changes.

Written by the indexing model from the issue text.

Description

area:Documentation type:Documentation

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.

Dominant language
C++
Stars
1.7k
Forks
749
Avg merge
1d 4h
Merged PRs (30d)
57

Contributor guide

Open the contributing guide

First steps

  1. Read the whole issue, then the project's contributing guide.
  2. Comment on the issue to say you are picking it up — it saves two people doing the same work.
  3. Fork the repository and make your change on a branch.
  4. Open a pull request that references the issue number.

More from InsightSoftwareConsortium/ITK

All issues in InsightSoftwareConsortium/ITK

Similar issues

More C++ issues

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.