InsightSoftwareConsortium / InsightSoftwareConsortium/ITK
COMP: Audit ITK's compiler warning flags and suppressions for the C++17 era
- Dominant language
- C++
- Stars
- 1.7k
- Forks
- 748
- Avg merge
- 1d 1h
- Merged PRs (30d)
- 64
Description
ITK's default warning/flag set in `CMake/ITKSetStandardCompilerFlags.cmake` has accreted since ~2009 and still contains entries whose original rationale is obsolete or compiler-inconsistent. Recent PRs removed four of them piecemeal; this issue tracks the systematic per-flag audit, and preserves the MSVC triage plan from the now-closed WIP PR #6442.
Motivating case: -Wno-deprecated (now removed)
`-Wno-deprecated` originated in commit `a33ca7ac0d7` (2009-02-12, Bill Lorensen, *"suppress deprecated header warnings for gcc compilers that support it"*), added inside an `IF(CMAKE_COMPILER_IS_GNUCXX)` guard to silence warnings about deprecated C++ *headers* (``, ``, ``). Commit `8a135bcb128` (2011-08-23) consolidated it into the cross-compiler list, dropping the GCC guard. `[[deprecated]]` did not exist until C++14.
Consequence: on Clang, `-Wno-deprecated` also disables `-Wdeprecated-declarations`, silently hiding every `[[deprecated]]` API warning; on GCC the two are independent. A stale 2009 header workaround had become a load-bearing, compiler-inconsistent suppression of ITK's own deprecation diagnostics.
Removing it (#6430) unmasked ~5,100 `[-Wdeprecated-declarations]` in an ITK-only build — ~99% from `itkMath.h` aliasing/re-exporting `vnl_math::` (cleared by the `itk::Math` decouple, #6427) plus 2 `-Wdeprecated-copy` in vendored GDCM. Every remaining flag deserves the same history-and-rationale treatment.
Current flag inventory on main (verified 2026-07-29)
From `CMake/ITKSetStandardCompilerFlags.cmake`:
```cmake
# C compiler only
set(c_flags -Wno-unused-parameter)
# -W3 on WIN32 (verbosity cap to keep Windows-header noise manageable), else -Wall
set(VerboseWarningsFlag ...)
# C and C++
set(c_and_cxx_flags
${VerboseWarningsFlag}
-Wcast-align -Wdisabled-optimization -Wextra -Wformat=2
-Winvalid-pch -Wpointer-arith -Wshadow -Wunused -Wwrite-strings)
# C++ only
set(cxx_flags
-Wno-undefined-var-template # suppress invalid warning when explicitly instantiated in another TU
-Woverloaded-virtual -Wctad-maybe-unsupported -Wstrict-null-sentinel)
```
Already removed by earlier work — do **not** re-audit these, they are gone: `-Wno-deprecated`, `-Wno-invalid-offsetof` (#6430), `-Wno-format-nonliteral`, `-Wno-strict-overflow` (#6442/#6444 lineage).
Override knob (unchanged): `ITK_C_WARNING_FLAGS` / `ITK_CXX_WARNING_FLAGS`, cache `STRING`, `mark_as_advanced`, checked at `:331` and `:347-352`. Setting either on the command line bypasses the computed list entirely.
Salvaged plan from the closed WIP PR #6442
PR #6442 (`WIP: Compiler-flag suppression audit (CI triage — do not merge)`) was closed unmerged on 2026-06-15. It was never a merge candidate — it deliberately removed suppressions so CI would re-surface the underlying warnings for triage, with the stated goal of determining *which suppressions are still load-bearing and which are dead, then fixing the code rather than re-suppressing*. Its plan is recorded here so it survives the PR scrolling out of view.
**Suppressions it exposed for CI triage:**
- **MSVC:** C4127 (`if (constant)`), C4244 (narrowing conversion), C4305 (truncation on assignment), C4309 (constant truncation)
- **GCC/Clang:** global `-Wno-strict-overflow` removed
- **Already dropped in separate commits, expected clean:** global `-Wno-format-nonliteral` (5 targeted suppressions retained), obsolete Solaris flags, dead MSVC `C4786`/`C4996` pragmas
**Why CI-first for the Windows warnings:** MSVC C4127/C4244/C4305/C4309 cannot be exercised on the Linux/macOS workstations driving the audit, so Windows CI is necessarily the first executor for those. The GCC `-Wno-strict-overflow` removal was triaged in parallel on a local Linux build.
**Recorded post-merge action:** *split confirmed real fixes into focused `COMP:` PRs; re-suppress only verified false positives.*
Working notes lived in a throwaway `Utilities/CompilerFlagAudit/` directory on branch `compiler-suppression-audit`, explicitly slated for removal before any real review — that directory is not on `main`. The MSVC C4127/C4244/C4305/C4309 triage is the main piece of #6442's plan that remains **unfinished**.
Suggested approach
1. Enumerate every flag in `ITKSetStandardCompilerFlags.cmake` (warnings *and* the non-warning required flags it sets), plus any per-module or per-target flag additions elsewhere under `CMake/`.
2. For each, `git log -S "" --follow` to recover the introducing commit, date, author, and stated rationale — and note whether it was originally compiler-guarded and whether that guard survived later consolidation.
3. Classify: (a) still needed on the current supported-platform/compiler matrix; (b) obsolete (targets a removed compiler, standard, or header); (c) compiler-inconsistent (means different things on GCC vs Clang vs MSVC — the `-Wno-deprecated` failure mode); (d) too-broad suppression hiding legitimate diagnostics.
4. Recommend keep / drop / narrow per flag with C++17-era justification. Prefer replacing a blanket `-Wno-X` with a targeted pragma at the few genuine sites.
5. Pick up the unfinished MSVC C4127/C4244/C4305/C4309 triage from #6442, using Windows CI as the first executor and splitting confirmed real fixes into focused `COMP:` PRs.
6. Sequence any large unmasking behind the cleanup that clears the resulting warning flood, as #6427 did for #6430.
Related
- PR #6430 — `COMP:` remove default `-Wno-deprecated` (merged 2026-06-11)
- PR #6444 — remove dead Intel-ICC / Apple / C warning config (merged 2026-06-15)
- PR #6442 — WIP CI-triage branch, **closed unmerged** 2026-06-15; plan salvaged above
- PR #6427 — `itk::Math` decouple from `vnl_math::` (cleared the unmasked warning flood)
- `a33ca7ac0d7` (2009-02-12) — origin of `-Wno-deprecated`; `8a135bcb128` (2011-08-23) — cross-compiler consolidation
- `CMake/ITKSetStandardCompilerFlags.cmake` — the flag lists and the `ITK_CXX_WARNING_FLAGS` override
Contributor guide
Research direction
Start with CMake/ITKSetStandardCompilerFlags.cmake, including the flag lists and ITK_C_WARNING_FLAGS/ITK_CXX_WARNING_FLAGS overrides, then use git log -S for each flag. Run the relevant Linux build and Windows CI to investigate the MSVC C4127, C4244, C4305, and C4309 warnings. Done means every flag has a documented C++17-era rationale and the MSVC triage is split into focused COMP: changes.
Written by the indexing model from the issue text.
Assessment
- Tech stack
- cmake, cpp
- Domain
- build-system, ci-cd, compilers
- Issue type
- Refactor
- Difficulty
- 5/5
- Estimated time
- Over a week
- Activity status
- Quiet
- Clarity
- Mostly clear
- Newbie friendliness
- 38/100