From 4919ccf543777542103c3d960c537f76eefd3d42 Mon Sep 17 00:00:00 2001 From: Ted Xu Date: Mon, 16 Dec 2024 09:58:43 +0800 Subject: [PATCH] enhance: eliminate compile warnings (#38420) See: #38435 --------- Signed-off-by: Ted Xu --- internal/core/CMakeLists.txt | 4 ++- internal/core/src/common/Chunk.h | 2 +- internal/core/src/common/ChunkWriter.cpp | 13 +++++----- internal/core/src/common/ChunkWriter.h | 12 ++++----- internal/core/src/common/FieldData.h | 7 +++-- internal/core/src/exec/expression/Expr.cpp | 2 +- internal/core/src/exec/expression/Expr.h | 1 - .../core/src/exec/expression/UnaryExpr.cpp | 5 +--- internal/core/src/exec/expression/UnaryExpr.h | 21 +++++++-------- .../operator/groupby/SearchGroupByOperator.h | 4 +-- internal/core/src/mmap/ChunkData.h | 4 --- internal/core/src/mmap/ChunkedColumn.h | 26 +++++++++++-------- internal/core/src/mmap/Column.h | 8 +++--- .../src/segcore/ChunkedSegmentSealedImpl.cpp | 14 ++-------- 14 files changed, 56 insertions(+), 67 deletions(-) diff --git a/internal/core/CMakeLists.txt b/internal/core/CMakeLists.txt index 2b33959183843..f86465758f435 100644 --- a/internal/core/CMakeLists.txt +++ b/internal/core/CMakeLists.txt @@ -148,7 +148,9 @@ if ( APPLE ) "-DELPP_THREAD_SAFE" "-fopenmp" "-pedantic" - "-Wno-all" + "-Wall" + "-Wno-gnu-zero-variadic-macro-arguments" + "-Wno-variadic-macros" "-DBOOST_STACKTRACE_GNU_SOURCE_NOT_REQUIRED=1" ) endif () diff --git a/internal/core/src/common/Chunk.h b/internal/core/src/common/Chunk.h index 7a1f56fec3c33..7c26bdeba7710 100644 --- a/internal/core/src/common/Chunk.h +++ b/internal/core/src/common/Chunk.h @@ -243,7 +243,7 @@ class SparseFloatVectorChunk : public Chunk { for (int i = 0; i < row_nums; i++) { vec_[i] = {(offsets_ptr[i + 1] - offsets_ptr[i]) / knowhere::sparse::SparseRow::element_size(), - (uint8_t*)(data + offsets_ptr[i]), + reinterpret_cast(data + offsets_ptr[i]), false}; dim_ = std::max(dim_, vec_[i].dim()); } diff --git a/internal/core/src/common/ChunkWriter.cpp b/internal/core/src/common/ChunkWriter.cpp index 5542556abc067..dc2d9e4bfe45e 100644 --- a/internal/core/src/common/ChunkWriter.cpp +++ b/internal/core/src/common/ChunkWriter.cpp @@ -12,6 +12,7 @@ #include "common/ChunkWriter.h" #include #include +#include #include #include "arrow/array/array_binary.h" #include "arrow/array/array_primitive.h" @@ -66,7 +67,7 @@ StringChunkWriter::write(std::shared_ptr data) { int offset_start_pos = target_->tell() + sizeof(uint64_t) * offset_num; std::vector offsets; - for (auto str : strs) { + for (const auto& str : strs) { offsets.push_back(offset_start_pos); offset_start_pos += str.size(); } @@ -133,7 +134,7 @@ JSONChunkWriter::write(std::shared_ptr data) { int offset_start_pos = target_->tell() + sizeof(uint64_t) * offset_num; std::vector offsets; - for (auto json : jsons) { + for (const auto& json : jsons) { offsets.push_back(offset_start_pos); offset_start_pos += json.data().size(); } @@ -142,7 +143,7 @@ JSONChunkWriter::write(std::shared_ptr data) { target_->write(offsets.data(), offsets.size() * sizeof(uint64_t)); // write data - for (auto json : jsons) { + for (const auto& json : jsons) { target_->write(json.data().data(), json.data().size()); } } @@ -288,7 +289,7 @@ SparseFloatVectorChunkWriter::write( int offset_start_pos = target_->tell() + sizeof(uint64_t) * offset_num; std::vector offsets; - for (auto str : strs) { + for (const auto& str : strs) { offsets.push_back(offset_start_pos); offset_start_pos += str.size(); } @@ -396,7 +397,7 @@ create_chunk(const FieldMeta& field_meta, PanicInfo(Unsupported, "Unsupported data type"); } - w->write(r); + w->write(std::move(r)); return w->finish(); } @@ -493,7 +494,7 @@ create_chunk(const FieldMeta& field_meta, PanicInfo(Unsupported, "Unsupported data type"); } - w->write(r); + w->write(std::move(r)); return w->finish(); } diff --git a/internal/core/src/common/ChunkWriter.h b/internal/core/src/common/ChunkWriter.h index c389b0e799096..09225f44afa02 100644 --- a/internal/core/src/common/ChunkWriter.h +++ b/internal/core/src/common/ChunkWriter.h @@ -67,7 +67,7 @@ class ChunkWriter : public ChunkWriterBase { auto batch_vec = data->ToRecordBatches().ValueOrDie(); - for (auto batch : batch_vec) { + for (auto& batch : batch_vec) { row_nums += batch->num_rows(); auto data = batch->column(0); auto array = std::dynamic_pointer_cast(data); @@ -83,7 +83,7 @@ class ChunkWriter : public ChunkWriterBase { } // chunk layout: nullbitmap, data1, data2, ..., datan - for (auto batch : batch_vec) { + for (auto& batch : batch_vec) { auto data = batch->column(0); auto null_bitmap = data->null_bitmap_data(); auto null_bitmap_n = (data->length() + 7) / 8; @@ -95,7 +95,7 @@ class ChunkWriter : public ChunkWriterBase { } } - for (auto batch : batch_vec) { + for (auto& batch : batch_vec) { auto data = batch->column(0); auto array = std::dynamic_pointer_cast(data); auto data_ptr = array->raw_values(); @@ -122,7 +122,7 @@ ChunkWriter::write( auto row_nums = 0; auto batch_vec = data->ToRecordBatches().ValueOrDie(); - for (auto batch : batch_vec) { + for (auto& batch : batch_vec) { row_nums += batch->num_rows(); auto data = batch->column(0); auto array = std::dynamic_pointer_cast(data); @@ -136,7 +136,7 @@ ChunkWriter::write( target_ = std::make_shared(size); } // chunk layout: nullbitmap, data1, data2, ..., datan - for (auto batch : batch_vec) { + for (auto& batch : batch_vec) { auto data = batch->column(0); auto null_bitmap = data->null_bitmap_data(); auto null_bitmap_n = (data->length() + 7) / 8; @@ -148,7 +148,7 @@ ChunkWriter::write( } } - for (auto batch : batch_vec) { + for (auto& batch : batch_vec) { auto data = batch->column(0); auto array = std::dynamic_pointer_cast(data); for (int i = 0; i < array->length(); i++) { diff --git a/internal/core/src/common/FieldData.h b/internal/core/src/common/FieldData.h index 334a46190f02e..cdd2b735464a2 100644 --- a/internal/core/src/common/FieldData.h +++ b/internal/core/src/common/FieldData.h @@ -18,6 +18,7 @@ #include #include +#include #include @@ -102,7 +103,7 @@ class FieldData : public FieldDataImpl { } int64_t - get_dim() const { + get_dim() const override { return binary_dim_; } @@ -149,7 +150,9 @@ struct ArrowDataWrapper { ArrowDataWrapper(std::shared_ptr reader, std::shared_ptr arrow_reader, std::shared_ptr file_data) - : reader(reader), arrow_reader(arrow_reader), file_data(file_data) { + : reader(std::move(reader)), + arrow_reader(std::move(arrow_reader)), + file_data(std::move(file_data)) { } std::shared_ptr reader; // file reader must outlive the record batch reader diff --git a/internal/core/src/exec/expression/Expr.cpp b/internal/core/src/exec/expression/Expr.cpp index 690c0e490dca5..445aa4c15278a 100644 --- a/internal/core/src/exec/expression/Expr.cpp +++ b/internal/core/src/exec/expression/Expr.cpp @@ -151,11 +151,11 @@ CompileExpression(const expr::TypedExprPtr& expr, bool enable_constant_folding) { ExprPtr result; - auto result_type = expr->type(); auto compiled_inputs = CompileInputs(expr, context, flatten_candidates); auto GetTypes = [](const std::vector& exprs) { std::vector types; + types.reserve(exprs.size()); for (auto& expr : exprs) { types.push_back(expr->type()); } diff --git a/internal/core/src/exec/expression/Expr.h b/internal/core/src/exec/expression/Expr.h index 8314609ddce4f..f2bbc8cd7f6bb 100644 --- a/internal/core/src/exec/expression/Expr.h +++ b/internal/core/src/exec/expression/Expr.h @@ -382,7 +382,6 @@ class SegmentExpr : public Expr { conditional_t, std::string, T> IndexInnerType; using Index = index::ScalarIndex; - int64_t processed_size = 0; const Index& index = segment_->chunk_scalar_index(field_id_, 0); auto* index_ptr = const_cast(&index); diff --git a/internal/core/src/exec/expression/UnaryExpr.cpp b/internal/core/src/exec/expression/UnaryExpr.cpp index 0b446f6df7f54..725af0854049a 100644 --- a/internal/core/src/exec/expression/UnaryExpr.cpp +++ b/internal/core/src/exec/expression/UnaryExpr.cpp @@ -259,9 +259,6 @@ PhyUnaryRangeFilterExpr::Eval(EvalCtx& context, VectorPtr& result) { template VectorPtr PhyUnaryRangeFilterExpr::ExecRangeVisitorImplArray(OffsetVector* input) { - using GetType = std::conditional_t, - std::string_view, - ValueType>; auto real_batch_size = has_offset_input_ ? input->size() : GetNextBatchSize(); if (real_batch_size == 0) { @@ -896,7 +893,7 @@ template ColumnVectorPtr PhyUnaryRangeFilterExpr::PreCheckOverflow(OffsetVector* input) { if constexpr (std::is_integral_v && !std::is_same_v) { - int64_t val = GetValueFromProto(expr_->val_); + auto val = GetValueFromProto(expr_->val_); if (milvus::query::out_of_range(val)) { int64_t batch_size; diff --git a/internal/core/src/exec/expression/UnaryExpr.h b/internal/core/src/exec/expression/UnaryExpr.h index ee8593a1fd251..159fe5abb4091 100644 --- a/internal/core/src/exec/expression/UnaryExpr.h +++ b/internal/core/src/exec/expression/UnaryExpr.h @@ -35,12 +35,12 @@ namespace exec { template struct UnaryElementFuncForMatch { - typedef std:: - conditional_t, std::string, T> - IndexInnerType; + using IndexInnerType = + std::conditional_t, std::string, T>; void operator()(const T* src, + size_t size, IndexInnerType val, TargetBitmapView res, @@ -60,9 +60,8 @@ struct UnaryElementFuncForMatch { template struct UnaryElementFunc { - typedef std:: - conditional_t, std::string, T> - IndexInnerType; + using IndexInnerType = + std::conditional_t, std::string, T>; void operator()(const T* src, @@ -235,9 +234,8 @@ struct UnaryElementFuncForArray { template struct UnaryIndexFuncForMatch { - typedef std:: - conditional_t, std::string, T> - IndexInnerType; + using IndexInnerType = + std::conditional_t, std::string, T>; using Index = index::ScalarIndex; TargetBitmap operator()(Index* index, IndexInnerType val) { @@ -275,9 +273,8 @@ struct UnaryIndexFuncForMatch { template struct UnaryIndexFunc { - typedef std:: - conditional_t, std::string, T> - IndexInnerType; + using IndexInnerType = + std::conditional_t, std::string, T>; using Index = index::ScalarIndex; TargetBitmap operator()(Index* index, IndexInnerType val) { diff --git a/internal/core/src/exec/operator/groupby/SearchGroupByOperator.h b/internal/core/src/exec/operator/groupby/SearchGroupByOperator.h index f05c42c22dbfe..80116de12be3a 100644 --- a/internal/core/src/exec/operator/groupby/SearchGroupByOperator.h +++ b/internal/core/src/exec/operator/groupby/SearchGroupByOperator.h @@ -118,11 +118,11 @@ template static const std::shared_ptr> GetDataGetter(const segcore::SegmentInternalInterface& segment, FieldId fieldId) { - if (const segcore::SegmentGrowingImpl* growing_segment = + if (const auto* growing_segment = dynamic_cast(&segment)) { return std::make_shared>(*growing_segment, fieldId); - } else if (const segcore::SegmentSealed* sealed_segment = + } else if (const auto* sealed_segment = dynamic_cast(&segment)) { return std::make_shared>(*sealed_segment, fieldId); } else { diff --git a/internal/core/src/mmap/ChunkData.h b/internal/core/src/mmap/ChunkData.h index d6a097e62d366..ec70d94ab6423 100644 --- a/internal/core/src/mmap/ChunkData.h +++ b/internal/core/src/mmap/ChunkData.h @@ -95,7 +95,6 @@ VariableLengthChunk::set(const std::string* src, uint32_t begin, uint32_t length) { auto mcm = storage::MmapManager::GetInstance().GetMmapChunkManager(); - milvus::ErrorCode err_code; AssertInfo( begin + length <= size_, "failed to set a chunk with length: {} from beign {}, map_size={}", @@ -126,7 +125,6 @@ VariableLengthChunk>::set( uint32_t begin, uint32_t length) { auto mcm = storage::MmapManager::GetInstance().GetMmapChunkManager(); - milvus::ErrorCode err_code; AssertInfo( begin + length <= size_, "failed to set a chunk with length: {} from beign {}, map_size={}", @@ -157,7 +155,6 @@ VariableLengthChunk::set(const Json* src, uint32_t begin, uint32_t length) { auto mcm = storage::MmapManager::GetInstance().GetMmapChunkManager(); - milvus::ErrorCode err_code; AssertInfo( begin + length <= size_, "failed to set a chunk with length: {} from beign {}, map_size={}", @@ -187,7 +184,6 @@ VariableLengthChunk::set(const Array* src, uint32_t begin, uint32_t length) { auto mcm = storage::MmapManager::GetInstance().GetMmapChunkManager(); - milvus::ErrorCode err_code; AssertInfo( begin + length <= size_, "failed to set a chunk with length: {} from beign {}, map_size={}", diff --git a/internal/core/src/mmap/ChunkedColumn.h b/internal/core/src/mmap/ChunkedColumn.h index e2924147428f7..91b3f9530b4ea 100644 --- a/internal/core/src/mmap/ChunkedColumn.h +++ b/internal/core/src/mmap/ChunkedColumn.h @@ -66,7 +66,7 @@ class ChunkedColumnBase : public ColumnBase { PanicInfo(ErrorCode::Unsupported, "AppendBatch not supported"); } - virtual const char* + const char* Data(int chunk_id) const override { return chunks_[chunk_id]->Data(); } @@ -125,7 +125,7 @@ class ChunkedColumnBase : public ColumnBase { chunks_.push_back(chunk); } - virtual size_t + size_t DataByteSize() const override { auto size = 0; for (auto& chunk : chunks_) { @@ -236,10 +236,11 @@ class ChunkedColumn : public ChunkedColumnBase { public: ChunkedColumn() = default; // memory mode ctor - ChunkedColumn(const FieldMeta& field_meta) : ChunkedColumnBase(field_meta) { + explicit ChunkedColumn(const FieldMeta& field_meta) + : ChunkedColumnBase(field_meta) { } - ChunkedColumn(std::vector> chunks) { + explicit ChunkedColumn(const std::vector>& chunks) { for (auto& chunk : chunks) { AddChunk(chunk); } @@ -247,7 +248,7 @@ class ChunkedColumn : public ChunkedColumnBase { ~ChunkedColumn() override = default; - virtual SpanBase + SpanBase Span(int64_t chunk_id) const override { return std::dynamic_pointer_cast(chunks_[chunk_id]) ->Span(); @@ -258,11 +259,12 @@ class ChunkedColumn : public ChunkedColumnBase { class ChunkedSparseFloatColumn : public ChunkedColumnBase { public: // memory mode ctor - ChunkedSparseFloatColumn(const FieldMeta& field_meta) + explicit ChunkedSparseFloatColumn(const FieldMeta& field_meta) : ChunkedColumnBase(field_meta) { } - ChunkedSparseFloatColumn(std::vector> chunks) { + explicit ChunkedSparseFloatColumn( + const std::vector>& chunks) { for (auto& chunk : chunks) { AddChunk(chunk); } @@ -311,11 +313,12 @@ class ChunkedVariableColumn : public ChunkedColumnBase { std::conditional_t, std::string_view, T>; // memory mode ctor - ChunkedVariableColumn(const FieldMeta& field_meta) + explicit ChunkedVariableColumn(const FieldMeta& field_meta) : ChunkedColumnBase(field_meta) { } - ChunkedVariableColumn(std::vector> chunks) { + explicit ChunkedVariableColumn( + const std::vector>& chunks) { for (auto& chunk : chunks) { AddChunk(chunk); } @@ -388,11 +391,12 @@ class ChunkedVariableColumn : public ChunkedColumnBase { class ChunkedArrayColumn : public ChunkedColumnBase { public: // memory mode ctor - ChunkedArrayColumn(const FieldMeta& field_meta) + explicit ChunkedArrayColumn(const FieldMeta& field_meta) : ChunkedColumnBase(field_meta) { } - ChunkedArrayColumn(std::vector> chunks) { + explicit ChunkedArrayColumn( + const std::vector>& chunks) { for (auto& chunk : chunks) { AddChunk(chunk); } diff --git a/internal/core/src/mmap/Column.h b/internal/core/src/mmap/Column.h index 59f48ef608b80..413bdcf89c8fa 100644 --- a/internal/core/src/mmap/Column.h +++ b/internal/core/src/mmap/Column.h @@ -698,7 +698,7 @@ class SingleChunkVariableColumn : public SingleChunkColumnBase { uint32_t size; size = *reinterpret_cast(pos); pos += sizeof(uint32_t); - res.emplace_back(std::string_view(pos, size)); + res.emplace_back(pos, size); pos += size; } return std::make_pair(res, valid_data_); @@ -710,9 +710,9 @@ class SingleChunkVariableColumn : public SingleChunkColumnBase { FixedVector valid; res.reserve(offsets.size()); valid.reserve(offsets.size()); - for (size_t i = 0; i < offsets.size(); ++i) { - res.emplace_back(RawAt(offsets[i])); - valid.emplace_back(IsValid(offsets[i])); + for (int offset : offsets) { + res.emplace_back(RawAt(offset)); + valid.emplace_back(IsValid(offset)); } return {res, valid}; } diff --git a/internal/core/src/segcore/ChunkedSegmentSealedImpl.cpp b/internal/core/src/segcore/ChunkedSegmentSealedImpl.cpp index 3caced553914f..56ba3fd074927 100644 --- a/internal/core/src/segcore/ChunkedSegmentSealedImpl.cpp +++ b/internal/core/src/segcore/ChunkedSegmentSealedImpl.cpp @@ -89,7 +89,6 @@ void ChunkedSegmentSealedImpl::LoadVecIndex(const LoadIndexInfo& info) { // NOTE: lock only when data is ready to avoid starvation auto field_id = FieldId(info.field_id); - auto& field_meta = schema_->operator[](field_id); AssertInfo(info.index_params.count("metric_type"), "Can't get metric_type in index_params"); @@ -355,11 +354,6 @@ ChunkedSegmentSealedImpl::LoadFieldData(FieldId field_id, FieldDataInfo& data) { // Don't allow raw data and index exist at the same time // AssertInfo(!get_bit(index_ready_bitset_, field_id), // "field data can't be loaded when indexing exists"); - auto get_block_size = [&]() -> size_t { - return schema_->get_primary_field_id() == field_id - ? DEFAULT_PK_VRCOL_BLOCK_SIZE - : DEFAULT_MEM_VRCOL_BLOCK_SIZE; - }; std::shared_ptr column{}; if (IsVariableDataType(data_type)) { @@ -535,7 +529,6 @@ ChunkedSegmentSealedImpl::MapFieldData(const FieldId field_id, auto data_type = field_meta.get_data_type(); // write the field data to disk - uint64_t total_written = 0; std::vector indices{}; std::vector> element_indices{}; // FixedVector valid_data{}; @@ -743,7 +736,6 @@ ChunkedSegmentSealedImpl::get_chunk_buffer(FieldId field_id, std::shared_lock lck(mutex_); AssertInfo(get_bit(field_data_ready_bitset_, field_id), "Can't get bitset element at " + std::to_string(field_id.get())); - auto& field_meta = schema_->operator[](field_id); if (auto it = fields_.find(field_id); it != fields_.end()) { auto& field_data = it->second; FixedVector valid_data; @@ -1025,8 +1017,8 @@ ChunkedSegmentSealedImpl::get_vector(FieldId field_id, ReadFromChunkCache, cc, data_path, mmap_descriptor_, field_meta)); } - for (int i = 0; i < futures.size(); ++i) { - const auto& [data_path, column] = futures[i].get(); + for (auto& future : futures) { + const auto& [data_path, column] = future.get(); path_to_column[data_path] = column; } @@ -1089,7 +1081,6 @@ ChunkedSegmentSealedImpl::DropFieldData(const FieldId field_id) { } lck.unlock(); } else { - auto& field_meta = schema_->operator[](field_id); std::unique_lock lck(mutex_); if (get_bit(field_data_ready_bitset_, field_id)) { fields_.erase(field_id); @@ -1776,7 +1767,6 @@ ChunkedSegmentSealedImpl::bulk_subscript( int64_t count, const std::vector& dynamic_field_names) const { Assert(!dynamic_field_names.empty()); - auto& field_meta = schema_->operator[](field_id); if (count == 0) { return fill_with_empty(field_id, 0); }