From cbe4ac365451e5ba9f462f5e3a7f7e9f5c8db0ad Mon Sep 17 00:00:00 2001 From: Claude Date: Tue, 28 Jul 2026 15:38:42 +0000 Subject: [PATCH 1/2] Hold TapTools' own code to the family's warning standard; export tap::tools TapTools was the one library in the family with no warnings interface target and no WERROR option -- every sibling carries the pair (AMBITAP_WERROR / TAP_DSP_WERROR / MUTAP_WERROR / TAP_RATIO_WERROR / SRT_WERROR) -- so its kernel compiled without -Wall -Wextra -Wpedantic -Wconversion -Wshadow while everything around it did not. Adds `taptools_warnings` and `TAPTOOLS_WERROR` with exactly the flag set the siblings use, and links the test target against it. Turning them on surfaced 42 warnings. Two were in a shipping header and are fixed here: vco.h's `tri_tick` took a `dt` parameter it never used and that both call sites already filled with `adt`, and `waveform_out_peek` took an `adt` it has no use for -- peek reads the triangle integrator rather than ticking it, so it adds no BLEP correction, which is now stated where the parameter used to be. Both are private helpers of a nested class with no callers outside this header, so removing the vestigial parameters is a no-op for behavior: the suite still passes 1637526 assertions across 156 test cases. The remaining 34 are -Wconversion in test files. WERROR is therefore left OFF and not yet enabled in CI -- the same staged approach the siblings took with MSVC /W4, where the flag waits until the output has been triaged. Also exports `tap::tools`, the alias taphouse's namespace convention documented but this repo never created (`TapTools::taptools` stays, so consumers are unaffected); fixes the book's site-url, which pointed at /TapTools/book/ while docs.yml serves the book from /TapTools/, giving mdBook's generated 404.html a broken home link; adds book/book/ to .gitignore, which AmbiTap and MuTap already ignore and which a local mdbook build otherwise leaves untracked; and corrects a header comment that still named the pre-split `taptools` namespace. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_01JhhQ93r2E1QTnCx46YfX8j --- .gitignore | 3 +++ CMakeLists.txt | 26 +++++++++++++++++++++++++- book/book.toml | 6 +++++- include/taptools/vco.h | 14 ++++++++------ tests/CMakeLists.txt | 2 +- 5 files changed, 42 insertions(+), 9 deletions(-) diff --git a/.gitignore b/.gitignore index 37b2ca0..6f2a246 100644 --- a/.gitignore +++ b/.gitignore @@ -15,3 +15,6 @@ __pycache__/ notebooks/_tr808_ref/ notebooks/_tr808_ours/ notebooks/_swing_figs/ + +# mdBook output (book/ sources build into here; deployed by CI) +book/book/ diff --git a/CMakeLists.txt b/CMakeLists.txt index f0f48a8..9177f20 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -2,7 +2,7 @@ # # TapTools kernel — the portable DSP library behind the TapTools Max package. # Header-only, plain C++20: no Max SDK, no min-api, no Jamoma. The kernels live -# under include/taptools/, one self-contained header per object, in the `taptools` namespace. +# under include/taptools/, one self-contained header per object, in the `tap::tools` namespace. # # This is a complete standalone CMake project (the AmbiTap / AmbiTap-Max pattern): it lives in # kernel/ of the TapTools monorepo and is designed to lift verbatim into its own repository. The @@ -36,6 +36,12 @@ add_subdirectory(submodules/dsptap) add_library(taptools INTERFACE) add_library(TapTools::taptools ALIAS taptools) +# The family-wide alias, per the namespace convention in taphouse's README: one +# `tap::` sub-namespace per repo, and the CMake alias matches it. The +# older spelling above stays so existing consumers keep working; prefer +# `tap::tools` in new code. Note this is a build-tree alias only -- the +# installed config package still exports under its own namespace. +add_library(tap::tools ALIAS taptools) target_compile_features(taptools INTERFACE cxx_std_20) target_include_directories(taptools INTERFACE $ @@ -43,6 +49,24 @@ target_include_directories(taptools INTERFACE # tap::dsp brings the real-FFT header and links the Ooura static lib. target_link_libraries(taptools INTERFACE tap::dsp) +# Warning flags for this project's own targets (tests, tools, benchmarks) -- +# never exported to consumers of the INTERFACE library, and never applied to +# third-party code. Every sibling *Tap library carries this pair +# (AMBITAP_WERROR / TAP_DSP_WERROR / MUTAP_WERROR / TAP_RATIO_WERROR / +# SRT_WERROR); TapTools was the one library holding its own code to a lower +# standard than the rest of the family, so the flag set here is deliberately the +# same one they use. +option(TAPTOOLS_WERROR "Treat warnings as errors in TapTools' own targets" OFF) +add_library(taptools_warnings INTERFACE) +target_compile_options(taptools_warnings INTERFACE + $<$:-Wall -Wextra -Wpedantic -Wconversion -Wshadow> + $<$:/W4 /permissive->) +if (TAPTOOLS_WERROR) + target_compile_options(taptools_warnings INTERFACE + $<$:-Werror> + $<$:/WX>) +endif () + if (TAPTOOLS_BUILD_TESTS) enable_testing() add_subdirectory(tests) diff --git a/book/book.toml b/book/book.toml index 3d68ab0..4288ef9 100644 --- a/book/book.toml +++ b/book/book.toml @@ -11,4 +11,8 @@ create-missing = false [output.html] default-theme = "rust" git-repository-url = "https://github.com/tap/TapTools" -site-url = "/TapTools/book/" +# docs.yml uploads book/book as the Pages artifact root, so the book is served +# from /TapTools/, not /TapTools/book/. site-url feeds the absolute links in the +# generated 404.html; pointing it at a path the site does not have gave the 404 +# page a broken "home" link. +site-url = "/TapTools/" diff --git a/include/taptools/vco.h b/include/taptools/vco.h index 169447a..2cb3e38 100644 --- a/include/taptools/vco.h +++ b/include/taptools/vco.h @@ -406,7 +406,7 @@ namespace tap::tools { // Triangle: leaky integration of the BLEP square. Only ticked when the morph needs it. // tri_pw skews the square's duty away from 0.5 (imperfect: asymmetry -> even harmonics). - double tri_tick(double p, double dt, double adt, double tri_pw) { + double tri_tick(double p, double adt, double tri_pw) { const double sq = pulse_at(p, adt, tri_pw); m_tri_state = 0.999 * m_tri_state + 4.0 * adt * sq; return m_tri_state; @@ -420,11 +420,11 @@ namespace tap::tools { if (a <= 0.0) { return s; } - return (1.0 - a) * s + a * tri_tick(p, adt, adt, tri_pw); + return (1.0 - a) * s + a * tri_tick(p, adt, tri_pw); } if (shape <= 2.0) { // triangle -> saw const double a = shape - 1.0; - const double t = tri_tick(p, adt, adt, tri_pw); + const double t = tri_tick(p, adt, tri_pw); if (a <= 0.0) { return t; } @@ -467,8 +467,8 @@ namespace tap::tools { const double frac = m_sync_prev / (m_sync_prev - sync); // 0..1 within this sample const double p_old = wrap01(m_phase + dt * frac); const double p_new = (1.0 - frac) * dt; - const double d = waveform_out_peek(p_old, adt, shape, pw, bend) - - waveform_out_peek(wrap01(p_new), adt, shape, pw, bend); + const double d = waveform_out_peek(p_old, shape, pw, bend) + - waveform_out_peek(wrap01(p_new), shape, pw, bend); // one-sided first-order correction of the reset step (minBLEP is the upgrade path) const double x = 1.0 - frac; correction += d * 0.5 * x * x; @@ -501,7 +501,9 @@ namespace tap::tools { } // Waveform value without advancing the triangle integrator (for sync discontinuity sizing). - double waveform_out_peek(double p, double adt, double shape, double pw, double bend) const { + // No adt parameter: unlike waveform_out(), peek adds no BLEP correction (it reads the + // triangle integrator rather than ticking it), so it has no use for the window width. + double waveform_out_peek(double p, double shape, double pw, double bend) const { if (shape <= 1.0) { const double a = shape; const double s = std::sin(2.0 * k_pi * bent(p, 0.5 * bend)); diff --git a/tests/CMakeLists.txt b/tests/CMakeLists.txt index b0720e2..ab3f2ba 100644 --- a/tests/CMakeLists.txt +++ b/tests/CMakeLists.txt @@ -34,7 +34,7 @@ add_executable(taptools_kernel_tests vocoder_test.cpp conv_engine_test.cpp ) -target_link_libraries(taptools_kernel_tests PRIVATE TapTools::taptools Catch2::Catch2WithMain) +target_link_libraries(taptools_kernel_tests PRIVATE tap::tools Catch2::Catch2WithMain taptools_warnings) set_target_properties(taptools_kernel_tests PROPERTIES CXX_STANDARD 20 CXX_STANDARD_REQUIRED ON From 5bccde82bfe92b8ed4e6bf372f05414fc335f0df Mon Sep 17 00:00:00 2001 From: Claude Date: Tue, 28 Jul 2026 17:37:55 +0000 Subject: [PATCH 2/2] Reformat the vco.h line the parameter removal left mis-aligned The style job's clang-format gate failed at include/taptools/vco.h:470. Removing waveform_out_peek's unused `adt` parameter shortened both calls, so the manual continuation-line alignment on that statement no longer matched what clang-format produces; it now fits differently and the formatter wraps it after the `=`. My miss: I ran clang-format over the .cpp files I touched in TapTools-Max but not over this repo's include/*.h, which its style job also globs. Now verified by reproducing the exact CI invocation -- `clang-format --dry-run --Werror` over `git ls-files 'include/*.h' 'tests/*.cpp' 'tools/*.cpp' 'tools/*.h' 'bench/*.cpp'` with clang-format 18.1.3, the pinned version -- clean across the whole set. Formatting only: rebuilt and re-ran the suite, still 1637526 assertions across 156 test cases, and include/taptools/ still compiles warning-free under the new taptools_warnings flags. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_01JhhQ93r2E1QTnCx46YfX8j --- include/taptools/vco.h | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/include/taptools/vco.h b/include/taptools/vco.h index 2cb3e38..f69c2c3 100644 --- a/include/taptools/vco.h +++ b/include/taptools/vco.h @@ -467,8 +467,8 @@ namespace tap::tools { const double frac = m_sync_prev / (m_sync_prev - sync); // 0..1 within this sample const double p_old = wrap01(m_phase + dt * frac); const double p_new = (1.0 - frac) * dt; - const double d = waveform_out_peek(p_old, shape, pw, bend) - - waveform_out_peek(wrap01(p_new), shape, pw, bend); + const double d = + waveform_out_peek(p_old, shape, pw, bend) - waveform_out_peek(wrap01(p_new), shape, pw, bend); // one-sided first-order correction of the reset step (minBLEP is the upgrade path) const double x = 1.0 - frac; correction += d * 0.5 * x * x;