InsightSoftwareConsortium / InsightSoftwareConsortium/ITK
DOC: Document two silent refactoring hazards — declare-then-init construction and const-addition overload changes
- 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
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