Skip to content

fix(headers): compile after <signal.h> / <sys/param.h> -- rename world_units' si_value, macro-proof MIN/MAX - #588

Merged
transfix merged 1 commit into
masterfrom
fix/world-units-si-value-macro
Oct 9, 2026
Merged

transfix merged 1 commit into
masterfrom
fix/world-units-si-value-macro

Conversation

@transfix

@transfix transfix commented Oct 9, 2026

Copy link
Copy Markdown
Owner

Problem

glibc's <signal.h> (bits/types/siginfo_t.h) defines

#define si_value _sifields._rt.si_sigval

so any TU that includes <signal.h> before <cvc/core/world_units.h> fails to compile:

world_units.h:147:28: error: expected ',' or '...' before '.' token
  147 |   double to_display(double si_value, dimension d) const;

Found building volrover3's GeometryDialog.cpp, IsosurfaceDialog.cpp, ProceduralGeometryDialog.cpp and SDFDialog.cpp against the installed 3.5.0 prefix (transfix/volrover#48).

Changes

  • world_units::to_display / world_units::format: renamed the si_value parameter to canonical_value in the header, world_units.cpp and docs/WORLD_UNITS_API.md. The name pairs with from_display's display_value and drops the si_* prefix, which glibc and musl use for siginfo_t accessor macros. Only parameter names change, so there is no API or ABI change.
  • cvc/utility/utility.h: the audit below found one more real clash. glibc's (and macOS's) <sys/param.h> and GLib define function-like MIN(a,b)/MAX(a,b) macros, which expand the declarations of the cvc::MIN/cvc::MAX templates. The declarator names are now parenthesized (const T &(MIN)(...)). Existing callers are unchanged: cvc::MIN(a, b), unqualified MIN(a, b), MIN<int>(...) and &cvc::MAX<long> all still compile.
  • New system_macro_headers_test (C++20, registered in all five places): the TU includes <signal.h>, <sys/param.h>, <sys/stat.h> and <sys/types.h> on POSIX, or <windows.h> behind the repo's usual WIN32_LEAN_AND_MEAN + NOMINMAX guards, before a representative public header from every module in the cvc target. It leaves out cvc/gl/* (cvcGL, needs VTK/GL), hdf5_utils.h, project_verts.h (CGAL) and the CUDA headers. If a header regresses, this test stops compiling. Its two small TESTs call world_units and (cvc::MIN)/(cvc::MAX).

Macro-name audit of inc/

Every identifier token in all 262 public headers (comments and strings stripped) was checked against:

Source Macros Result
~50 glibc/POSIX headers (signal.h, sys/{stat,types,sysmacros,param,wait,socket,...}.h, errno.h, stdio.h, unistd.h, netinet/*, ...) 7,855 si_value and MIN/MAX (both fixed). The other hits are intentional uses (NULL, UINT32_MAX, std::isfinite, ...).
mingw-w64 <windows.h>, full and with WIN32_LEAN_AND_MEAN+NOMINMAX 27,332 / 18,604 Only RGB, which appears only as the enumerator pixel_format::RGB and is never followed by (, so the function-like macro can't expand.
Explicit list: si_value si_pid si_uid si_addr si_status si_band si_code si_errno sa_handler sa_sigaction st_atime st_mtime st_ctime errno stdin stdout stderr major minor makedev unix linux min max small near far interface ERROR DELETE IN OUT OPTIONAL TRUE FALSE, plus common Win32 A/W names and BSD sys/param.h helpers — Nothing beyond the above. Lowercase min/max (about 70 uses, e.g. voxels::min(), std::numeric_limits<>::max()) only clash with <windows.h> without NOMINMAX, which the repo already handles by convention (config.h, magick_io.cpp, (max)() in async_lane.h).
X11 (Xlib.h, Xutil.h, glx.h), informational 8,590 #define None 0L collides with gl::Switch::None and ariadne::BorderShow::None. Not changed here: the only fix is renaming public enumerators/constants, an API change that deserves its own PR.

Verification

Tested locally with g++ 11, -std=c++20, against this tree's inc/ plus the installed 3.5.0 SDK's headers and defines; gtest was a local stub, not committed.

  • Repro: a TU containing just #include <signal.h> then #include <cvc/core/world_units.h> fails on master and compiles with this branch.
  • New test on master's headers: fails with world_units.h:147/163 and utility.h:77/78 errors. On this branch: compiles with no warnings under -Wall -Wextra, also under -std=gnu++20. Linked against libcvc 3.5.0, both TESTs pass.
  • world_units.cpp: compiles, including with -include signal.h.
  • check_test_targets.py: OK: all 161 test executables reach TEST_TARGETS.
  • clang-format: emulated git clang-format --diff origin/master reports 0 files to reformat, and the new file is also clean when checked in full. The system headers sit in their own #ifndef _WIN32 block, so include regrouping can't move them after the libcvc headers.
  • Includes: all 28 <cvc/...> includes in the test resolve in inc/ and are installed via install(DIRECTORY inc/cvc/).
  • Not checked locally: Windows/MSVC and macOS, which are left to CI.

…d_units' si_value, macro-proof MIN/MAX

glibc's <signal.h> defines `#define si_value _sifields._rt.si_sigval`, so
any TU that includes it before <cvc/core/world_units.h> fails:

  world_units.h:147:28: error: expected ',' or '...' before '.' token

Seen building volrover3's GeometryDialog/IsosurfaceDialog/
ProceduralGeometryDialog/SDFDialog against the installed 3.5.0 prefix
(transfix/volrover#48).

- world_units::to_display/format: rename the `si_value` parameter to
  `canonical_value` (pairs with from_display's `display_value`; no si_*
  prefix, which glibc/musl reserve for siginfo_t accessor macros).
  Parameter names only, so no ABI/API change; docs updated to match.
- utility.h: an audit of every identifier in inc/ against the macros
  defined by ~50 glibc/POSIX headers found one more clash: the cvc::MIN /
  cvc::MAX templates break after <sys/param.h> (also macOS, GLib), whose
  function-like MIN(a,b)/MAX(a,b) expand the declarations. Parenthesize
  the declarator names; existing callers are unchanged.
- New system_macro_headers_test: includes <signal.h>, <sys/param.h>,
  <sys/stat.h>, <sys/types.h> (POSIX) or <windows.h> (the repo's
  WIN32_LEAN_AND_MEAN + NOMINMAX pattern) before a representative
  public header from every cvc module, so a regression fails the build.
@transfix
transfix merged commit c3058cb into master Oct 9, 2026
14 checks passed
transfix added a commit that referenced this pull request Oct 9, 2026
Patch release carrying the fixes since 3.5.0: the cvcGL M_PI build fix
(#582), CameraController Map mode holding its pose (#584), the tag-release /
publish workflow fixes (#585, #586), macro-safe public headers after
<signal.h> / <sys/param.h> (#588), and the X11 `None` renames (#589:
ariadne::BorderShow::None -> Off, gl::Switch::None -> Off). Bumps
project(VERSION) 3.5.0 -> 3.5.1 and every cvcpkg recipe's upstream_version to
match (cvc_revision stays 1, per the recipe-version gate).
@transfix
transfix deleted the fix/world-units-si-value-macro branch October 10, 2026 03:38
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant