InsightSoftwareConsortium / InsightSoftwareConsortium/ITK

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
area:Documentation type:Documentation
Dominant language
C++
Stars
1.7k
Forks
748
Avg merge
1d 1h
Merged PRs (30d)
64

Description

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

```cpp
T x;
x = expr; // exercises operator=
```

to

```cpp
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`:

```cpp
// itkPoint.h:179,183 itkPoint.hxx:118-131
vnl_vector_ref Point::GetVnlVector(); // :120 — aliasing VIEW
vnl_vector Point::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.

Contributor guide

Open the contributing guide

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.

Assessment

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

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.