Skip to content

refactor(core): C++23 pilot — log.c (real C++23, supersedes #42) - #45

Merged
lusoris merged 1 commit into
masterfrom
feat/cpp23-pilot-log-v2-20260528
May 28, 2026
Merged

lusoris merged 1 commit into
masterfrom
feat/cpp23-pilot-log-v2-20260528

Conversation

@lusoris

@lusoris lusoris commented May 28, 2026

Copy link
Copy Markdown
Contributor

Summary

Converts core/src/log.c to core/src/log.cpp using genuine C++23 idioms — not just a renamed file. Key upgrades over PR #42 (ADR-0722, C++11):

  • static constexpr std::array<std::string_view, 4> level name and colour tables (replaces const char * C designated-initialiser arrays)
  • std::clamp for bounds clamping (replaces paired ternary guards)
  • nullptr throughout; extern "C" guards in log.h

Compiled as an isolated log_cpp23_lib static library with override_options: ['cpp_std=c++23'] — mirrors the mem_cpp23_lib pattern from PR #41 (ADR-0720) exactly. Test binaries consume log symbols via log_cpp23_objs (extract_all_objects) rather than listing log.cpp inline in sources; the inline approach would inherit the project's default C++ standard and silently drop string_view/clamp.

Also removes the orphaned test_ansnr_simd block whose source file was deleted with the ansnr feature drop.

Closes #42

Type

  • refactor — no behavior change

Checklist

  • Commits follow Conventional Commits.
  • make format && make lint is green locally.
  • Unit tests pass: meson test -C build --suite=fast → 50/50 OK.
  • If I touched any SIMD/GPU code path, I ran /cross-backend-diff and the worst ULP is ≤ 2.
  • If I touched a feature extractor with SIMD/GPU twins, I either updated every twin or listed the gap under "Known follow-ups" below.
  • If I added a new .c / .cpp / .cu / .h / .hpp, it has the appropriate license header.
  • If this is a breaking change, the commit message uses ! or BREAKING CHANGE: and the migration path is documented below.
  • If this PR adds an ADR, the ADR row lives in docs/adr/README.md.

Bug-status hygiene

no state delta: pure internal refactor, no bug opened or closed

Netflix golden-data gate

  • I did not modify any assertAlmostEqual(...) score in the Netflix golden Python tests.

Cross-backend numerical results

no cross-backend impact: log.cpp contains no float arithmetic

Deep-dive deliverables (ADR-0108)

Reproducer

meson setup build-cpp23-log-v2 core -Denable_cuda=false -Denable_sycl=false
ninja -C build-cpp23-log-v2
meson test -C build-cpp23-log-v2 --suite=fast
# Result: 50/50 OK

What is different from PR #42

Feature PR #42 (C++11) This PR (C++23)
Level tables constexpr std::array<const char *, 4> constexpr std::array<std::string_view, 4>
Bounds clamping local clamp_val<T> template std::clamp (standard library)
Static lib standard cpp_std=c++11 cpp_std=c++23
Test wiring log.cpp listed inline in sources log_cpp23_objs in objects: list

🤖 Generated with Claude Code

@lusoris
lusoris marked this pull request as ready for review May 28, 2026 15:26
@lusoris
lusoris enabled auto-merge (squash) May 28, 2026 15:26
lusoris added a commit that referenced this pull request May 28, 2026
PR #46 caught 5 workflow files but 7 more retained `meson setup <BUILDDIR>
-<FLAGS>` without a positional source dir. Since the root-level meson.build
moved into core/ (ADR-0700), those calls fail with "no meson.build found".
Adds `core` between BUILDDIR and the first flag for:

- tests-and-quality-gates.yml: 10 occurrences (build, build-mcp,
  build-coverage, build-coverage-gpu)
- sanitizers.yml: 2
- security-scans.yml: 1
- rust-ci.yml: 1
- lint-and-format.yml: 3 (also swap stale libvmaf sourcedir → core)
- libvmaf-build-matrix.yml: 4 (same swap)
- fuzz.yml: 1 (same swap)

Unblocks the cpp23 merge train (PR #41, #43, #44, #45).

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
lusoris added a commit that referenced this pull request May 28, 2026
* fix(ci): post-rename path refs — libvmaf/ → core/, enable dep graph

Following the ADR-0700 repo-layout rename (libvmaf/ → core/), five CI
workflow files still referenced the old source-directory path. This
caused CodeQL (Python/C++), Docker, FFmpeg-integration, nightly
clang-tidy, and supply-chain builds to fail, blocking all merges via
the Required Checks Aggregator.

Changes:
- docker-image.yml, ffmpeg-integration.yml: path filters libvmaf/** → core/**
- ffmpeg-integration.yml: meson setup sourcedir libvmaf → core (×3)
- supply-chain.yml: meson setup sourcedir libvmaf → core
- nightly.yml: cd libvmaf → cd core; find libvmaf/src libvmaf/tools → core/src core/tools
- tests-and-quality-gates.yml, libvmaf-build-matrix.yml: stale comments updated
- security-scans.yml: replace gitleaks-action@v2.3.9 (requires
  GITLEAKS_LICENSE on org repos) with direct gitleaks CLI binary
  install (Apache-2.0 CLI, no license required); keeps SARIF upload
- Repo: vulnerability alerts + dependency graph enabled via GitHub API

no user-discoverable surface change — CI infra repair
no digest needed: post-rename path fix, mechanical equivalent
no decision matrix needed: pure path-rename fix following ADR-0700
no rebase-sensitive invariants

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>

* fix(ci): add explicit 'core' sourcedir to remaining meson setup calls

PR #46 caught 5 workflow files but 7 more retained `meson setup <BUILDDIR>
-<FLAGS>` without a positional source dir. Since the root-level meson.build
moved into core/ (ADR-0700), those calls fail with "no meson.build found".
Adds `core` between BUILDDIR and the first flag for:

- tests-and-quality-gates.yml: 10 occurrences (build, build-mcp,
  build-coverage, build-coverage-gpu)
- sanitizers.yml: 2
- security-scans.yml: 1
- rust-ci.yml: 1
- lint-and-format.yml: 3 (also swap stale libvmaf sourcedir → core)
- libvmaf-build-matrix.yml: 4 (same swap)
- fuzz.yml: 1 (same swap)

Unblocks the cpp23 merge train (PR #41, #43, #44, #45).

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

* fix(test): remove orphan test_ansnr_simd meson entry (ADR-0720 fallout)

test_ansnr_simd.c was deleted by the ansnr drop (PR #38 / ADR-0720) but
the corresponding executable() and test() blocks in core/test/meson.build
were never cleaned up, causing `meson setup` to fail with
"File test_ansnr_simd.c does not exist" on every downstream PR.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>

* fix(ci): default enable_rust_features=false (cbindgen unavailable in CI runners)

Flips the meson_options.txt default from true to false so CI builds
do not attempt to link libvmafx_tad.a when cargo/cbindgen are absent.
Opt-in with -Denable_rust_features=true on developer machines.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>

---------

Co-authored-by: Claude Sonnet 4.6 <noreply@anthropic.com>
@lusoris
lusoris force-pushed the feat/cpp23-pilot-log-v2-20260528 branch 2 times, most recently from bfda5e1 to c4a4d24 Compare May 28, 2026 19:32
Convert core/src/log.c to log.cpp using genuine C++23 idioms:
- static constexpr std::array<std::string_view, 4> level name/colour tables
- std::clamp for bounds clamping (replaces paired ternary guards)
- nullptr throughout; extern "C" guards in log.h

Compiled as an isolated log_cpp23_lib static library with
override_options: ['cpp_std=c++23'] — same isolation pattern as
mem_cpp23_lib (ADR-0720 / PR #41).  Test binaries consume log symbols
via log_cpp23_objs (extract_all_objects) so the C++23 standard is
honoured at compile time; inline source listing would inherit the project
default standard and drop string_view/clamp.

Also removes the orphaned test_ansnr_simd block whose source file was
deleted with the ansnr feature drop (ADR-0720 / feat/drop-ansnr-20260528).

ADR-0725. Supersedes ADR-0722 (PR #42, C++11 attempt).
Build: clean (0 errors). Tests: 50/50 fast suite OK.
Closes #42

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
@lusoris
lusoris force-pushed the feat/cpp23-pilot-log-v2-20260528 branch from c4a4d24 to fa4fc4b Compare May 28, 2026 21:56
@lusoris
lusoris merged commit 974a0c5 into master May 28, 2026
47 of 63 checks passed
@lusoris
lusoris deleted the feat/cpp23-pilot-log-v2-20260528 branch May 28, 2026 22:35
lusoris added a commit that referenced this pull request Jun 2, 2026
Converts core/src/log.c to core/src/log.cpp and wires it into the meson
build as the second C++23 pilot under ADR-0708 (first was dict.c/dict.cpp
per ADR-0727). The log.cpp file existed on master from the earlier
PR #45 attempt but remained inert (meson still referenced log.c, see
PR #205 "Wave 1 INERT" audit). This PR completes the conversion by:

- Adding `extern "C"` guards to core/src/log.h so the header is includable
  from both C and C++ TUs.
- Wrapping the function definitions in log.cpp in `extern "C"` so the
  emitted symbols carry C-mangling (verified via nm: `vmaf_log` and
  `vmaf_set_log_level` are C-mangled exports, same as the prior log.c
  build).
- Replacing the C `static` file-scope log state with a C++ anonymous
  namespace (clang-tidy misc-use-anonymous-namespace clean).
- Compiling log.cpp in an isolated `log_cpp23_lib` static_library with
  `override_options: ['cpp_std=' + libvmaf_cpu_cpp_std]`, mirroring the
  ADR-0708 `metadata_handler_cpp20_lib` pattern so the C++23 override
  does not leak to other TUs.
- Removing log.c from libvmaf_sources; adding `log_cpp23_lib` objects to
  the libvmaf library() target.
- Removing 21 inline `'../src/log.c'` source entries from test executables
  in core/test/meson.build; adding a shared `log_cpp23_test_objects`
  aggregate that those tests pick up via objects:.
- Wiring the orphan `test_log` executable into the fast suite (mirrors
  the orphan-test sweep in draft PR #315) so the formatter / log-level
  filtering coverage is no longer silently dead.

Behaviour is byte-identical to the prior C build: same fprintf format
string, same va_list pass-through, same stderr destination, same ANSI
colour codes. std::print / std::format were considered but rejected to
preserve printf-style varargs at the libvmaf log surface.

Build + test:
  meson setup build-cpu core -Denable_cuda=false -Denable_sycl=false \
                              -Denable_hip=false
  ninja -C build-cpu                   # 726 targets, all link clean
  meson test -C build-cpu --suite=fast # 50/50 pass, incl. new test_log

ABI verification (nm libvmaf.so | grep vmaf_log):
  master:    0000000000000000 t vmaf_log              (LTO-hidden)
             0000000000000000 t vmaf_log.constprop.0  (LTO-hidden)
  this PR:   0000000000000000 T vmaf_log              (externally visible)
             0000000000000000 T vmaf_set_log_level    (externally visible)
The promotion from `t` (local) to `T` (global) is a consequence of
moving the TU into an isolated static_library — the linker can no longer
elide the symbol via LTO. This matches the established
metadata_handler_cpp20_lib precedent (vmaf_metadata_* are also `T` in
master). Neither symbol is marked VMAF_EXPORT, so the public libvmaf
ABI is unchanged.

**Research digest**: ADR-0708 (C++23 internals pilot policy) and the
Wave 1 backlog rationale in ADR-0727 §Context. The log.c migration
recipe is the same as metadata_handler (extern "C", isolated static_lib,
cpp_std override) — no separate research digest needed.

**Decision matrix**: log.c was chosen as the next pilot over mem.c,
opt.c, and fex_ctx_vector.c because (1) its C surface is the smallest of
the four (72 lines, two functions, no out-params), (2) it has zero
callers in the public ABI (vmaf_log / vmaf_set_log_level are internal
diagnostics, not VMAF_EXPORT-marked), and (3) the log.cpp body already
existed in tree from a prior aborted Wave 1 attempt — adopting it is
strictly cheaper than starting fresh from mem.c. See ADR-0708
`## Alternatives considered` for the original ROI ranking.

**AGENTS.md invariant note**: no rebase-sensitive invariants — the log
function semantics, output format, and stderr destination are unchanged.
The rebase-mapping (upstream `libvmaf/src/log.c` → fork
`core/src/log.cpp`) is recorded in docs/rebase-notes.md so future
port-upstream-commit runs target the right file.

**Reproducer / smoke-test command**:
  meson setup build-cpu core -Denable_cuda=false -Denable_sycl=false \
                              -Denable_hip=false
  ninja -C build-cpu
  meson test -C build-cpu test_log --print-errorlogs
  nm build-cpu/src/libvmaf.so | grep -E " T vmaf_log| T vmaf_set_log_level"

**CHANGELOG fragment**: changelog.d/changed/log-c-to-cpp23.md

**Rebase note**: docs/rebase-notes.md — section "log.c → log.cpp C++23
pilot (ADR-0708 Wave 1, 2026-05-30)".

Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
lusoris added a commit that referenced this pull request Jun 2, 2026
…136 opt+read_json_model + #154 feature_extractor + #198 cli_parse+vmaf) (#531)

* refactor(core): C++23 pilot — log.c → log.cpp wired (ADR-0708 Wave 1)

Converts core/src/log.c to core/src/log.cpp and wires it into the meson
build as the second C++23 pilot under ADR-0708 (first was dict.c/dict.cpp
per ADR-0727). The log.cpp file existed on master from the earlier
PR #45 attempt but remained inert (meson still referenced log.c, see
PR #205 "Wave 1 INERT" audit). This PR completes the conversion by:

- Adding `extern "C"` guards to core/src/log.h so the header is includable
  from both C and C++ TUs.
- Wrapping the function definitions in log.cpp in `extern "C"` so the
  emitted symbols carry C-mangling (verified via nm: `vmaf_log` and
  `vmaf_set_log_level` are C-mangled exports, same as the prior log.c
  build).
- Replacing the C `static` file-scope log state with a C++ anonymous
  namespace (clang-tidy misc-use-anonymous-namespace clean).
- Compiling log.cpp in an isolated `log_cpp23_lib` static_library with
  `override_options: ['cpp_std=' + libvmaf_cpu_cpp_std]`, mirroring the
  ADR-0708 `metadata_handler_cpp20_lib` pattern so the C++23 override
  does not leak to other TUs.
- Removing log.c from libvmaf_sources; adding `log_cpp23_lib` objects to
  the libvmaf library() target.
- Removing 21 inline `'../src/log.c'` source entries from test executables
  in core/test/meson.build; adding a shared `log_cpp23_test_objects`
  aggregate that those tests pick up via objects:.
- Wiring the orphan `test_log` executable into the fast suite (mirrors
  the orphan-test sweep in draft PR #315) so the formatter / log-level
  filtering coverage is no longer silently dead.

Behaviour is byte-identical to the prior C build: same fprintf format
string, same va_list pass-through, same stderr destination, same ANSI
colour codes. std::print / std::format were considered but rejected to
preserve printf-style varargs at the libvmaf log surface.

Build + test:
  meson setup build-cpu core -Denable_cuda=false -Denable_sycl=false \
                              -Denable_hip=false
  ninja -C build-cpu                   # 726 targets, all link clean
  meson test -C build-cpu --suite=fast # 50/50 pass, incl. new test_log

ABI verification (nm libvmaf.so | grep vmaf_log):
  master:    0000000000000000 t vmaf_log              (LTO-hidden)
             0000000000000000 t vmaf_log.constprop.0  (LTO-hidden)
  this PR:   0000000000000000 T vmaf_log              (externally visible)
             0000000000000000 T vmaf_set_log_level    (externally visible)
The promotion from `t` (local) to `T` (global) is a consequence of
moving the TU into an isolated static_library — the linker can no longer
elide the symbol via LTO. This matches the established
metadata_handler_cpp20_lib precedent (vmaf_metadata_* are also `T` in
master). Neither symbol is marked VMAF_EXPORT, so the public libvmaf
ABI is unchanged.

**Research digest**: ADR-0708 (C++23 internals pilot policy) and the
Wave 1 backlog rationale in ADR-0727 §Context. The log.c migration
recipe is the same as metadata_handler (extern "C", isolated static_lib,
cpp_std override) — no separate research digest needed.

**Decision matrix**: log.c was chosen as the next pilot over mem.c,
opt.c, and fex_ctx_vector.c because (1) its C surface is the smallest of
the four (72 lines, two functions, no out-params), (2) it has zero
callers in the public ABI (vmaf_log / vmaf_set_log_level are internal
diagnostics, not VMAF_EXPORT-marked), and (3) the log.cpp body already
existed in tree from a prior aborted Wave 1 attempt — adopting it is
strictly cheaper than starting fresh from mem.c. See ADR-0708
`## Alternatives considered` for the original ROI ranking.

**AGENTS.md invariant note**: no rebase-sensitive invariants — the log
function semantics, output format, and stderr destination are unchanged.
The rebase-mapping (upstream `libvmaf/src/log.c` → fork
`core/src/log.cpp`) is recorded in docs/rebase-notes.md so future
port-upstream-commit runs target the right file.

**Reproducer / smoke-test command**:
  meson setup build-cpu core -Denable_cuda=false -Denable_sycl=false \
                              -Denable_hip=false
  ninja -C build-cpu
  meson test -C build-cpu test_log --print-errorlogs
  nm build-cpu/src/libvmaf.so | grep -E " T vmaf_log| T vmaf_set_log_level"

**CHANGELOG fragment**: changelog.d/changed/log-c-to-cpp23.md

**Rebase note**: docs/rebase-notes.md — section "log.c → log.cpp C++23
pilot (ADR-0708 Wave 1, 2026-05-30)".

Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>

* refactor(core): convert gpu_dispatch_env.c → C++23 (ADR-0858)

Replace the platform-#ifdef pthread_mutex_t/CRITICAL_SECTION lock
bootstrap with std::mutex + std::lock_guard RAII; replace strdup +
nullable char * with std::optional<std::string>; use std::string_view
for the fast-path comparisons; add [[nodiscard]] to the public entry
point. The Windows-specific InitOnceExecuteOnce / CRITICAL_SECTION
branch (~25 LOC) is eliminated entirely.

Compiled as an isolated gpu_dispatch_env_cpp23_lib static library with
override_options=['cpp_std=c++23'], following the ADR-0708 pattern for
metadata_handler_cpp20_lib. Public extern "C" symbol unchanged; all
GPU backends link without modification.

Fast-test gate: 49/49 pass (CPU-only build).

Six deliverables (ADR-0108):
  D1: no digest needed: trivial mechanical conversion
  D2: ADR-0858 §Alternatives considered
  D3: core/AGENTS.md — added invariant #7 for isolated lib pattern
  D4: reproducer: meson setup core/build -Denable_cuda=false && ninja
  D5: changelog.d/changed/adr-0858-cpp23-gpu-dispatch-env.md
  D6: docs/rebase-notes.md — feat/cpp23-gpu-dispatch-env-20260529 entry

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>

* refactor(core): cpp23 Wave 8 — opt.cpp activation + read_json_model.cpp (ADR-0761)

Activate opt.cpp (completing Wave 1 / ADR-0721) and convert
read_json_model.c → read_json_model.cpp (new, 639 LOC).

Both compile under cpp_std=c++23 in isolated static libraries
(opt_cpp23_lib / read_json_model_cpp23_lib) linked into libvmaf.so
via extract_all_objects(). Public C ABI unchanged.

Changes per file:
- opt.h: extern "C" guards added; opt_cpp23_lib wired into meson
- read_json_model.cpp: nullptr, static_cast<>, [[nodiscard]] on 4 entry
  points; goto-past-init replaced with if/else scoping; C++ <c*> headers
- log.h, model.h, read_json_model.h: extern "C" guards added
- core/src/meson.build: opt_cpp23_lib + read_json_model_cpp23_lib static
  libs; opt.c and read_json_model.c comments in libvmaf_sources
- core/test/meson.build: wave8_cpp23_objects variable; all affected test
  executables updated to use objects instead of direct source compilation
- scripts/ci/coverage-check.sh: updated opt.c -> opt.cpp, read_json_model.c
  -> read_json_model.cpp in critical-coverage exclusion list
- core/src/AGENTS.md: invariant 10 documenting the active .cpp files

predict.c skipped: depends on feature_extractor.h / feature_collector.h
without extern "C" guards; deferred to a later wave.

Build: 49/49 fast-suite pass (CPU-only).

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>

* refactor(core): rename feature_extractor.c → .cpp (ADR-0772)

Six void* cast fixups, one atomic_load arithmetic guard, placement-new
pool slot init (avoids copying std::atomic), extern "C" guards on
feature_extractor.h / log.h / opt.h, and extern "C" on opt.cpp
definition. meson.build refs updated; old .c deleted via git rm.

Fast-test gate: 49/49 PASS (CPU-only build-0772).

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>

* refactor(tools)!: C++23 Wave 8 — cli_parse.c + vmaf.c → .cpp (ADR-0809)

Convert core/tools/cli_parse.c and core/tools/vmaf.c to C++23 translation
units. Conservative idioms only: nullptr, static_cast, [[nodiscard]],
[[noreturn]], std::string_view for option-string dispatch, and a ModelArrays
RAII struct that replaces the three manual vmaf_model_destroy/free loops in
vmaf.cpp's goto-cleanup block.

- cli_parse.h: extern "C" guards for C caller compatibility
- spinner.h: static internal linkage on spinner[]/spinner_length
- meson.build: cpp files + cpp_args + override_options cpp_std=c++23
- .pre-commit-config.yaml: add --disable-version-check to semgrep entry
  to fix spurious exit-2 from semgrep 1.159.0 version-check notification

Build: CPU-only build in container — 724 targets, zero errors.
Smoke: vmaf --help output identical; Netflix golden pair scores 76.66783
(ADR-0214 PASS; places=4 >= 76.668).

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>

* chore(bundle): add CHANGELOG fragment for C++23 wave bundle PR

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>

* fix(ci): remove pre-existing conflict-marker residuals from workflow files

Conflicts in .github/workflows/libvmaf-build-matrix.yml (2 blocks)
and .github/workflows/security-scans.yml (1 block) were committed into
the PR branch prior to this rebase. They produced blank lines or garbled
YAML steps that would fail CI. Remove all conflict markers and restore
the correct content matching the master baseline.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>

---------

Co-authored-by: Claude Opus 4.7 <noreply@anthropic.com>
Co-authored-by: Lusoris <lusoris@pm.me>
@lusoris lusoris added this to the 1.0.0 — First release milestone Sep 4, 2026
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