From 91f04c6b102db36a7139ff926b533ba721a4f0b7 Mon Sep 17 00:00:00 2001 From: gouyt13 Date: Fri, 9 Oct 2026 00:09:15 +0800 Subject: [PATCH 1/2] fix: isolate CMake optimization flags from consumers --- .github/workflows/test.yaml | 33 ++++++++++++ CMakeLists.txt | 15 +++--- README.md | 5 ++ python_bindings/CMakeLists.txt | 2 +- sample/cpp/CMakeLists.txt | 2 +- tests/CMakeLists.txt | 6 +-- tests/README.md | 22 ++++++++ tests/consumer/CMakeLists.txt | 33 +++++++++++- tests/consumer/check_options.cmake | 35 +++++++++++++ tests/consumer/main.cpp | 81 +++++++++++++++++------------- 10 files changed, 185 insertions(+), 49 deletions(-) create mode 100644 tests/consumer/check_options.cmake diff --git a/.github/workflows/test.yaml b/.github/workflows/test.yaml index 5d2988bb..64282272 100644 --- a/.github/workflows/test.yaml +++ b/.github/workflows/test.yaml @@ -118,6 +118,21 @@ jobs: env: RABITQ_REQUIRE_CPP_EXAMPLES: "1" run: python -m pytest tests/python/test_cpp_clustering_examples.py --tb=short -q + - name: Test native installed package isolation + run: | + cmake --install build --prefix "$PWD/rabitqlib-native-install" + cmake -S tests/consumer -B build-native-consumer -G Ninja \ + -DCMAKE_BUILD_TYPE=Release \ + -DCMAKE_PREFIX_PATH="$PWD/rabitqlib-native-install" + cmake --build build-native-consumer + ctest --test-dir build-native-consumer --output-on-failure + - name: Test add_subdirectory isolation + run: | + cmake -S tests/consumer -B build-source-consumer -G Ninja \ + -DCMAKE_BUILD_TYPE=Release -DRABITQ_SOURCE_DIR="$PWD" \ + -DRABITQ_ENABLE_NATIVE_OPTIMIZATION=ON + cmake --build build-source-consumer + ctest --test-dir build-source-consumer --output-on-failure windows: needs: changes @@ -136,6 +151,13 @@ jobs: run: cmake --build build-windows --config Release --parallel 2 - name: Test run: ctest --test-dir build-windows -C Release --output-on-failure + - name: Test installed CMake package and header requirements + run: | + cmake --install build-windows --config Release --prefix "$PWD/rabitqlib-install" + cmake -S tests/consumer -B build-consumer -G "Visual Studio 18 2026" -A x64 ` + "-DCMAKE_PREFIX_PATH=$PWD/rabitqlib-install" + cmake --build build-consumer --config Release --parallel 2 + ctest --test-dir build-consumer -C Release --output-on-failure sanitizers: needs: changes @@ -164,6 +186,17 @@ jobs: ASAN_OPTIONS: detect_leaks=1 UBSAN_OPTIONS: halt_on_error=1:print_stacktrace=1 run: ctest --test-dir build-sanitizers --output-on-failure + - name: Test installed sanitizer usage requirements + env: + ASAN_OPTIONS: detect_leaks=1 + UBSAN_OPTIONS: halt_on_error=1:print_stacktrace=1 + run: | + cmake --install build-sanitizers --prefix "$PWD/rabitqlib-sanitized-install" + cmake -S tests/consumer -B build-sanitized-consumer -G Ninja \ + -DCMAKE_PREFIX_PATH="$PWD/rabitqlib-sanitized-install" \ + -DRABITQ_EXPECT_SANITIZERS=ON + cmake --build build-sanitized-consumer + ctest --test-dir build-sanitized-consumer --output-on-failure downstream-consumer: needs: changes diff --git a/CMakeLists.txt b/CMakeLists.txt index 9dffcc8f..1cf685b3 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -26,10 +26,6 @@ find_package(OpenMP REQUIRED) add_library(rabitq_compile_options INTERFACE) target_compile_features(rabitq_compile_options INTERFACE cxx_std_17) -target_compile_options(rabitq_compile_options INTERFACE - $<$,$>:-O3> - $<$,$>:-march=native> -) target_link_libraries(rabitq_compile_options INTERFACE OpenMP::OpenMP_CXX) if(MSVC) # MSVC spells the GNU restrict extension differently. Public headers use @@ -40,16 +36,21 @@ if(MSVC) target_compile_definitions(rabitq_compile_options INTERFACE EIGEN_DONT_VECTORIZE) endif() if(RABITQ_ENABLE_SANITIZERS) + # Keep sanitizer instrumentation consistent across public inline code and + # the core (Eigen's allocation policy depends on __SANITIZE_ADDRESS__). target_compile_options(rabitq_compile_options INTERFACE - $<$:-fsanitize=address,undefined;-fno-omit-frame-pointer> + "$<$:-fsanitize=address,undefined;-fno-omit-frame-pointer>" ) target_link_options(rabitq_compile_options INTERFACE $<$:-fsanitize=address,undefined> ) endif() -function(rabitq_enable_warnings target) +function(rabitq_configure_target target) + # Apply project build policy only to first-party targets, never consumers. target_compile_options(${target} PRIVATE + $<$,$>:-O3> + $<$,$>:-march=native> "$<$:-Wall;-Wextra>" ) endfunction() @@ -151,7 +152,7 @@ set_target_properties(rabitq_core PROPERTIES EXPORT_NAME core POSITION_INDEPENDENT_CODE ON ) -rabitq_enable_warnings(rabitq_core) +rabitq_configure_target(rabitq_core) if(MSVC) set_source_files_properties( diff --git a/README.md b/README.md index 6724bf7e..b58a6d79 100644 --- a/README.md +++ b/README.md @@ -200,6 +200,11 @@ add_subdirectory(third_party/rabitqlib) target_link_libraries(my_program PRIVATE rabitqlib::rabitqlib) ``` +The target propagates C++17, OpenMP, and required public-header definitions. +Optimization and native-tuning options apply only to RaBitQ's own targets; +configure your application's optimization flags separately. Explicit sanitizer +builds propagate instrumentation and runtime linkage for header compatibility. + The C++ API and ABI are evolving; update the pinned revision deliberately. For installed packages, use `find_package(rabitqlib CONFIG REQUIRED)` and the same target. Both approaches require OpenMP. diff --git a/python_bindings/CMakeLists.txt b/python_bindings/CMakeLists.txt index 082f0e4e..7c8cbde4 100644 --- a/python_bindings/CMakeLists.txt +++ b/python_bindings/CMakeLists.txt @@ -26,7 +26,7 @@ pybind11_add_module(_rabitqlib target_include_directories(_rabitqlib PRIVATE ${CMAKE_CURRENT_SOURCE_DIR} ) -rabitq_enable_warnings(_rabitqlib) +rabitq_configure_target(_rabitqlib) # 4. Imported targets provide the matching Python, NumPy, and OpenMP settings. target_link_libraries(_rabitqlib PRIVATE diff --git a/sample/cpp/CMakeLists.txt b/sample/cpp/CMakeLists.txt index 7045e9d6..060966dc 100644 --- a/sample/cpp/CMakeLists.txt +++ b/sample/cpp/CMakeLists.txt @@ -22,5 +22,5 @@ foreach(RABITQ_SAMPLE_TARGET hnsw_rabitq_querying ) target_link_libraries(${RABITQ_SAMPLE_TARGET} PRIVATE rabitq_headers) - rabitq_enable_warnings(${RABITQ_SAMPLE_TARGET}) + rabitq_configure_target(${RABITQ_SAMPLE_TARGET}) endforeach() diff --git a/tests/CMakeLists.txt b/tests/CMakeLists.txt index b19db71c..e03bcf03 100644 --- a/tests/CMakeLists.txt +++ b/tests/CMakeLists.txt @@ -27,7 +27,7 @@ add_executable(rabitq_tests ${INTEG_TESTS} ) target_include_directories(rabitq_tests PRIVATE ${CMAKE_CURRENT_SOURCE_DIR}/common) -rabitq_enable_warnings(rabitq_tests) +rabitq_configure_target(rabitq_tests) # Link google test and rabitqlib headers target_link_libraries(rabitq_tests @@ -50,13 +50,13 @@ set_tests_properties(OpenMPBinding PROPERTIES # on a CPU without SIMD. The static library's detector object is not linked here. add_executable(fastscan_no_simd_test dispatch/fastscan_no_simd_test.cpp) target_link_libraries(fastscan_no_simd_test PRIVATE GTest::gtest_main rabitq_headers) -rabitq_enable_warnings(fastscan_no_simd_test) +rabitq_configure_target(fastscan_no_simd_test) gtest_discover_tests(fastscan_no_simd_test DISCOVERY_MODE PRE_TEST) # Keep the global allocation-failure override isolated from the main suite. add_executable(hnsw_add_allocation_test failures/hnsw_add_allocation_test.cpp) target_link_libraries(hnsw_add_allocation_test PRIVATE GTest::gtest_main rabitq_headers) -rabitq_enable_warnings(hnsw_add_allocation_test) +rabitq_configure_target(hnsw_add_allocation_test) gtest_discover_tests(hnsw_add_allocation_test DISCOVERY_MODE PRE_TEST) # Message to verify files are found diff --git a/tests/README.md b/tests/README.md index 571ef76d..469c1897 100644 --- a/tests/README.md +++ b/tests/README.md @@ -167,6 +167,28 @@ ctest --test-dir build-consumer -C Release --output-on-failure On Windows, add `-G "Visual Studio 18 2026" -A x64` to the consumer configure command. +The consumer also checks evaluated compile options for unwanted transitive build +policy. Only OpenMP compiler options, the C++17 requirement, and required MSVC +header definitions are propagated; optimization, native tuning, and warnings +stay private to first-party targets. Sanitized builds additionally propagate +instrumentation and runtime link options: Eigen's public inline allocation code +depends on the sanitizer mode and must stay consistent with the core. Pass +`-DRABITQ_EXPECT_SANITIZERS=ON` when testing such an installation with GCC/Clang. + +Test source-tree consumption through `add_subdirectory` with the same consumer: + +```text +cmake -S tests/consumer -B build-source-consumer -DCMAKE_BUILD_TYPE=Release -DRABITQ_SOURCE_DIR="" -DRABITQ_ENABLE_NATIVE_OPTIMIZATION=ON +cmake --build build-source-consumer --config Release +ctest --test-dir build-source-consumer -C Release --output-on-failure +``` + +CI checks both source-tree and installed native builds, portable installations, +and an installed sanitizer build. Native tuning retains its existing default +(`ON`) for first-party targets; disabling propagation does not make an already +native-compiled library portable. Consumers choose their own optimization +flags, including for inline code in the public headers. + ## Test structure - `unit/`: C++ index, quantization, FastScan, and utility tests diff --git a/tests/consumer/CMakeLists.txt b/tests/consumer/CMakeLists.txt index 7f98ddea..8926ffef 100644 --- a/tests/consumer/CMakeLists.txt +++ b/tests/consumer/CMakeLists.txt @@ -2,10 +2,41 @@ cmake_minimum_required(VERSION 3.20) project(rabitqlib_consumer LANGUAGES CXX) -find_package(rabitqlib CONFIG REQUIRED) +set(RABITQ_SOURCE_DIR "" CACHE PATH "Consume the source tree instead of an installed package") +option(RABITQ_EXPECT_SANITIZERS "Require sanitizer usage requirements from the package" OFF) +if(RABITQ_SOURCE_DIR) + set(RABITQ_BUILD_SAMPLES OFF) + set(RABITQ_BUILD_TESTS OFF) + set(RABITQ_BUILD_PYTHON_BINDINGS OFF) + add_subdirectory("${RABITQ_SOURCE_DIR}" rabitqlib) + # Imported targets created in the child directory are not visible here. + find_package(OpenMP REQUIRED) +else() + find_package(rabitqlib CONFIG REQUIRED) +endif() add_executable(rabitqlib_consumer main.cpp) target_link_libraries(rabitqlib_consumer PRIVATE rabitqlib::rabitqlib) enable_testing() add_test(NAME rabitqlib_consumer COMMAND rabitqlib_consumer) + +# Inspect evaluated, transitive usage requirements rather than just the direct +# properties of the public interface target. This works with multi-config builds. +file(GENERATE OUTPUT "${CMAKE_CURRENT_BINARY_DIR}/consumer-options-$-$.cmake" + CONTENT " +set(consumer_options [==[$]==]) +set(consumer_features [==[$]==]) +set(consumer_definitions [==[$]==]) +set(consumer_link_options [==[$]==]) +set(openmp_options [==[$]==]) +set(expect_msvc ${MSVC}) +set(expect_sanitizers ${RABITQ_EXPECT_SANITIZERS}) +" + TARGET rabitqlib_consumer +) +add_test(NAME rabitqlib_consumer_options + COMMAND "${CMAKE_COMMAND}" + "-DOPTIONS_FILE=${CMAKE_CURRENT_BINARY_DIR}/consumer-options-$-CXX.cmake" + -P "${CMAKE_CURRENT_SOURCE_DIR}/check_options.cmake" +) diff --git a/tests/consumer/check_options.cmake b/tests/consumer/check_options.cmake new file mode 100644 index 00000000..1c2a3d55 --- /dev/null +++ b/tests/consumer/check_options.cmake @@ -0,0 +1,35 @@ +cmake_minimum_required(VERSION 3.20) + +include("${OPTIONS_FILE}") + +# Only public-header requirements may add compiler options. Sanitizer builds +# intentionally propagate instrumentation to keep Eigen allocation consistent. +# Catch optimization, architecture and warnings leaking through any level of +# the target graph, including installed exports. +set(required_options ${openmp_options}) +if(expect_sanitizers) + list(APPEND required_options -fsanitize=address,undefined -fno-omit-frame-pointer) +endif() +foreach(flag IN LISTS consumer_options) + if(NOT flag STREQUAL "" AND NOT flag IN_LIST required_options) + message(FATAL_ERROR "Unexpected downstream compile option: ${flag}") + endif() +endforeach() +foreach(flag IN LISTS required_options) + if(NOT flag STREQUAL "" AND NOT flag IN_LIST consumer_options) + message(FATAL_ERROR "Missing required downstream compile option: ${flag}") + endif() +endforeach() +if(NOT "cxx_std_17" IN_LIST consumer_features) + message(FATAL_ERROR "The package must propagate its C++17 requirement") +endif() +if(expect_msvc) + foreach(definition IN ITEMS __restrict__=__restrict EIGEN_DONT_VECTORIZE) + if(NOT definition IN_LIST consumer_definitions) + message(FATAL_ERROR "Missing public-header definition: ${definition}") + endif() + endforeach() +endif() +if(expect_sanitizers AND NOT "-fsanitize=address,undefined" IN_LIST consumer_link_options) + message(FATAL_ERROR "The instrumented library must propagate sanitizer runtime linkage") +endif() diff --git a/tests/consumer/main.cpp b/tests/consumer/main.cpp index 58a97420..a05cd225 100644 --- a/tests/consumer/main.cpp +++ b/tests/consumer/main.cpp @@ -93,45 +93,54 @@ int main() { return 1; } - rabitqlib::ivf::IVF index( - kPoints, - kDimension, - flat.k, - 4, - rabitqlib::METRIC_L2, - rabitqlib::RotatorType::FhtKacRotator, - rabitqlib::ivf::InitializerType::FlatRaBitQ - ); - if (index.initializer_type() != rabitqlib::ivf::InitializerType::FlatRaBitQ) { - std::cerr << "Installed IVF did not preserve explicit centroid routing\n"; - return 1; - } - index.construct(data.data(), flat.centroids.data(), flat.assignments.data(), false, 2); - std::vector ids(kPoints); - std::vector distances(kPoints); - index.search_batch(data.data(), kPoints, 1, flat.k, ids.data(), distances.data()); - for (size_t i = 0; i < kPoints; ++i) { - rabitqlib::PID id = 0; - float distance = 0; - index.search(data.data() + i * kDimension, 1, flat.k, &id, &distance); - if (ids[i] != id || distances[i] != distance) { - std::cerr << "Installed IVF batch and single-query results differ\n"; + // Exercise both header-defined IVF and compiled SymphonyQG with both + // rotators, including Eigen-backed ownership across the library boundary. + for (const auto rotator : + {rabitqlib::RotatorType::FhtKacRotator, rabitqlib::RotatorType::MatrixRotator}) { + rabitqlib::ivf::IVF index( + kPoints, + kDimension, + flat.k, + 4, + rabitqlib::METRIC_L2, + rotator, + rabitqlib::ivf::InitializerType::FlatRaBitQ + ); + if (index.initializer_type() != rabitqlib::ivf::InitializerType::FlatRaBitQ) { + std::cerr << "Installed IVF did not preserve explicit centroid routing\n"; return 1; } - } + index.construct( + data.data(), flat.centroids.data(), flat.assignments.data(), false, 2 + ); + std::vector ids(kPoints); + std::vector distances(kPoints); + index.search_batch(data.data(), kPoints, 1, flat.k, ids.data(), distances.data()); + for (size_t i = 0; i < kPoints; ++i) { + rabitqlib::PID id = 0; + float distance = 0; + index.search(data.data() + i * kDimension, 1, flat.k, &id, &distance); + if (ids[i] != id || distances[i] != distance) { + std::cerr << "Installed IVF batch and single-query results differ\n"; + return 1; + } + } - rabitqlib::symqg::QuantizedGraph symqg(kPoints, kDimension, 32); - rabitqlib::symqg::QGBuilder builder(symqg, 64, data.data(), 2); - builder.build(); - symqg.set_ef(64); - symqg.search_batch(data.data(), kPoints, 1, ids.data(), distances.data(), 2); - for (size_t i = 0; i < kPoints; ++i) { - rabitqlib::PID id = 0; - float distance = 0; - symqg.search(data.data() + i * kDimension, 1, &id, &distance); - if (ids[i] != id || distances[i] != distance) { - std::cerr << "Installed SymphonyQG batch and single-query results differ\n"; - return 1; + rabitqlib::symqg::QuantizedGraph symqg( + kPoints, kDimension, 32, rabitqlib::METRIC_L2, rotator + ); + rabitqlib::symqg::QGBuilder builder(symqg, 64, data.data(), 2); + builder.build(); + symqg.set_ef(64); + symqg.search_batch(data.data(), kPoints, 1, ids.data(), distances.data(), 2); + for (size_t i = 0; i < kPoints; ++i) { + rabitqlib::PID id = 0; + float distance = 0; + symqg.search(data.data() + i * kDimension, 1, &id, &distance); + if (ids[i] != id || distances[i] != distance) { + std::cerr << "Installed SymphonyQG batch and single-query results differ\n"; + return 1; + } } } return 0; From e963244414e2fe58aec3ba4c2cce7ad5be4a0152 Mon Sep 17 00:00:00 2001 From: gouyt13clear Date: Fri, 9 Oct 2026 13:22:11 +0800 Subject: [PATCH 2/2] test: detect leaked CMake options across compilers --- tests/consumer/CMakeLists.txt | 15 +++++++++++++-- 1 file changed, 13 insertions(+), 2 deletions(-) diff --git a/tests/consumer/CMakeLists.txt b/tests/consumer/CMakeLists.txt index 8926ffef..a1c3bd57 100644 --- a/tests/consumer/CMakeLists.txt +++ b/tests/consumer/CMakeLists.txt @@ -4,17 +4,28 @@ project(rabitqlib_consumer LANGUAGES CXX) set(RABITQ_SOURCE_DIR "" CACHE PATH "Consume the source tree instead of an installed package") option(RABITQ_EXPECT_SANITIZERS "Require sanitizer usage requirements from the package" OFF) +find_package(OpenMP REQUIRED) if(RABITQ_SOURCE_DIR) set(RABITQ_BUILD_SAMPLES OFF) set(RABITQ_BUILD_TESTS OFF) set(RABITQ_BUILD_PYTHON_BINDINGS OFF) add_subdirectory("${RABITQ_SOURCE_DIR}" rabitqlib) - # Imported targets created in the child directory are not visible here. - find_package(OpenMP REQUIRED) + set(rabitq_options_target rabitq_compile_options) else() find_package(rabitqlib CONFIG REQUIRED) + set(rabitq_options_target rabitqlib::compile_options) endif() +# Check the raw public interface as well as evaluated consumer options below. +# The optimization generators are inactive with AppleClang, so an evaluated-only +# check would miss an accidental declaration that affects GCC and upstream Clang. +get_target_property(rabitq_public_options ${rabitq_options_target} INTERFACE_COMPILE_OPTIONS) +foreach(forbidden IN ITEMS -O3 -march=native -Wall -Wextra) + if("${rabitq_public_options}" MATCHES "${forbidden}") + message(FATAL_ERROR "Unexpected public compile option: ${forbidden}") + endif() +endforeach() + add_executable(rabitqlib_consumer main.cpp) target_link_libraries(rabitqlib_consumer PRIVATE rabitqlib::rabitqlib)