Fix int/size_t signedness mismatches in HNSW add (#5116)

Summary:
Pull Request resolved: #5116

The `hnsw_add_vertices()` function in both `IndexHNSW.cpp` and `IndexBinaryHNSW.cpp` accepts `size_t n0` and `size_t n` parameters, but the loop variables and counters inside were declared as `int`. This caused signed/unsigned comparison mismatches and implicit narrowing conversions. This diff fixes all such mismatches with six targeted changes:

1. **Loop variables `i0`, `i1`, `j` changed from `int` to `size_t`; OpenMP loop variable `i` changed from `int` to `int64_t`**: These variables derive from `n` (which is `size_t`) and are used as indices into `std::vector<int> order(n)`. Using `int` produced signed/unsigned comparison warnings in loop conditions (`j < i1`, `i < i1`) and risked undefined behavior via signed integer overflow for indices above `INT_MAX` (~2.1B). The outer variables use `size_t` to match the function parameter types. The OpenMP `for` loop variable uses `int64_t` because MSVC enforces that OpenMP loop counters must have signed integral types (`error C3016`), and `size_t` (unsigned) violates this requirement. `int64_t` is signed, 64-bit, and compatible with both the `size_t` loop bounds and MSVC OpenMP.

2. **`int n0 = ntotal` changed to `size_t n0 = ntotal` in `add()` methods**: `ntotal` is `idx_t` (`int64_t`), and `n0` is passed directly to `hnsw_add_vertices(..., size_t n0, ...)`. The `int` declaration caused an implicit 64-to-32-bit narrowing conversion at the call site.

3. **`printf` format specifiers changed from `%d` to `%zu`**: Required to match the new `size_t` types. Using `%d` with `size_t` arguments is undefined behavior per the C standard.

4. **`prev_display` sentinel logic refactored**: The old pattern used `int prev_display = verbose ? 0 : -1` with a `prev_display >= 0` guard, encoding a boolean in a signed integer sentinel. This was incompatible with the `size_t` migration (cannot represent -1). Replaced with an explicit `bool do_display` flag and `size_t prev_display = 0`, which is semantically identical and clearer.

5. **`std::make_unique<IndexBinaryFlat>(d_).release()` simplified to `new IndexBinaryFlat(d_)`**: The `make_unique().release()` pattern creates a `unique_ptr` only to immediately release ownership, which is equivalent to `new` but with unnecessary intermediate allocation tracking. Since `storage` is a raw pointer with `own_fields = true` ownership semantics (deleted in the destructor), plain `new` matches the established Faiss ownership pattern used throughout the codebase.

6. **`static_cast<int>` added at `rand_int()` call sites**: The `int`-to-`size_t` migration changed `i1 - j` from `int` to `size_t`, introducing a new `clang-diagnostic-shorten-64-to-32` warning when passed to `rand_int(int max)`. The explicit cast documents the intentional narrowing. The value is safe: `i1 - j` represents the remaining elements in a single HNSW level bucket, bounded well below `INT_MAX`.

All changes are mechanical. No functional changes.

Reviewed By: junjieqi

Differential Revision: D101353511

fbshipit-source-id: a4a44b6a5436a77d97d47389db52b8721b2f97c4

Author: alibeklfc

faiss/IndexBinaryHNSW.cpp

@@ -99,20 +99,22 @@ void hnsw_add_vertices(
     { // perform add
         RandomGenerator rng2(789);
 
-        int i1 = n;
+        size_t i1 = n;
 
         for (int pt_level = static_cast<int>(hist.size()) - 1;
              pt_level >= int(!index_hnsw.init_level0);
              pt_level--) {
-            int i0 = i1 - hist[pt_level];
+            size_t i0 = i1 - hist[pt_level];
 
             if (verbose) {
-                printf("Adding %d elements at level %d\n", i1 - i0, pt_level);
+                printf("Adding %zu elements at level %d\n", i1 - i0, pt_level);
             }
 
             // random permutation to get rid of dataset order bias
-            for (int j = i0; j < i1; j++) {
-                std::swap(order[j], order[j + rng2.rand_int(i1 - j)]);
+            for (size_t j = i0; j < i1; j++) {
+                std::swap(
+                        order[j],
+                        order[j + rng2.rand_int(static_cast<int>(i1 - j))]);
             }
 
 #pragma omp parallel
@@ -121,11 +123,11 @@ void hnsw_add_vertices(
 
                 std::unique_ptr<DistanceComputer> dis(
                         index_hnsw.get_distance_computer());
-                int prev_display =
-                        verbose && omp_get_thread_num() == 0 ? 0 : -1;
+                bool do_display = verbose && omp_get_thread_num() == 0;
+                size_t prev_display = 0;
 
 #pragma omp for schedule(dynamic)
-                for (int i = i0; i < i1; i++) {
+                for (int64_t i = i0; i < i1; i++) {
                     HNSW::storage_idx_t pt_id = order[i];
                     dis->set_query(
                             (float*)(x + (pt_id - n0) * index_hnsw.code_size));
@@ -138,9 +140,9 @@ void hnsw_add_vertices(
                             vt,
                             index_hnsw.keep_max_size_level0 && (pt_level == 0));
 
-                    if (prev_display >= 0 && i - i0 > prev_display + 10000) {
+                    if (do_display && i - i0 > prev_display + 10000) {
                         prev_display = i - i0;
-                        printf("  %d / %d\r", i - i0, i1 - i0);
+                        printf("  %zu / %zu\r", i - i0, i1 - i0);
                         fflush(stdout);
                     }
                 }
@@ -176,7 +178,7 @@ IndexBinaryHNSW::IndexBinaryHNSW(int d_, int M)
         : IndexBinary(d_),
           hnsw(M),
           own_fields(true),
-          storage(std::make_unique<IndexBinaryFlat>(d_).release()) {
+          storage(new IndexBinaryFlat(d_)) {
     is_trained = true;
 }
 
@@ -257,7 +259,7 @@ void IndexBinaryHNSW::search(
 
 void IndexBinaryHNSW::add(idx_t n, const uint8_t* x) {
     FAISS_THROW_IF_NOT(is_trained);
-    int n0 = ntotal;
+    size_t n0 = ntotal;
     storage->add(n, x);
     ntotal = storage->ntotal;
 

faiss/IndexHNSW.cpp

@@ -122,20 +122,22 @@ void hnsw_add_vertices(
     { // perform add
         RandomGenerator rng2(789);
 
-        int i1 = n;
+        size_t i1 = n;
 
         for (int pt_level = static_cast<int>(hist.size()) - 1;
              pt_level >= int(!index_hnsw.init_level0);
              pt_level--) {
-            int i0 = i1 - hist[pt_level];
+            size_t i0 = i1 - hist[pt_level];
 
             if (verbose) {
-                printf("Adding %d elements at level %d\n", i1 - i0, pt_level);
+                printf("Adding %zu elements at level %d\n", i1 - i0, pt_level);
             }
 
             // random permutation to get rid of dataset order bias
-            for (int j = i0; j < i1; j++) {
-                std::swap(order[j], order[j + rng2.rand_int(i1 - j)]);
+            for (size_t j = i0; j < i1; j++) {
+                std::swap(
+                        order[j],
+                        order[j + rng2.rand_int(static_cast<int>(i1 - j))]);
             }
 
             bool interrupt = false;
@@ -146,15 +148,15 @@ void hnsw_add_vertices(
 
                 std::unique_ptr<DistanceComputer> dis(
                         storage_distance_computer(index_hnsw.storage));
-                int prev_display =
-                        verbose && omp_get_thread_num() == 0 ? 0 : -1;
+                bool do_display = verbose && omp_get_thread_num() == 0;
+                size_t prev_display = 0;
                 size_t counter = 0;
 
                 // here we should do schedule(dynamic) but this segfaults for
                 // some versions of LLVM. The performance impact should not be
                 // too large when (i1 - i0) / num_threads >> 1
 #pragma omp for schedule(static)
-                for (int i = i0; i < i1; i++) {
+                for (int64_t i = i0; i < i1; i++) {
                     storage_idx_t pt_id = order[i];
                     dis->set_query(x + (pt_id - n0) * d);
 
@@ -171,9 +173,9 @@ void hnsw_add_vertices(
                             vt,
                             index_hnsw.keep_max_size_level0 && (pt_level == 0));
 
-                    if (prev_display >= 0 && i - i0 > prev_display + 10000) {
+                    if (do_display && i - i0 > prev_display + 10000) {
                         prev_display = i - i0;
-                        printf("  %d / %d\r", i - i0, i1 - i0);
+                        printf("  %zu / %zu\r", i - i0, i1 - i0);
                         fflush(stdout);
                     }
                     if (counter % check_period == 0) {
@@ -349,7 +351,7 @@ void IndexHNSW::add(idx_t n, const float* x) {
             storage,
             "Please use IndexHNSWFlat (or variants) instead of IndexHNSW directly");
     FAISS_THROW_IF_NOT(is_trained);
-    int n0 = ntotal;
+    size_t n0 = ntotal;
     storage->add(n, x);
     ntotal = storage->ntotal;