InsightSoftwareConsortium / InsightSoftwareConsortium/ITK
Consolidate stringify macros: 34 duplicate definitions (`_STRING`/`TOSTRING` + `ITK_VERSION_TO_STRING`)
- Dominant language
- C++
- Stars
- 1.7k
- Forks
- 748
- Avg merge
- 1d 1h
- Merged PRs (30d)
- 64
Description
ITK has **33 hand-rolled copies** of the same two-level stringify idiom across 17 files. Sixteen use `_STRING`, a name reserved to the implementation, so each is undefined behavior as well as a duplicate. [PR #6787](https://github.com/InsightSoftwareConsortium/ITK/pull/6787) adds a general `ITK_STRINGIFY` to `itkMacro.h`; this issue tracks removing the duplicates once that lands.
**Blocked on #6787** — `ITK_STRINGIFY` must exist before anything here can be done. No action needed until that merges.
Why _STRING is a defect, not just a duplicate
```c
#define _STRING(s) #s // 16 GTest .cxx files
```
Identifiers that begin with an underscore followed by an uppercase letter are reserved to the implementation **in all scopes** ([lex.name]/3.1). Defining one is undefined behavior and can collide with a compiler or standard-library macro. This is also why the replacement must not be named `_ITK_STRINGIFY_HELPER` — the leading underscore is the one prefix that cannot portably mean "private".
All 16 are in test `.cxx` files; **none is in an installed header**, so nothing downstream can depend on them.
Scope: the 17 sites
**1. Sixteen files, each defining BOTH halves of the idiom (32 definitions)**
Every one of these files defines `_STRING` *and* a `TOSTRING` partner:
```c
#define _STRING(s) #s
#define TOSTRING(s) std::string(_STRING(s)) // 12 files -> std::string
#define TOSTRING(s) _STRING(s) // 4 files -> const char*
```
Both `TOSTRING` shapes are correct two-level stringifies; they differ only in
return type. The 4 `const char*` ones map directly to `ITK_STRINGIFY`; the 12
`std::string` ones become `std::string(ITK_STRINGIFY(s))` at the call site (or
keep a single shared helper if the wrapping is worth preserving).
Files:
```
Modules/IO/GDCM/test/itkGDCMImageIOGTest.cxx
Modules/IO/GDCM/test/itkGDCMImageIOInvalidVRTypeGTest.cxx
Modules/IO/GDCM/test/itkGDCMSeriesDirectionGTest.cxx
Modules/IO/GDCM/test/itkGDCMSeriesFileNamesContractGTest.cxx
Modules/IO/GIPL/test/itkGiplImageIOGTest.cxx
Modules/IO/HDF5/test/itkHDF5ImageIOGTest.cxx
Modules/IO/IOTransformDCMTK/test/itkDCMTKSeriesFileNamesOrderingGTest.cxx
Modules/IO/IOTransformDCMTK/test/itkDCMTKSeriesFileNamesParityGTest.cxx
Modules/IO/ImageBase/test/itkWriteImageFunctionGTest.cxx
Modules/IO/JPEG/test/itkJPEGImageIOGTest.cxx
Modules/IO/NIFTI/test/itkNiftiImageIOGTest.cxx
Modules/IO/PNG/test/itkPNGImageIOGTest.cxx
Modules/IO/TIFF/test/itkImageSeriesReaderReverse.cxx
Modules/IO/TIFF/test/itkTIFFImageIOGTest.cxx
Modules/IO/TIFF/test/itkTIFFImageIOInt32Test.cxx
Modules/IO/TransformMatlab/test/itkIOTransformMatlabGTest.cxx
```
**2. `itkVersion.h:34-35`** — `ITK_VERSION_TO_STRING` / `ITK_VERSION_TO_STRING0`, the same idiom under a correct name, now redundant.
Total: 32 definitions in test sources + 2 in `itkVersion.h` = **34 macro definitions collapsing to one pair in `itkMacro.h`**.
Downstream impact scan — no negative effects found
Scanned the forest build testbed (33 projects, source files only, build directories pruned): MITK, Plastimatch, elastix, ITKSNAP, RTK, TubeTK, c3d, SimpleITK, Slicer, BRAINSTools, ANTs, AlizaMS, IGSIO, vtkAddon, PlusLib, OpenIGTLink, Ultrasound, TractographyTRX, LesionSizingToolkit, and others.
| Check | Result |
|---|---|
| `ITK_STRINGIFY` occurrences | **0** — the new public macro collides with nothing |
| `#define _STRING(` definitions | **ITK only: 16, all in test `.cxx`, 0 in headers** |
| `ITK_VERSION_TO_STRING` consumers | **0** outside its own definition |
Two scanning caveats for anyone reproducing this:
- A naive `grep -rl '_STRING('` returns 86 files across 8 projects. Those are *uses* of other projects' own macros ending in `_STRING(` (Slicer 44, MITK 6, Plastimatch 5, …), plus `ITK_VERSION_TO_STRING(` matching as a substring. Only `#define _STRING(` isolates the reserved-name definitions.
- Grepping the whole forest in one command **times out** on the multi-GB build trees, and the pipeline can still exit 0 — producing a convincing false "zero hits". Scan per project with a timeout.
Removing `ITK_VERSION_TO_STRING` deletes a macro from an installed public header. The scan is strong evidence but not exhaustive; if a reviewer objects, a one-line back-compat alias (`#define ITK_VERSION_TO_STRING(x) ITK_STRINGIFY(x)`) preserves compatibility while still removing the duplication.
Suggested approach
1. Wait for #6787 to merge.
2. **Commit 1** — `itkVersion.h` uses `ITK_STRINGIFY`. Expansion verified byte-identical (`"6" "." "0" "." "0"` both before and after). Note that clang-format reflows the backslash continuations; stage its fix and re-commit.
3. **Commit 2** — in each of the 16 test files, delete both `#define _STRING(s) #s` and `#define TOSTRING(s) ...`, and switch uses to `ITK_STRINGIFY` (wrapping in `std::string(...)` for the 12 files whose `TOSTRING` returned one). Add an `itkMacro.h` include where one is not already reached transitively.
4. Build with `BUILD_SHARED_LIBS=ON` and run the affected GTest drivers; these are IO-module tests and several need test data.
5. The helper is named `ITK_STRINGIFY_HELPER` as landed in #6787. Do **not** rename it to a leading-underscore form — that is the very defect being removed here.
Contributor guide
Research direction
Wait for PR #6787 and read the new ITK_STRINGIFY in itkMacro.h first. Then update itkVersion.h and the 16 listed IO test .cxx files, removing duplicate macros and adding itkMacro.h where needed. Run the affected IO GTest drivers; done means the duplicate definitions are gone and tests build and pass.
Written by the indexing model from the issue text.
Assessment
- Tech stack
- cpp
- Domain
- testing, tooling
- Issue type
- Refactor
- Difficulty
- 4/5
- Estimated time
- 3-5 days
- Activity status
- Stale
- Clarity
- Clearly specified
- Newbie friendliness
- 35/100