InsightSoftwareConsortium / InsightSoftwareConsortium/ITK

Consolidate stringify macros: 34 duplicate definitions (`_STRING`/`TOSTRING` + `ITK_VERSION_TO_STRING`)

Open
#6,788 1 comment 0 reactions 0 assignees View on GitHub
area:Core type:Style type:Testing
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

Open the contributing 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

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.