diff --git a/CHANGELOG.md b/CHANGELOG.md index 7cfc2f87d5979cb3a2bcf4cbfcd585d6b0712a6d..13e1a1828646a0148120be6a7ff69482791eacc2 100644 --- a/CHANGELOG.md +++ b/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 diff --git a/core/CMakeLists.txt b/core/CMakeLists.txt index c24eea0cceba321aaad0bfdc33c13a1d1d279d49..96bc68dcf3036e6308ed86a5e2cb45082a219708 100644 --- a/core/CMakeLists.txt +++ b/core/CMakeLists.txt @@ -100,7 +100,7 @@ append_flags( CMAKE_CXX_FLAGS "-fPIC" "-DELPP_THREAD_SAFE" "-fopenmp" - "-Werror=return-type" + "-Werror" ) # **************************** Source files **************************** diff --git a/core/src/index/cmake/ThirdPartyPackagesCore.cmake b/core/src/index/cmake/ThirdPartyPackagesCore.cmake index fc5aabb8674a1edbe6182ddf74eba0bc02230c32..6695797043fe9c89d87141ee8cba0705925a77c4 100644 --- a/core/src/index/cmake/ThirdPartyPackagesCore.cmake +++ b/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 diff --git a/core/src/index/knowhere/knowhere/index/vector_index/IndexHNSW.cpp b/core/src/index/knowhere/knowhere/index/vector_index/IndexHNSW.cpp index 422b7ee2cc5a7c219a38dfc3160fd3079cb1baee..1ceae4540d8c78520c0fecf88dfbdd88247b50a7 100644 --- a/core/src/index/knowhere/knowhere/index/vector_index/IndexHNSW.cpp +++ b/core/src/index/knowhere/knowhere/index/vector_index/IndexHNSW.cpp @@ -14,6 +14,7 @@ #include #include #include +#include #include #include @@ -82,11 +83,14 @@ IndexHNSW::Train(const DatasetPtr& dataset_ptr, const Config& config) { int64_t rows = dataset_ptr->Get(meta::ROWS); hnswlib::SpaceInterface* 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>(space, rows, config[IndexParams::M].get(), config[IndexParams::efConstruction].get()); diff --git a/core/src/index/knowhere/knowhere/index/vector_index/IndexRHNSW.cpp b/core/src/index/knowhere/knowhere/index/vector_index/IndexRHNSW.cpp index 6508a34217d067e1c524f0dcad9eda6a8a43ad6e..1da965f8575208b6aa2a35e46901eda5553b5473 100644 --- a/core/src/index/knowhere/knowhere/index/vector_index/IndexRHNSW.cpp +++ b/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(); - size_t id_size = sizeof(int64_t) * k; - size_t dist_size = sizeof(float) * k; + int64_t k = config[meta::TOPK].get(); + 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) { diff --git a/core/src/index/knowhere/knowhere/index/vector_index/impl/nsg/Distance.h b/core/src/index/knowhere/knowhere/index/vector_index/impl/nsg/Distance.h index 9561d95936b3920db043bc60e249a1e30bc19de0..ac8d20c5598280d3f6320f7adde98403bc997b22 100644 --- a/core/src/index/knowhere/knowhere/index/vector_index/impl/nsg/Distance.h +++ b/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; }; diff --git a/core/src/index/knowhere/knowhere/index/vector_index/impl/nsg/NSG.cpp b/core/src/index/knowhere/knowhere/index/vector_index/impl/nsg/NSG.cpp index 3e7d52b7ac414adee54f2c3c9d28575a6fdb19d2..b5be90ae9676e4b2cf8a6bfab10ab4b660f8d9d4 100644 --- a/core/src/index/knowhere/knowhere/index/vector_index/impl/nsg/NSG.cpp +++ b/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; diff --git a/core/src/index/thirdparty/faiss/utils/instruction_set.h b/core/src/index/thirdparty/faiss/utils/instruction_set.h index 90ccc2c3689cf92ba823d28b18d94aa243eabc96..c2fe84ca70ed652ddd06d5ddedf3b3007077cfad 100644 --- a/core/src/index/thirdparty/faiss/utils/instruction_set.h +++ b/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)); diff --git a/core/src/index/thirdparty/hnswlib/hnswalg.h b/core/src/index/thirdparty/hnswlib/hnswalg.h index f0805d649e0a3ecf4ef2003eee66ec1a824cbcdb..61158a61d8166e1020568675a1e15c09b7c1dff6 100644 --- a/core/src/index/thirdparty/hnswlib/hnswalg.h +++ b/core/src/index/thirdparty/hnswlib/hnswalg.h @@ -1140,7 +1140,7 @@ class HierarchicalNSW : public AlgorithmInterface { 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; diff --git a/core/src/index/unittest/test_structured_index_sort.cpp b/core/src/index/unittest/test_structured_index_sort.cpp index 13bbdd11bf74cc7d1740c139afd76fe606de5659..896ab8888e4435accb1dcbd38a4ba13ac2d4b936 100644 --- a/core/src/index/unittest/test_structured_index_sort.cpp +++ b/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 structuredIndexSort((size_t)n, p); // Build default - size_t test_times = 10; + int test_times = 10; std::vector 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 structuredIndexSort((size_t)n, p); // Build default - size_t test_times = 10; + int test_times = 10; std::vector 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); } diff --git a/core/thirdparty/dablooms/dablooms.cpp b/core/thirdparty/dablooms/dablooms.cpp index 3479be202d84dac82579940443fc5c84d3d15a14..5dd9d1d32e96783a78198c823f4912c8c07b8f0b 100644 --- a/core/thirdparty/dablooms/dablooms.cpp +++ b/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 #include #include