From d9e473e371807c0db60270ce0a6a3cf53a90eecd Mon Sep 17 00:00:00 2001 From: Rafik Saliev Date: Fri, 8 May 2026 12:21:08 +0200 Subject: [PATCH 01/12] [CPPRuntime] Force -Winline-namespace-reopened-noninline --- bindings/cpp/CMakeLists.txt | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/bindings/cpp/CMakeLists.txt b/bindings/cpp/CMakeLists.txt index 85db0c72..6473d3e6 100644 --- a/bindings/cpp/CMakeLists.txt +++ b/bindings/cpp/CMakeLists.txt @@ -82,6 +82,10 @@ target_compile_options(${TARGET_NAME} PRIVATE -fvisibility=hidden ) +if(CMAKE_CXX_COMPILER_ID STREQUAL "Clang") + target_compile_options(${TARGET_NAME} PRIVATE -Winline-namespace-reopened-noninline) +endif() + if(UNIX AND NOT APPLE) # Don't export 3rd-party symbols from the lib target_link_options(${TARGET_NAME} PRIVATE "SHELL:-Wl,--exclude-libs,ALL") From d3ef2c888fc7c07cc787aa79db0873f7f8e6d31d Mon Sep 17 00:00:00 2001 From: Rafik Saliev Date: Fri, 8 May 2026 13:07:15 +0200 Subject: [PATCH 02/12] Enhance build scripts to support Clang as an alternative compiler for cpp runtime bindings --- .github/scripts/build-cpp-runtime-bindings.sh | 7 ++++++- .github/workflows/build-cpp-runtime-bindings.yml | 11 +++++++++++ 2 files changed, 17 insertions(+), 1 deletion(-) diff --git a/.github/scripts/build-cpp-runtime-bindings.sh b/.github/scripts/build-cpp-runtime-bindings.sh index c71ff43b..2cb31227 100644 --- a/.github/scripts/build-cpp-runtime-bindings.sh +++ b/.github/scripts/build-cpp-runtime-bindings.sh @@ -48,7 +48,12 @@ if [ -n "$SVS_URL" ]; then fi # Build and install runtime bindings library (from bindings/cpp) -CC=gcc CXX=g++ cmake .. "${CMAKE_ARGS[@]}" +if [ -n "$CC" ]; then + echo "Using CC=${CC} and CXX=${CXX} for building cpp runtime bindings" +else + echo "Using default compiler for building cpp runtime bindings" +fi +CC=${CC:-gcc} CXX=${CXX:-g++} cmake .. "${CMAKE_ARGS[@]}" cmake --build . -j cmake --install . diff --git a/.github/workflows/build-cpp-runtime-bindings.yml b/.github/workflows/build-cpp-runtime-bindings.yml index 468789bf..351a47d2 100644 --- a/.github/workflows/build-cpp-runtime-bindings.yml +++ b/.github/workflows/build-cpp-runtime-bindings.yml @@ -39,9 +39,18 @@ jobs: - name: "with static library" enable_lvq_leanvec: "ON" suffix: "" + cc: "gcc" + cxx: "g++" - name: "public only" enable_lvq_leanvec: "OFF" suffix: "-public-only" + cc: "gcc" + cxx: "g++" + - name: public only with Clang + enable_lvq_leanvec: "OFF" + suffix: "-public-only-clang" + cc: "clang" + cxx: "clang++" fail-fast: false steps: @@ -58,6 +67,8 @@ jobs: -w /workspace \ -e ENABLE_LVQ_LEANVEC=${{ matrix.enable_lvq_leanvec }} \ -e SUFFIX=${{ matrix.suffix }} \ + -e CC=${{ matrix.cc }} \ + -e CXX=${{ matrix.cxx }} \ svs-manylinux228:latest \ /bin/bash .github/scripts/build-cpp-runtime-bindings.sh From 8a44fa4f2527d18cb1738cabdf70e0334d0dd609 Mon Sep 17 00:00:00 2001 From: Rafik Saliev Date: Fri, 8 May 2026 14:23:38 +0200 Subject: [PATCH 03/12] Update Dockerfile to include clang installation alongside gcc-11 --- docker/x86_64/manylinux228/Dockerfile | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/docker/x86_64/manylinux228/Dockerfile b/docker/x86_64/manylinux228/Dockerfile index 0818982e..02d237d9 100644 --- a/docker/x86_64/manylinux228/Dockerfile +++ b/docker/x86_64/manylinux228/Dockerfile @@ -20,8 +20,8 @@ RUN yum install -y cmake-3.26.5 \ && (if [ -x /usr/local/bin/cmake ]; then rm /usr/local/bin/cmake; fi) \ && ln -sf /usr/bin/cmake3 /usr/local/bin/cmake -# Install gcc-11 -RUN yum install -y gcc-toolset-11-11.1-1.el8 wget-1.19.5 \ +# Install gcc-11 and clang +RUN yum install -y gcc-toolset-11-11.1-1.el8 wget-1.19.5 clang \ && yum clean all # Configure environment setup properly without recursion From 570b0758f1b0e8460f1ff4f79bea6ed2b49e4666 Mon Sep 17 00:00:00 2001 From: Rafik Saliev Date: Fri, 8 May 2026 14:36:08 +0200 Subject: [PATCH 04/12] Refactor namespace declarations to use SVS_DECLARE_NAMESPACE_VERSION for versioning consistency --- .clang-format | 1 + bindings/cpp/include/svs/runtime/api_defs.h | 6 ++-- .../include/svs/runtime/dynamic_ivf_index.h | 6 ++-- .../svs/runtime/dynamic_vamana_index.h | 6 ++-- bindings/cpp/include/svs/runtime/flat_index.h | 6 ++-- bindings/cpp/include/svs/runtime/ivf_index.h | 6 ++-- bindings/cpp/include/svs/runtime/training.h | 6 ++-- .../cpp/include/svs/runtime/vamana_index.h | 6 ++-- bindings/cpp/include/svs/runtime/version.h | 32 ++++++++++--------- 9 files changed, 32 insertions(+), 43 deletions(-) diff --git a/.clang-format b/.clang-format index 250b8e12..b9c31482 100644 --- a/.clang-format +++ b/.clang-format @@ -29,4 +29,5 @@ KeepEmptyLinesAtTheStartOfBlocks: false PointerAlignment: Left ReferenceAlignment: Left ReflowComments: true +NamespaceMacros: ['SVS_DECLARE_NAMESPACE_VERSION'] --- diff --git a/bindings/cpp/include/svs/runtime/api_defs.h b/bindings/cpp/include/svs/runtime/api_defs.h index f77df9d7..504df76c 100644 --- a/bindings/cpp/include/svs/runtime/api_defs.h +++ b/bindings/cpp/include/svs/runtime/api_defs.h @@ -32,8 +32,7 @@ namespace svs { namespace runtime { -namespace v0 { - +SVS_DECLARE_NAMESPACE_VERSION(0) { class OptionalBool { enum class Value : int8_t { Undef = -1, True = 1, False = 0 }; Value value_; @@ -206,7 +205,6 @@ struct SVS_RUNTIME_API_INTERFACE ResultsAllocator { return this->allocate(result_counts); } }; - -} // namespace v0 +} // SVS_DECLARE_NAMESPACE_VERSION(0) } // namespace runtime } // namespace svs diff --git a/bindings/cpp/include/svs/runtime/dynamic_ivf_index.h b/bindings/cpp/include/svs/runtime/dynamic_ivf_index.h index 39eda2af..ebc35f75 100644 --- a/bindings/cpp/include/svs/runtime/dynamic_ivf_index.h +++ b/bindings/cpp/include/svs/runtime/dynamic_ivf_index.h @@ -24,8 +24,7 @@ namespace svs { namespace runtime { -namespace v0 { - +SVS_DECLARE_NAMESPACE_VERSION(0) { /// @brief Abstract interface for dynamic IVF indices (supports add/delete). struct SVS_RUNTIME_API DynamicIVFIndex : public IVFIndex { /// @brief Utility function to check storage kind support. @@ -160,7 +159,6 @@ struct SVS_RUNTIME_API DynamicIVFIndexLeanVec : public DynamicIVFIndex { size_t intra_query_threads = 1 ) noexcept; }; - -} // namespace v0 +} // SVS_DECLARE_NAMESPACE_VERSION(0) } // namespace runtime } // namespace svs diff --git a/bindings/cpp/include/svs/runtime/dynamic_vamana_index.h b/bindings/cpp/include/svs/runtime/dynamic_vamana_index.h index 1d487baf..60d5412b 100644 --- a/bindings/cpp/include/svs/runtime/dynamic_vamana_index.h +++ b/bindings/cpp/include/svs/runtime/dynamic_vamana_index.h @@ -25,8 +25,7 @@ namespace svs { namespace runtime { -namespace v0 { - +SVS_DECLARE_NAMESPACE_VERSION(0) { // Abstract interface for Dynamic Vamana-based indexes. struct SVS_RUNTIME_API DynamicVamanaIndex : public VamanaIndex { virtual Status add(size_t n, const size_t* labels, const float* x) noexcept = 0; @@ -131,7 +130,6 @@ struct SVS_RUNTIME_API DynamicVamanaIndexLeanVec : public DynamicVamanaIndex { const VamanaIndex::DynamicIndexParams& dynamic_index_params ) noexcept; }; - -} // namespace v0 +} // SVS_DECLARE_NAMESPACE_VERSION(0) } // namespace runtime } // namespace svs diff --git a/bindings/cpp/include/svs/runtime/flat_index.h b/bindings/cpp/include/svs/runtime/flat_index.h index 9c635e35..c6bb947d 100644 --- a/bindings/cpp/include/svs/runtime/flat_index.h +++ b/bindings/cpp/include/svs/runtime/flat_index.h @@ -23,8 +23,7 @@ namespace svs { namespace runtime { -namespace v0 { - +SVS_DECLARE_NAMESPACE_VERSION(0) { // Abstract interface for Flat indices. struct SVS_RUNTIME_API FlatIndex { // Utility function to check storage kind support @@ -45,7 +44,6 @@ struct SVS_RUNTIME_API FlatIndex { virtual Status save(std::ostream& out) const noexcept = 0; static Status load(FlatIndex** index, std::istream& in, MetricType metric) noexcept; }; - -} // namespace v0 +} // SVS_DECLARE_NAMESPACE_VERSION(0) } // namespace runtime } // namespace svs diff --git a/bindings/cpp/include/svs/runtime/ivf_index.h b/bindings/cpp/include/svs/runtime/ivf_index.h index 3bbe8d10..37bb8647 100644 --- a/bindings/cpp/include/svs/runtime/ivf_index.h +++ b/bindings/cpp/include/svs/runtime/ivf_index.h @@ -24,8 +24,7 @@ namespace svs { namespace runtime { -namespace v0 { - +SVS_DECLARE_NAMESPACE_VERSION(0) { // Abstract interface for IVF (Inverted File) indices. struct SVS_RUNTIME_API IVFIndex { virtual ~IVFIndex(); @@ -170,7 +169,6 @@ struct SVS_RUNTIME_API IVFIndexLeanVec : public IVFIndex { size_t intra_query_threads = 1 ) noexcept; }; - -} // namespace v0 +} // SVS_DECLARE_NAMESPACE_VERSION(0) } // namespace runtime } // namespace svs diff --git a/bindings/cpp/include/svs/runtime/training.h b/bindings/cpp/include/svs/runtime/training.h index da072e42..02c669dd 100644 --- a/bindings/cpp/include/svs/runtime/training.h +++ b/bindings/cpp/include/svs/runtime/training.h @@ -23,8 +23,7 @@ namespace svs { namespace runtime { -namespace v0 { - +SVS_DECLARE_NAMESPACE_VERSION(0) { struct SVS_RUNTIME_API LeanVecTrainingData { virtual ~LeanVecTrainingData(); @@ -70,7 +69,6 @@ struct SVS_RUNTIME_API LeanVecTrainingData { virtual Status save(std::ostream& out) const noexcept = 0; static Status load(LeanVecTrainingData** training_data, std::istream& in) noexcept; }; - -} // namespace v0 +} // SVS_DECLARE_NAMESPACE_VERSION(0) } // namespace runtime } // namespace svs diff --git a/bindings/cpp/include/svs/runtime/vamana_index.h b/bindings/cpp/include/svs/runtime/vamana_index.h index 2149c0d3..68001761 100644 --- a/bindings/cpp/include/svs/runtime/vamana_index.h +++ b/bindings/cpp/include/svs/runtime/vamana_index.h @@ -23,8 +23,7 @@ namespace svs { namespace runtime { -namespace v0 { - +SVS_DECLARE_NAMESPACE_VERSION(0) { namespace detail { struct VamanaBuildParameters { size_t graph_max_degree = Unspecify(); @@ -128,7 +127,6 @@ struct SVS_RUNTIME_API VamanaIndexLeanVec : public VamanaIndex { const VamanaIndex::SearchParams& default_search_params = {} ) noexcept; }; - -} // namespace v0 +} // SVS_DECLARE_NAMESPACE_VERSION(0) } // namespace runtime } // namespace svs diff --git a/bindings/cpp/include/svs/runtime/version.h b/bindings/cpp/include/svs/runtime/version.h index bdf56450..5898949a 100644 --- a/bindings/cpp/include/svs/runtime/version.h +++ b/bindings/cpp/include/svs/runtime/version.h @@ -59,21 +59,20 @@ #define SVS_RUNTIME_API_VERSION SVS_RUNTIME_VERSION_MAJOR #endif +#define SVS_API_VERSION_NS(version) v##version +#define SVS_RUNTIME_API_VERSION_NAMESPACE SVS_API_VERSION_NS(SVS_RUNTIME_API_VERSION) + #if (SVS_RUNTIME_API_VERSION == 0) -/// Use v0 API -/// Current API version namespace -#define SVS_RUNTIME_CURRENT_API_NAMESPACE v0 -namespace svs { -namespace runtime { /// Current API version namespace (v0) /// All public runtime APIs live here and are accessible as svs::runtime::FunctionName /// due to inline namespace -inline namespace v0 { -// Public runtime APIs will be defined in their respective headers -// IMPORTANT: include this header before other runtime headers to ensure proper versioning -} -} // namespace runtime -} // namespace svs +#define SVS_DECLARE_NAMESPACE_VERSION_0 inline namespace SVS_API_VERSION_NS(0) + +// Forward declaration for the current version namespace as inline namespace +// This enforces compilation warnings/errors if the v0 namespace is not inlined below +namespace svs::runtime { +inline namespace v0 {} +} // namespace svs::runtime #else #error "Unsupported SVS Runtime major version" #endif @@ -86,11 +85,14 @@ inline namespace v0 { #define SVS_RUNTIME_CREATE_API_ALIAS(alias_name, version_ns) \ namespace alias_name = svs::runtime::version_ns +/// Helper macro to declare versioned namespaces for API definitions +#define SVS_DECLARE_NAMESPACE_VERSION(version) SVS_DECLARE_NAMESPACE_VERSION_##version + /// /// @brief Version information structure for runtime queries /// -namespace svs::runtime::v0 { - +namespace svs::runtime { +SVS_DECLARE_NAMESPACE_VERSION(0) { struct VersionInfo { static constexpr int major = SVS_RUNTIME_VERSION_MAJOR; static constexpr int minor = SVS_RUNTIME_VERSION_MINOR; @@ -109,5 +111,5 @@ struct VersionInfo { return major == requested_major; } }; - -} // namespace svs::runtime::v0 +} // SVS_DECLARE_NAMESPACE_VERSION(0) +} // namespace svs::runtime From 4514a0d28b6cd13556d174638e571b8f27f54981 Mon Sep 17 00:00:00 2001 From: Rafik Saliev Date: Fri, 8 May 2026 14:44:24 +0200 Subject: [PATCH 05/12] Fix Clang compilation errors --- bindings/cpp/src/dynamic_vamana_index.cpp | 2 +- bindings/cpp/src/dynamic_vamana_index_impl.h | 2 ++ bindings/cpp/src/vamana_index_impl.h | 2 ++ 3 files changed, 5 insertions(+), 1 deletion(-) diff --git a/bindings/cpp/src/dynamic_vamana_index.cpp b/bindings/cpp/src/dynamic_vamana_index.cpp index 0c1a6a89..e2399b5d 100644 --- a/bindings/cpp/src/dynamic_vamana_index.cpp +++ b/bindings/cpp/src/dynamic_vamana_index.cpp @@ -63,7 +63,7 @@ struct DynamicVamanaIndexManagerBase : public DynamicVamanaIndex { }); } - size_t blocksize_bytes() const noexcept { return impl_->blocksize_bytes(); } + size_t blocksize_bytes() const noexcept override { return impl_->blocksize_bytes(); } Status remove_selected(size_t* num_removed, const IDFilter& selector) noexcept override { diff --git a/bindings/cpp/src/dynamic_vamana_index_impl.h b/bindings/cpp/src/dynamic_vamana_index_impl.h index 516a1653..3cc0a611 100644 --- a/bindings/cpp/src/dynamic_vamana_index_impl.h +++ b/bindings/cpp/src/dynamic_vamana_index_impl.h @@ -65,6 +65,8 @@ class DynamicVamanaIndexImpl { } } + virtual ~DynamicVamanaIndexImpl() = default; + size_t size() const { return impl_ ? impl_->size() : 0; } size_t blocksize_bytes() const { return 1u << dynamic_index_params_.blocksize_exp; } diff --git a/bindings/cpp/src/vamana_index_impl.h b/bindings/cpp/src/vamana_index_impl.h index 550257d4..44e44075 100644 --- a/bindings/cpp/src/vamana_index_impl.h +++ b/bindings/cpp/src/vamana_index_impl.h @@ -69,6 +69,8 @@ class VamanaIndexImpl { } } + virtual ~VamanaIndexImpl() = default; + size_t size() const { return impl_ ? get_impl()->size() : 0; } size_t dimensions() const { return dim_; } From f87a7443dd70d9a761e15616d755379eeaa6f0e8 Mon Sep 17 00:00:00 2001 From: Rafik Saliev Date: Fri, 8 May 2026 14:57:16 +0200 Subject: [PATCH 06/12] More fixes. --- bindings/cpp/include/svs/runtime/dynamic_vamana_index.h | 3 --- docker/x86_64/manylinux228/Dockerfile | 2 +- 2 files changed, 1 insertion(+), 4 deletions(-) diff --git a/bindings/cpp/include/svs/runtime/dynamic_vamana_index.h b/bindings/cpp/include/svs/runtime/dynamic_vamana_index.h index 60d5412b..fb588f6b 100644 --- a/bindings/cpp/include/svs/runtime/dynamic_vamana_index.h +++ b/bindings/cpp/include/svs/runtime/dynamic_vamana_index.h @@ -33,8 +33,6 @@ struct SVS_RUNTIME_API DynamicVamanaIndex : public VamanaIndex { remove_selected(size_t* num_removed, const IDFilter& selector) noexcept = 0; virtual Status remove(size_t n, const size_t* labels) noexcept = 0; - virtual Status reset() noexcept = 0; - // Utility function to check storage kind support static Status check_storage_kind(StorageKind storage_kind) noexcept; @@ -64,7 +62,6 @@ struct SVS_RUNTIME_API DynamicVamanaIndex : public VamanaIndex { static Status destroy(DynamicVamanaIndex* index) noexcept; - virtual Status save(std::ostream& out) const noexcept = 0; static Status load( DynamicVamanaIndex** index, std::istream& in, diff --git a/docker/x86_64/manylinux228/Dockerfile b/docker/x86_64/manylinux228/Dockerfile index 02d237d9..708f3aec 100644 --- a/docker/x86_64/manylinux228/Dockerfile +++ b/docker/x86_64/manylinux228/Dockerfile @@ -21,7 +21,7 @@ RUN yum install -y cmake-3.26.5 \ && ln -sf /usr/bin/cmake3 /usr/local/bin/cmake # Install gcc-11 and clang -RUN yum install -y gcc-toolset-11-11.1-1.el8 wget-1.19.5 clang \ +RUN yum install -y gcc-toolset-11-11.1-1.el8 wget-1.19.5 clang-20.1.8 \ && yum clean all # Configure environment setup properly without recursion From 322d1cad90a6f7be30f585075fb160f71f084644 Mon Sep 17 00:00:00 2001 From: Rafik Saliev Date: Fri, 8 May 2026 15:24:40 +0200 Subject: [PATCH 07/12] Fix unused exeption parameter warning Co-Authored-By: mnorris11 --- bindings/cpp/CMakeLists.txt | 5 ++++- include/svs/lib/threads/thread.h | 2 +- 2 files changed, 5 insertions(+), 2 deletions(-) diff --git a/bindings/cpp/CMakeLists.txt b/bindings/cpp/CMakeLists.txt index 6473d3e6..569fda7b 100644 --- a/bindings/cpp/CMakeLists.txt +++ b/bindings/cpp/CMakeLists.txt @@ -83,7 +83,10 @@ target_compile_options(${TARGET_NAME} PRIVATE ) if(CMAKE_CXX_COMPILER_ID STREQUAL "Clang") - target_compile_options(${TARGET_NAME} PRIVATE -Winline-namespace-reopened-noninline) + target_compile_options(${TARGET_NAME} PRIVATE + -Winline-namespace-reopened-noninline + -Wunused-exception-parameter + ) endif() if(UNIX AND NOT APPLE) diff --git a/include/svs/lib/threads/thread.h b/include/svs/lib/threads/thread.h index 5a5776ce..6de938d3 100644 --- a/include/svs/lib/threads/thread.h +++ b/include/svs/lib/threads/thread.h @@ -888,7 +888,7 @@ template class ThreadImpl { // * Catch this exception and wrap its message inside a `ThreadError`. try { unsafe_assign(fn); - } catch (const ThreadCrashedError& err) { + } catch (const ThreadCrashedError&) { try { unsafe_get_exception(); } catch (const std::exception& inner_error) { throw ThreadError{inner_error}; } From c11ab124050d84d1696537f28754f24a2f3d7e5e Mon Sep 17 00:00:00 2001 From: Rafik Saliev Date: Fri, 8 May 2026 16:40:53 +0200 Subject: [PATCH 08/12] Address code review comments --- bindings/cpp/CMakeLists.txt | 22 +++++++++++++++++----- bindings/cpp/include/svs/runtime/version.h | 8 +++++--- 2 files changed, 22 insertions(+), 8 deletions(-) diff --git a/bindings/cpp/CMakeLists.txt b/bindings/cpp/CMakeLists.txt index 569fda7b..360fa91d 100644 --- a/bindings/cpp/CMakeLists.txt +++ b/bindings/cpp/CMakeLists.txt @@ -82,11 +82,23 @@ target_compile_options(${TARGET_NAME} PRIVATE -fvisibility=hidden ) -if(CMAKE_CXX_COMPILER_ID STREQUAL "Clang") - target_compile_options(${TARGET_NAME} PRIVATE - -Winline-namespace-reopened-noninline - -Wunused-exception-parameter - ) +# Add extra warnings for GCC/Clang if supported +include(CheckCXXCompilerFlag) + +check_cxx_compiler_flag( + "-Winline-namespace-reopened-noninline" + SVS_RUNTIME_HAS_WINLINE_NAMESPACE_REOPENED_NONINLINE +) +if(SVS_RUNTIME_HAS_WINLINE_NAMESPACE_REOPENED_NONINLINE) + target_compile_options(${TARGET_NAME} PRIVATE -Winline-namespace-reopened-noninline) +endif() + +check_cxx_compiler_flag( + "-Wunused-exception-parameter" + SVS_RUNTIME_HAS_WUNUSED_EXCEPTION_PARAMETER +) +if(SVS_RUNTIME_HAS_WUNUSED_EXCEPTION_PARAMETER) + target_compile_options(${TARGET_NAME} PRIVATE -Wunused-exception-parameter) endif() if(UNIX AND NOT APPLE) diff --git a/bindings/cpp/include/svs/runtime/version.h b/bindings/cpp/include/svs/runtime/version.h index 5898949a..97b027fc 100644 --- a/bindings/cpp/include/svs/runtime/version.h +++ b/bindings/cpp/include/svs/runtime/version.h @@ -21,7 +21,7 @@ /// /// This header defines the SVS Runtime API versioning scheme: /// 1. Versioned namespaces (e.g., v0, v1) for API stability -/// 2. Using declarations to bring current version to parent namespace +/// 2. Inline namespace to expose the current API version at svs::runtime::* /// 3. Clean integration points for external libraries /// /// Usage: @@ -59,7 +59,8 @@ #define SVS_RUNTIME_API_VERSION SVS_RUNTIME_VERSION_MAJOR #endif -#define SVS_API_VERSION_NS(version) v##version +#define SVS_API_VERSION_NS_IMPL(version) v##version +#define SVS_API_VERSION_NS(version) SVS_API_VERSION_NS_IMPL(version) #define SVS_RUNTIME_API_VERSION_NAMESPACE SVS_API_VERSION_NS(SVS_RUNTIME_API_VERSION) #if (SVS_RUNTIME_API_VERSION == 0) @@ -86,7 +87,8 @@ inline namespace v0 {} namespace alias_name = svs::runtime::version_ns /// Helper macro to declare versioned namespaces for API definitions -#define SVS_DECLARE_NAMESPACE_VERSION(version) SVS_DECLARE_NAMESPACE_VERSION_##version +#define SVS_DECLARE_NAMESPACE_VERSION_IMPL(version) SVS_DECLARE_NAMESPACE_VERSION_##version +#define SVS_DECLARE_NAMESPACE_VERSION(version) SVS_DECLARE_NAMESPACE_VERSION_IMPL(version) /// /// @brief Version information structure for runtime queries From f85cf2a03aa88aca31a72c00553c14d3b1b57b08 Mon Sep 17 00:00:00 2001 From: ethanglaser Date: Mon, 18 May 2026 12:41:00 -0700 Subject: [PATCH 09/12] drop clang for clang-tidy --- .clang-tidy | 71 ++++++++++++++++--- .github/scripts/build-cpp-runtime-bindings.sh | 8 +-- .../workflows/build-cpp-runtime-bindings.yml | 11 --- bindings/cpp/CMakeLists.txt | 30 ++++---- cmake/clang-tidy.cmake | 26 +++++-- docker/x86_64/manylinux228/Dockerfile | 4 +- 6 files changed, 101 insertions(+), 49 deletions(-) diff --git a/.clang-tidy b/.clang-tidy index 2cbeea94..278e839a 100644 --- a/.clang-tidy +++ b/.clang-tidy @@ -1,12 +1,63 @@ -Checks: "-*, - bugprone*, - -bugprone-easily-swappable-parameters, - -bugprone-macro-parentheses, - clang-analyzer*, - readability*, - -readability-magic-numbers, - -readability-identifier-length" +Checks: ' +-*, +bugprone-*, +-bugprone-easily-swappable-parameters, +-bugprone-macro-parentheses, +clang-analyzer-*, +clang-diagnostic-*, +-clang-diagnostic-nrvo, +-clang-diagnostic-missing-designated-field-initializers, +cppcoreguidelines-pro-type-member-init, +cppcoreguidelines-special-member-functions, +google-build-using-namespace, +misc-definitions-in-headers, +modernize-use-emplace, +modernize-use-using, +performance-faster-string-find, +performance-for-range-copy, +performance-implicit-conversion-in-loop, +performance-inefficient-algorithm, +performance-inefficient-string-concatenation, +performance-inefficient-vector-operation, +performance-move-const-arg, +performance-move-constructor-init, +performance-no-automatic-move, +performance-no-int-to-ptr, +performance-noexcept-move-constructor, +performance-trivially-destructible, +performance-type-promotion-in-math-fn, +performance-unnecessary-copy-initialization, +performance-unnecessary-value-param, +readability-*, +-readability-magic-numbers, +-readability-identifier-length, +' + +WarningsAsErrors: ' +bugprone-use-after-move, +' CheckOptions: - - key: readability-implicit-bool-conversion.AllowIntegerConditions - value: '1' +- key: bugprone-easily-swappable-parameters.MinimumLength + value: 4 +- key: cppcoreguidelines-special-member-functions.AllowSoleDefaultDtor + value: true +- key: cppcoreguidelines-special-member-functions.AllowImplicitlyDeletedCopyOrMove + value: 1 +- key: modernize-use-using.IgnoreExternC + value: true +- key: performance-move-const-arg.CheckTriviallyCopyableMove + value: false +- key: performance-unnecessary-value-param.AllowedTypes + value: '[Pp]ointer$;[Pp]tr$;[Rr]ef(erence)?$' +- key: performance-unnecessary-copy-initialization.AllowedTypes + value: '[Pp]ointer$;[Pp]tr$;[Rr]ef(erence)?$' +- key: readability-operators-representation.BinaryOperators + value: '&&;&=;&;|;~;!;!=;||;|=;^;^=' +- key: readability-redundant-string-init.StringNames + value: '::std::basic_string' +- key: readability-named-parameter.InsertPlainNamesInForwardDecls + value: true +- key: readability-implicit-bool-conversion.AllowIntegerConditions + value: '1' +... diff --git a/.github/scripts/build-cpp-runtime-bindings.sh b/.github/scripts/build-cpp-runtime-bindings.sh index 2cb31227..74ab7990 100644 --- a/.github/scripts/build-cpp-runtime-bindings.sh +++ b/.github/scripts/build-cpp-runtime-bindings.sh @@ -41,6 +41,7 @@ CMAKE_ARGS=( "-DCMAKE_INSTALL_LIBDIR=lib" "-DSVS_RUNTIME_ENABLE_LVQ_LEANVEC=${ENABLE_LVQ_LEANVEC:-ON}" "-DSVS_RUNTIME_ENABLE_IVF=ON" + "-DSVS_EXPERIMENTAL_CLANG_TIDY=ON" ) if [ -n "$SVS_URL" ]; then @@ -48,12 +49,7 @@ if [ -n "$SVS_URL" ]; then fi # Build and install runtime bindings library (from bindings/cpp) -if [ -n "$CC" ]; then - echo "Using CC=${CC} and CXX=${CXX} for building cpp runtime bindings" -else - echo "Using default compiler for building cpp runtime bindings" -fi -CC=${CC:-gcc} CXX=${CXX:-g++} cmake .. "${CMAKE_ARGS[@]}" +CC=gcc CXX=g++ cmake .. "${CMAKE_ARGS[@]}" cmake --build . -j cmake --install . diff --git a/.github/workflows/build-cpp-runtime-bindings.yml b/.github/workflows/build-cpp-runtime-bindings.yml index 351a47d2..468789bf 100644 --- a/.github/workflows/build-cpp-runtime-bindings.yml +++ b/.github/workflows/build-cpp-runtime-bindings.yml @@ -39,18 +39,9 @@ jobs: - name: "with static library" enable_lvq_leanvec: "ON" suffix: "" - cc: "gcc" - cxx: "g++" - name: "public only" enable_lvq_leanvec: "OFF" suffix: "-public-only" - cc: "gcc" - cxx: "g++" - - name: public only with Clang - enable_lvq_leanvec: "OFF" - suffix: "-public-only-clang" - cc: "clang" - cxx: "clang++" fail-fast: false steps: @@ -67,8 +58,6 @@ jobs: -w /workspace \ -e ENABLE_LVQ_LEANVEC=${{ matrix.enable_lvq_leanvec }} \ -e SUFFIX=${{ matrix.suffix }} \ - -e CC=${{ matrix.cc }} \ - -e CXX=${{ matrix.cxx }} \ svs-manylinux228:latest \ /bin/bash .github/scripts/build-cpp-runtime-bindings.sh diff --git a/bindings/cpp/CMakeLists.txt b/bindings/cpp/CMakeLists.txt index 360fa91d..c8b2dd58 100644 --- a/bindings/cpp/CMakeLists.txt +++ b/bindings/cpp/CMakeLists.txt @@ -82,23 +82,21 @@ target_compile_options(${TARGET_NAME} PRIVATE -fvisibility=hidden ) -# Add extra warnings for GCC/Clang if supported -include(CheckCXXCompilerFlag) - -check_cxx_compiler_flag( - "-Winline-namespace-reopened-noninline" - SVS_RUNTIME_HAS_WINLINE_NAMESPACE_REOPENED_NONINLINE -) -if(SVS_RUNTIME_HAS_WINLINE_NAMESPACE_REOPENED_NONINLINE) - target_compile_options(${TARGET_NAME} PRIVATE -Winline-namespace-reopened-noninline) -endif() - -check_cxx_compiler_flag( - "-Wunused-exception-parameter" - SVS_RUNTIME_HAS_WUNUSED_EXCEPTION_PARAMETER +# Optional clang-tidy integration. Reuses the top-level repo's +# cmake/clang-tidy.cmake module so the runtime build runs the same +# .clang-tidy config as the rest of the library. +option(SVS_EXPERIMENTAL_CLANG_TIDY + "Run the clang-tidy static analyzer on the cpp runtime bindings." + OFF ) -if(SVS_RUNTIME_HAS_WUNUSED_EXCEPTION_PARAMETER) - target_compile_options(${TARGET_NAME} PRIVATE -Wunused-exception-parameter) +if(SVS_EXPERIMENTAL_CLANG_TIDY) + list(APPEND CMAKE_MODULE_PATH "${CMAKE_CURRENT_LIST_DIR}/../../cmake") + include(clang-tidy) + if(CLANG_TIDY_COMMAND) + set_target_properties(${TARGET_NAME} + PROPERTIES CXX_CLANG_TIDY "${CLANG_TIDY_COMMAND}" + ) + endif() endif() if(UNIX AND NOT APPLE) diff --git a/cmake/clang-tidy.cmake b/cmake/clang-tidy.cmake index 167f5b7c..5f770d2e 100644 --- a/cmake/clang-tidy.cmake +++ b/cmake/clang-tidy.cmake @@ -17,18 +17,36 @@ ##### if(SVS_EXPERIMENTAL_CLANG_TIDY) - find_program(CLANG_TIDY_EXE NAMES clang-tidy-14 clang-tidy-13 clang-tidy-12 clang-tidy) + find_program(CLANG_TIDY_EXE + NAMES + clang-tidy-20 clang-tidy-19 clang-tidy-18 clang-tidy-17 clang-tidy-16 + clang-tidy-15 clang-tidy-14 clang-tidy-13 clang-tidy-12 clang-tidy + ) if(NOT CLANG_TIDY_EXE) message(WARNING "SVS_EXPERIMENTAL_CLANG_TIDY is ON but clang-tidy is not found!") set(CLANG_TIDY_COMMAND "" CACHE STRING "" FORCE) else() + # Resolve the .clang-tidy config from the top-level repo root. When the + # runtime bindings are built standalone (bindings/cpp as source root), + # CMAKE_SOURCE_DIR points at bindings/cpp, so walk up two directories. + if(EXISTS "${CMAKE_SOURCE_DIR}/.clang-tidy") + set(SVS_CLANG_TIDY_CONFIG "${CMAKE_SOURCE_DIR}/.clang-tidy") + set(SVS_CLANG_TIDY_HEADER_FILTER "${CMAKE_SOURCE_DIR}/include/svs/.*") + elseif(EXISTS "${CMAKE_SOURCE_DIR}/../../.clang-tidy") + set(SVS_CLANG_TIDY_CONFIG "${CMAKE_SOURCE_DIR}/../../.clang-tidy") + set(SVS_CLANG_TIDY_HEADER_FILTER "${CMAKE_SOURCE_DIR}/include/svs/.*") + else() + message(FATAL_ERROR "SVS_EXPERIMENTAL_CLANG_TIDY is ON but no .clang-tidy was found") + endif() + set(CLANG_TIDY_COMMAND "${CLANG_TIDY_EXE}" "--format-style=file" - "--config-file=${CMAKE_SOURCE_DIR}/.clang-tidy" - "--header-filter=${CMAKE_SOURCE_DIR}/include/svs/*" + "--config-file=${SVS_CLANG_TIDY_CONFIG}" + "--header-filter=${SVS_CLANG_TIDY_HEADER_FILTER}" + "--warnings-as-errors=*" ) - message("Clang tidy command: ${CLANG_TIDY_COMMAND}") + message(STATUS "Clang tidy command: ${CLANG_TIDY_COMMAND}") endif() endif() diff --git a/docker/x86_64/manylinux228/Dockerfile b/docker/x86_64/manylinux228/Dockerfile index 708f3aec..d8ee79d1 100644 --- a/docker/x86_64/manylinux228/Dockerfile +++ b/docker/x86_64/manylinux228/Dockerfile @@ -20,8 +20,8 @@ RUN yum install -y cmake-3.26.5 \ && (if [ -x /usr/local/bin/cmake ]; then rm /usr/local/bin/cmake; fi) \ && ln -sf /usr/bin/cmake3 /usr/local/bin/cmake -# Install gcc-11 and clang -RUN yum install -y gcc-toolset-11-11.1-1.el8 wget-1.19.5 clang-20.1.8 \ +# Install gcc-11 and clang-tools (for clang-tidy) +RUN yum install -y gcc-toolset-11-11.1-1.el8 wget-1.19.5 clang-tools-extra \ && yum clean all # Configure environment setup properly without recursion From 0d70ac671424d99afa023ef028fd512b8daa1c45 Mon Sep 17 00:00:00 2001 From: ethanglaser Date: Mon, 18 May 2026 16:35:42 -0700 Subject: [PATCH 10/12] address gnu/clang-tidy incompatibilities --- bindings/cpp/CMakeLists.txt | 13 +++++++++++++ cmake/clang-tidy.cmake | 14 +++++++++++--- cmake/options.cmake | 19 ++++++++++++------- 3 files changed, 36 insertions(+), 10 deletions(-) diff --git a/bindings/cpp/CMakeLists.txt b/bindings/cpp/CMakeLists.txt index c8b2dd58..db6053f9 100644 --- a/bindings/cpp/CMakeLists.txt +++ b/bindings/cpp/CMakeLists.txt @@ -159,6 +159,19 @@ if (SVS_RUNTIME_ENABLE_LVQ_LEANVEC) FetchContent_MakeAvailable(svs) list(APPEND CMAKE_PREFIX_PATH "${svs_SOURCE_DIR}") find_package(svs REQUIRED) + + # Strip svs::svs_compile_options gcc-only flags for clang-tidy + if(SVS_EXPERIMENTAL_CLANG_TIDY AND TARGET svs::svs_compile_options) + get_target_property(_svs_co_opts svs::svs_compile_options INTERFACE_COMPILE_OPTIONS) + if(_svs_co_opts) + list(FILTER _svs_co_opts EXCLUDE REGEX "^-fconcepts-diagnostics-depth") + list(FILTER _svs_co_opts EXCLUDE REGEX "^-ftemplate-backtrace-limit") + set_property(TARGET svs::svs_compile_options + PROPERTY INTERFACE_COMPILE_OPTIONS "${_svs_co_opts}" + ) + endif() + endif() + target_link_libraries(${TARGET_NAME} PRIVATE svs::svs svs::svs_compile_options diff --git a/cmake/clang-tidy.cmake b/cmake/clang-tidy.cmake index 5f770d2e..ab8d8001 100644 --- a/cmake/clang-tidy.cmake +++ b/cmake/clang-tidy.cmake @@ -27,9 +27,7 @@ if(SVS_EXPERIMENTAL_CLANG_TIDY) message(WARNING "SVS_EXPERIMENTAL_CLANG_TIDY is ON but clang-tidy is not found!") set(CLANG_TIDY_COMMAND "" CACHE STRING "" FORCE) else() - # Resolve the .clang-tidy config from the top-level repo root. When the - # runtime bindings are built standalone (bindings/cpp as source root), - # CMAKE_SOURCE_DIR points at bindings/cpp, so walk up two directories. + # Walk up to find .clang-tidy when built standalone from bindings/cpp. if(EXISTS "${CMAKE_SOURCE_DIR}/.clang-tidy") set(SVS_CLANG_TIDY_CONFIG "${CMAKE_SOURCE_DIR}/.clang-tidy") set(SVS_CLANG_TIDY_HEADER_FILTER "${CMAKE_SOURCE_DIR}/include/svs/.*") @@ -47,6 +45,16 @@ if(SVS_EXPERIMENTAL_CLANG_TIDY) "--header-filter=${SVS_CLANG_TIDY_HEADER_FILTER}" "--warnings-as-errors=*" ) + + # Point clang-tidy at gcc's toolchain so it can find libstdc++ headers. + if(CMAKE_CXX_COMPILER_ID STREQUAL "GNU") + get_filename_component(_svs_gcc_bin_dir "${CMAKE_CXX_COMPILER}" DIRECTORY) + get_filename_component(_svs_gcc_toolchain "${_svs_gcc_bin_dir}" DIRECTORY) + list(APPEND CLANG_TIDY_COMMAND + "--extra-arg=--gcc-toolchain=${_svs_gcc_toolchain}" + ) + endif() + message(STATUS "Clang tidy command: ${CLANG_TIDY_COMMAND}") endif() endif() diff --git a/cmake/options.cmake b/cmake/options.cmake index d83b179a..b02b6785 100644 --- a/cmake/options.cmake +++ b/cmake/options.cmake @@ -201,14 +201,19 @@ if (CMAKE_CXX_COMPILER_ID STREQUAL "Clang" OR CMAKE_CXX_COMPILER_ID STREQUAL "In endif() endif() -# Provide better diagnostics for broken templates. if (CMAKE_CXX_COMPILER_ID STREQUAL "GNU") - target_compile_options( - svs_compile_options - INTERFACE - -fconcepts-diagnostics-depth=10 - -ftemplate-backtrace-limit=0 - ) + # Provide better diagnostics for broken templates. + # These flags are gcc-only; clang-tidy uses the clang frontend to parse the + # compile command and would error with "unknown argument", so skip them when + # clang-tidy is enabled. + if (NOT SVS_EXPERIMENTAL_CLANG_TIDY) + target_compile_options( + svs_compile_options + INTERFACE + -fconcepts-diagnostics-depth=10 + -ftemplate-backtrace-limit=0 + ) + endif() if (CMAKE_CXX_COMPILER_VERSION GREATER_EQUAL 12.0) # GCC-12 throws errors in its own intrinsics library with uninitialized variables. From dec2c13cd279fa0b25ee30ca441eb1185e468c69 Mon Sep 17 00:00:00 2001 From: ethanglaser Date: Mon, 18 May 2026 17:02:50 -0700 Subject: [PATCH 11/12] further faiss alignment --- .clang-tidy | 15 ++++++--------- cmake/clang-tidy.cmake | 5 ++--- docker/x86_64/manylinux228/Dockerfile | 9 +++++++-- 3 files changed, 15 insertions(+), 14 deletions(-) diff --git a/.clang-tidy b/.clang-tidy index 278e839a..5b53f3ac 100644 --- a/.clang-tidy +++ b/.clang-tidy @@ -1,9 +1,8 @@ Checks: ' -*, -bugprone-*, --bugprone-easily-swappable-parameters, --bugprone-macro-parentheses, -clang-analyzer-*, +bugprone-argument-comment, +bugprone-sizeof-*, +bugprone-use-after-move, clang-diagnostic-*, -clang-diagnostic-nrvo, -clang-diagnostic-missing-designated-field-initializers, @@ -28,9 +27,9 @@ performance-trivially-destructible, performance-type-promotion-in-math-fn, performance-unnecessary-copy-initialization, performance-unnecessary-value-param, -readability-*, --readability-magic-numbers, --readability-identifier-length, +readability-operators-representation, +readability-redundant-string-init, +readability-braces-around-statements, ' WarningsAsErrors: ' @@ -58,6 +57,4 @@ CheckOptions: value: '::std::basic_string' - key: readability-named-parameter.InsertPlainNamesInForwardDecls value: true -- key: readability-implicit-bool-conversion.AllowIntegerConditions - value: '1' ... diff --git a/cmake/clang-tidy.cmake b/cmake/clang-tidy.cmake index ab8d8001..00cafa2f 100644 --- a/cmake/clang-tidy.cmake +++ b/cmake/clang-tidy.cmake @@ -19,8 +19,7 @@ if(SVS_EXPERIMENTAL_CLANG_TIDY) find_program(CLANG_TIDY_EXE NAMES - clang-tidy-20 clang-tidy-19 clang-tidy-18 clang-tidy-17 clang-tidy-16 - clang-tidy-15 clang-tidy-14 clang-tidy-13 clang-tidy-12 clang-tidy + clang-tidy-17 clang-tidy-18 clang-tidy ) if(NOT CLANG_TIDY_EXE) @@ -43,7 +42,7 @@ if(SVS_EXPERIMENTAL_CLANG_TIDY) "--format-style=file" "--config-file=${SVS_CLANG_TIDY_CONFIG}" "--header-filter=${SVS_CLANG_TIDY_HEADER_FILTER}" - "--warnings-as-errors=*" + "--warnings-as-errors=clang-diagnostic-*,bugprone-use-after-move" ) # Point clang-tidy at gcc's toolchain so it can find libstdc++ headers. diff --git a/docker/x86_64/manylinux228/Dockerfile b/docker/x86_64/manylinux228/Dockerfile index d8ee79d1..28881fd7 100644 --- a/docker/x86_64/manylinux228/Dockerfile +++ b/docker/x86_64/manylinux228/Dockerfile @@ -20,8 +20,13 @@ RUN yum install -y cmake-3.26.5 \ && (if [ -x /usr/local/bin/cmake ]; then rm /usr/local/bin/cmake; fi) \ && ln -sf /usr/bin/cmake3 /usr/local/bin/cmake -# Install gcc-11 and clang-tools (for clang-tidy) -RUN yum install -y gcc-toolset-11-11.1-1.el8 wget-1.19.5 clang-tools-extra \ +# Install gcc-11 +RUN yum install -y gcc-toolset-11-11.1-1.el8 wget-1.19.5 \ + && yum clean all + +# Install clang-tidy via the LLVM 17 module; newer versions crash on our templates. +RUN dnf module install -y llvm-toolset \ + && yum install -y clang-tools-extra \ && yum clean all # Configure environment setup properly without recursion From c1cda26f841e3309ad80e40498fc6b51430711c9 Mon Sep 17 00:00:00 2001 From: ethanglaser Date: Mon, 18 May 2026 17:36:57 -0700 Subject: [PATCH 12/12] address errors --- cmake/clang-tidy.cmake | 3 +++ docker/x86_64/manylinux228/Dockerfile | 6 +++++- 2 files changed, 8 insertions(+), 1 deletion(-) diff --git a/cmake/clang-tidy.cmake b/cmake/clang-tidy.cmake index 00cafa2f..6df315e7 100644 --- a/cmake/clang-tidy.cmake +++ b/cmake/clang-tidy.cmake @@ -43,6 +43,9 @@ if(SVS_EXPERIMENTAL_CLANG_TIDY) "--config-file=${SVS_CLANG_TIDY_CONFIG}" "--header-filter=${SVS_CLANG_TIDY_HEADER_FILTER}" "--warnings-as-errors=clang-diagnostic-*,bugprone-use-after-move" + # Suppress noise from args (e.g. --gcc-toolchain) that affect link-time + # paths but are unused during clang-tidy's parse-only invocation. + "--extra-arg=-Wno-unused-command-line-argument" ) # Point clang-tidy at gcc's toolchain so it can find libstdc++ headers. diff --git a/docker/x86_64/manylinux228/Dockerfile b/docker/x86_64/manylinux228/Dockerfile index 28881fd7..491ca196 100644 --- a/docker/x86_64/manylinux228/Dockerfile +++ b/docker/x86_64/manylinux228/Dockerfile @@ -25,7 +25,11 @@ RUN yum install -y gcc-toolset-11-11.1-1.el8 wget-1.19.5 \ && yum clean all # Install clang-tidy via the LLVM 17 module; newer versions crash on our templates. -RUN dnf module install -y llvm-toolset \ +# The base image ships LLVM 21.x by default, so remove it first to ensure the +# module's older binary wins on PATH. +RUN yum remove -y clang-tools-extra clang || true \ + && dnf module reset -y llvm-toolset \ + && dnf module install -y llvm-toolset:rhel8 \ && yum install -y clang-tools-extra \ && yum clean all