enable -Werror to improve code quality (#3221)

* clean warning

Signed-off-by: yudong.cai <yudong.cai@zilliz.com>

* update faiss make options

Signed-off-by: yudong.cai <yudong.cai@zilliz.com>

* update changelog

Signed-off-by: yudong.cai <yudong.cai@zilliz.com>

* fix clang format

Signed-off-by: yudong.cai <yudong.cai@zilliz.com>

* remove wrong file

Signed-off-by: yudong.cai <yudong.cai@zilliz.com>

* update unittest

Signed-off-by: yudong.cai <yudong.cai@zilliz.com>

Co-authored-by: shengjun.li <shengjun.li@zilliz.com>

CHANGELOG.md

@@ -49,6 +49,7 @@ Please mark all changes in change log and use the issue from GitHub
 -   \#2841 Replace IndexType/EngineType/MetricType
 -   \#2858 Unify index name in db
 -   \#2884 Using BlockingQueue in JobMgr
+-   \#3220 Enable -Werror to improve code quality
 
 ## Task
 

core/CMakeLists.txt

@@ -100,7 +100,7 @@ append_flags( CMAKE_CXX_FLAGS
                     "-fPIC"
                     "-DELPP_THREAD_SAFE"
                     "-fopenmp"
-                    "-Werror=return-type"
+                    "-Werror"
                     )
 
 # **************************** Source files ****************************

core/src/index/cmake/ThirdPartyPackagesCore.cmake

@@ -119,8 +119,9 @@ set(THIRDPARTY_DIR "${INDEX_SOURCE_DIR}/thirdparty")
 
 string(TOUPPER ${CMAKE_BUILD_TYPE} UPPERCASE_BUILD_TYPE)
 
-set(EP_CXX_FLAGS "${CMAKE_CXX_FLAGS} ${CMAKE_CXX_FLAGS_${UPPERCASE_BUILD_TYPE}}")
-set(EP_C_FLAGS "${CMAKE_C_FLAGS} ${CMAKE_C_FLAGS_${UPPERCASE_BUILD_TYPE}}")
+set(FAISS_FLAGS "-DELPP_THREAD_SAFE -fopenmp -Werror=return-type")
+set(EP_CXX_FLAGS "${FAISS_FLAGS} ${CMAKE_CXX_FLAGS_${UPPERCASE_BUILD_TYPE}}")
+set(EP_C_FLAGS   "${FAISS_FLAGS} ${CMAKE_C_FLAGS_${UPPERCASE_BUILD_TYPE}}")
 
 if (NOT MSVC)
     # Set -fPIC on all external projects

core/src/index/knowhere/knowhere/index/vector_index/IndexHNSW.cpp

@@ -14,6 +14,7 @@
 #include <algorithm>
 #include <cassert>
 #include <iterator>
+#include <string>
 #include <utility>
 #include <vector>
 
@@ -82,11 +83,14 @@ IndexHNSW::Train(const DatasetPtr& dataset_ptr, const Config& config) {
         int64_t rows = dataset_ptr->Get<int64_t>(meta::ROWS);
 
         hnswlib::SpaceInterface<float>* space;
-        if (config[Metric::TYPE] == Metric::L2) {
+        std::string metric_type = config[Metric::TYPE];
+        if (metric_type == Metric::L2) {
             space = new hnswlib::L2Space(dim);
-        } else if (config[Metric::TYPE] == Metric::IP) {
+        } else if (metric_type == Metric::IP) {
             space = new hnswlib::InnerProductSpace(dim);
             normalize = true;
+        } else {
+            KNOWHERE_THROW_MSG("Metric type not supported: " + metric_type);
         }
         index_ = std::make_shared<hnswlib::HierarchicalNSW<float>>(space, rows, config[IndexParams::M].get<int64_t>(),
                                                                    config[IndexParams::efConstruction].get<int64_t>());

core/src/index/knowhere/knowhere/index/vector_index/IndexRHNSW.cpp

@@ -85,9 +85,9 @@ IndexRHNSW::Query(const DatasetPtr& dataset_ptr, const Config& config) {
     }
     GET_TENSOR_DATA(dataset_ptr)
 
-    size_t k = config[meta::TOPK].get<int64_t>();
-    size_t id_size = sizeof(int64_t) * k;
-    size_t dist_size = sizeof(float) * k;
+    int64_t k = config[meta::TOPK].get<int64_t>();
+    int64_t id_size = sizeof(int64_t) * k;
+    int64_t dist_size = sizeof(float) * k;
     auto p_id = (int64_t*)malloc(id_size * rows);
     auto p_dist = (float*)malloc(dist_size * rows);
     for (auto i = 0; i < k * rows; ++i) {

core/src/index/knowhere/knowhere/index/vector_index/impl/nsg/Distance.h

@@ -16,6 +16,7 @@ namespace knowhere {
 namespace impl {
 
 struct Distance {
+    virtual ~Distance() = default;
     virtual float
     Compare(const float* a, const float* b, unsigned size) const = 0;
 };

core/src/index/knowhere/knowhere/index/vector_index/impl/nsg/NSG.cpp

@@ -880,10 +880,10 @@ NsgIndex::GetSize() {
     ret += ntotal * dimension * sizeof(float);
     ret += ntotal * sizeof(int64_t);
     ret += sizeof(*distance_);
-    for (auto i = 0; i < nsg.size(); ++i) {
+    for (size_t i = 0; i < nsg.size(); ++i) {
         ret += nsg[i].size() * sizeof(node_t);
     }
-    for (auto i = 0; i < knng.size(); ++i) {
+    for (size_t i = 0; i < knng.size(); ++i) {
         ret += knng[i].size() * sizeof(node_t);
     }
     return ret;

core/src/index/thirdparty/faiss/utils/instruction_set.h

@@ -84,13 +84,13 @@ class InstructionSet {
         }
 
         // load bitset with flags for function 0x80000001
-        if (nExIds_ >= 0x80000001) {
+        if (nExIds_ >= (int)0x80000001) {
             f_81_ECX_ = extdata_[1][2];
             f_81_EDX_ = extdata_[1][3];
         }
 
         // Interpret CPU brand string if reported
-        if (nExIds_ >= 0x80000004) {
+        if (nExIds_ >= (int)0x80000004) {
             memcpy(brand, extdata_[2].data(), sizeof(cpui));
             memcpy(brand + 16, extdata_[3].data(), sizeof(cpui));
             memcpy(brand + 32, extdata_[4].data(), sizeof(cpui));

core/src/index/thirdparty/hnswlib/hnswalg.h

@@ -1140,7 +1140,7 @@ class HierarchicalNSW : public AlgorithmInterface<dist_t> {
         ret += element_levels_.size() * sizeof(int);
         ret += max_elements_ * size_data_per_element_;
         ret += max_elements_ * sizeof(void*);
-        for (auto i = 0; i < max_elements_; ++ i) {
+        for (size_t i = 0; i < max_elements_; ++ i) {
             ret += linkLists_[i] ? size_links_per_element_ * element_levels_[i] : 0;
         }
         return ret;

core/src/index/unittest/test_structured_index_sort.cpp

@@ -116,7 +116,7 @@ TEST(STRUCTUREDINDEXSORT_TEST, test_in) {
     gen_rand_data(range, n, p);
     milvus::knowhere::StructuredIndexSort<int> structuredIndexSort((size_t)n, p);  // Build default
 
-    size_t test_times = 10;
+    int test_times = 10;
     std::vector<int> test_vals, test_off;
     test_vals.reserve(test_times);
     test_off.reserve(test_times);
@@ -141,25 +141,25 @@ TEST(STRUCTUREDINDEXSORT_TEST, test_not_in) {
     gen_rand_data(range, n, p);
     milvus::knowhere::StructuredIndexSort<int> structuredIndexSort((size_t)n, p);  // Build default
 
-    size_t test_times = 10;
+    int test_times = 10;
     std::vector<int> test_vals, test_off;
     test_vals.reserve(test_times);
     test_off.reserve(test_times);
-    //    std::cout << "STRUCTUREDINDEXSORT_TEST test_notin" << std::endl;
+    // std::cout << "STRUCTUREDINDEXSORT_TEST test_notin" << std::endl;
     for (auto i = 0; i < test_times; ++i) {
         auto off = random() % n;
         test_vals.emplace_back(*(p + off));
         test_off.emplace_back(off);
-        //        std::cout << off << " ";
+        // std::cout << off << " ";
     }
-    //    std::cout << std::endl;
+    // std::cout << std::endl;
     auto res = structuredIndexSort.NotIn(test_times, test_vals.data());
-    //    std::cout << "assert values: " << std::endl;
+    // std::cout << "assert values: " << std::endl;
     for (auto i = 0; i < test_times; ++i) {
-        //        std::cout << test_off[i] << " ";
+        // std::cout << test_off[i] << " ";
         ASSERT_EQ(false, res->test(test_off[i]));
     }
-    //    std::cout << std::endl;
+    // std::cout << std::endl;
 
     free(p);
 }

core/thirdparty/dablooms/dablooms.cpp

@@ -1,6 +1,5 @@
 /* Copyright @2012 by Justin Hines at Bitly under a very liberal license. See LICENSE in the source distribution. */
 
-#define _GNU_SOURCE
 #include <sys/stat.h>
 #include <stdint.h>
 #include <stdio.h>