AcademySoftwareFoundation / AcademySoftwareFoundation/openvdb
NanoVDB: make the nano components warning-clean under OPENVDB_CXX_STRICT and enable it in NanoVDB CI
- Dominant language
- C++
- Stars
- 3.4k
- Forks
- 774
- Avg merge
- 3d 9h
- Merged PRs (30d)
- 34
Description
### Context
#2280 made every CUDA translation unit in the NanoVDB build warnings-as-errors by default (`NANOVDB_CUDA_WERROR`, covering both the NVCC front end and the forwarded host-compiler pass). The remaining gap is pure host TUs — `.cc` sources such as `TestNanoVDB.cc`, the tools, and the python bindings — which build with no `-Werror` at all.
The repository already has the mechanism: `OPENVDB_CXX_STRICT` (`-Werror -Wall -Wextra` plus the curated per-compiler set in `OpenVDBCXX.cmake`), which the core library builds with in CI (`build.yml`). The NanoVDB components have never built under it, and today they cannot: replicating the strict GNU and Clang flag sets over a standalone NanoVDB build (unit tests + tools + examples, Release and Debug) fails on pre-existing warnings.
### Measured inventory
**gcc (13.3): 74 unique sites**
| Category | Sites | Notes |
|---|---|---|
| `-Wunused-parameter` | 39 | mostly named-but-unused params in header interfaces, e.g. the `dummy` parameter of `DeviceBuffer::create` |
| `-Wunused-function` | 17 | file-static functions in the portable C headers (CNanoVDB.h, PNanoVDB.h) — see policy question below |
| `-Wsign-compare` | 10 | Debug-only: inside `assert` bodies, so Release builds never see them |
| `-Wdangling-else` | 3 | |
| `-Wpedantic` / `-Wformat` / `-Wunused-variable` | 5 | includes `%p` passed non-`void*` in an example |
**clang: 33 unique sites** — overlapping categories plus `-Wconversion` (in the Clang strict set but not the GNU one) in NanoVDB.h, PNanoVDB.h, math/Math.h and tools/CreateNanoGrid.h.
Raw-error counts are much higher (~180 sign-compare instances in a Debug build) because header sites repeat per TU; the unique-site counts above are the real work.
### Policy decisions needed
1. **CNanoVDB.h / PNanoVDB.h**: file-static functions are the design of these portable C-style headers, and PNanoVDB.h is synced with external consumers. Options: attribute/edit every site, suppress `-Wunused-function` at the including test TUs, or exclude these headers' test TUs from strict. Editing PNanoVDB.h piecemeal seems wrong; a deliberate choice is needed.
2. **clang `-Wconversion`** in public headers is semantic-adjacent (narrowing arithmetic in core math paths) and needs careful review rather than mechanical casts.
### Suggested direction
Fix in slices, each independently reviewable: (1) the mechanical classes (unused parameters/variables, dangling-else, format); (2) the Debug-only sign-compares in asserts; (3) the C-header policy decision and its implementation; (4) `-Wconversion` last. Once clean, add `-DOPENVDB_CXX_STRICT=ON` to the `linux-nanovdb` matrix in `nanovdb.yml` (all four gcc/clang × Release/Debug configs), mirroring what `build.yml` already does for the core. `OPENVDB_CXX_STRICT` itself should stay opt-in in CMake — it is a repo-wide option — but on in CI; the CUDA-side `NANOVDB_CUDA_WERROR` remains default-ON as landed in #2280.
Contributor guide
Research direction
Compare the strict compiler setup in OpenVDBCXX.cmake and build.yml, then reproduce the standalone NanoVDB Release and Debug builds with GCC and Clang to verify the warning inventory. Review CNanoVDB.h and PNanoVDB.h policy choices before addressing the listed warning classes. Done means all four linux-nanovdb configurations are warning-clean and nanovdb.yml enables OPENVDB_CXX_STRICT for them.
Written by the indexing model from the issue text.
Assessment
- Tech stack
- cmake, cpp, github-actions
- Domain
- build-system, ci-cd
- Issue type
- Refactor
- Difficulty
- 5/5
- Estimated time
- Over a week
- Activity status
- Quiet
- Clarity
- Mostly clear
- Newbie friendliness
- 38/100