Repository navigation
refactor(core): C++23 pilot — log.c (real C++23, supersedes #42) - #45
Merged
Merged
Conversation
6 tasks done
lusoris
marked this pull request as ready for review
May 28, 2026 15:26
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>
6 tasks done
lusoris
force-pushed
the
feat/cpp23-pilot-log-v2-20260528
branch
2 times, most recently
from
May 28, 2026 19:32
bfda5e1 to
c4a4d24
Compare
This was referenced May 28, 2026
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
force-pushed
the
feat/cpp23-pilot-log-v2-20260528
branch
from
May 28, 2026 21:56
c4a4d24 to
fa4fc4b
Compare
13 tasks done
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>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Converts
core/src/log.ctocore/src/log.cppusing 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 (replacesconst char *C designated-initialiser arrays)std::clampfor bounds clamping (replaces paired ternary guards)nullptrthroughout;extern "C"guards inlog.hCompiled as an isolated
log_cpp23_libstatic library withoverride_options: ['cpp_std=c++23']— mirrors themem_cpp23_libpattern from PR #41 (ADR-0720) exactly. Test binaries consume log symbols vialog_cpp23_objs(extract_all_objects) rather than listinglog.cppinline in sources; the inline approach would inherit the project's default C++ standard and silently dropstring_view/clamp.Also removes the orphaned
test_ansnr_simdblock whose source file was deleted with the ansnr feature drop.Closes #42
Type
refactor— no behavior changeChecklist
make format && make lintis green locally.meson test -C build --suite=fast→ 50/50 OK./cross-backend-diffand the worst ULP is ≤ 2..c/.cpp/.cu/.h/.hpp, it has the appropriate license header.!orBREAKING CHANGE:and the migration path is documented below.docs/adr/README.md.Bug-status hygiene
no state delta: pure internal refactor, no bug opened or closed
Netflix golden-data gate
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)
docs/adr/0725-cpp23-pilot-log-v2.md§ Alternatives consideredAGENTS.mdinvariant note — added tocore/AGENTS.md§ Rebase-sensitive invariantschangelog.d/changed/0725-cpp23-log-v2.mddocs/rebase-notes.md§core/src/log.cppReproducer
What is different from PR #42
constexpr std::array<const char *, 4>constexpr std::array<std::string_view, 4>clamp_val<T>templatestd::clamp(standard library)cpp_std=c++11cpp_std=c++23log.cpplisted inline in sourceslog_cpp23_objsinobjects:list🤖 Generated with Claude Code