From 69533bcbc62ea4850f788e092332d93046ced033 Mon Sep 17 00:00:00 2001 From: Jon Siwek Date: Thu, 21 May 2020 16:24:34 -0700 Subject: [PATCH] Switch VectorVal BroValUnion to store std::vector> This changes the return type of AsVector() from std::vector* --- NEWS | 2 ++ src/Val.cc | 31 +++++++------------------------ src/Val.h | 9 +++------ src/strings.bif | 2 +- src/zeek.bif | 30 ++++++++++++++---------------- 5 files changed, 27 insertions(+), 47 deletions(-) diff --git a/NEWS b/NEWS index 5f4f479c6f..40206022cb 100644 --- a/NEWS +++ b/NEWS @@ -92,6 +92,8 @@ Changed Functionality - ``AsRecord()`` and ``AsNonConstRecord()`` have changed to return ``std::vector>*``. +- ``AsVector()`` has changed to return ``std::vector>*``. + Removed Functionality --------------------- diff --git a/src/Val.cc b/src/Val.cc index d1c914e951..8b150d18b3 100644 --- a/src/Val.cc +++ b/src/Val.cc @@ -3040,14 +3040,11 @@ VectorVal::VectorVal(VectorType* t) : VectorVal({NewRef{}, t}) VectorVal::VectorVal(IntrusivePtr t) : Val(std::move(t)) { - val.vector_val = new vector(); + val.vector_val = new vector>(); } VectorVal::~VectorVal() { - for ( unsigned int i = 0; i < val.vector_val->size(); ++i ) - Unref((*val.vector_val)[i]); - delete val.vector_val; } @@ -3062,19 +3059,10 @@ bool VectorVal::Assign(unsigned int index, IntrusivePtr element) ! same_type(element->GetType().get(), GetType()->AsVectorType()->Yield().get(), false) ) return false; - Val* val_at_index = nullptr; - - if ( index < val.vector_val->size() ) - val_at_index = (*val.vector_val)[index]; - else + if ( index >= val.vector_val->size() ) val.vector_val->resize(index + 1); - Unref(val_at_index); - - // Note: we do *not* Ref() the element, if any, at this point. - // AssignExpr::Eval() already does this; other callers must remember - // to do it similarly. - (*val.vector_val)[index] = element.release(); + (*val.vector_val)[index] = std::move(element); Modified(); return true; @@ -3100,17 +3088,14 @@ bool VectorVal::Insert(unsigned int index, IntrusivePtr element) return false; } - vector::iterator it; + vector>::iterator it; if ( index < val.vector_val->size() ) it = std::next(val.vector_val->begin(), index); else it = val.vector_val->end(); - // Note: we do *not* Ref() the element, if any, at this point. - // AssignExpr::Eval() already does this; other callers must remember - // to do it similarly. - val.vector_val->insert(it, element.release()); + val.vector_val->insert(it, std::move(element)); Modified(); return true; @@ -3121,10 +3106,8 @@ bool VectorVal::Remove(unsigned int index) if ( index >= val.vector_val->size() ) return false; - Val* val_at_index = (*val.vector_val)[index]; auto it = std::next(val.vector_val->begin(), index); val.vector_val->erase(it); - Unref(val_at_index); Modified(); return true; @@ -3159,7 +3142,7 @@ Val* VectorVal::Lookup(unsigned int index) const if ( index >= val.vector_val->size() ) return nullptr; - return (*val.vector_val)[index]; + return (*val.vector_val)[index].get(); } unsigned int VectorVal::Resize(unsigned int new_num_elements) @@ -3188,7 +3171,7 @@ IntrusivePtr VectorVal::DoClone(CloneState* state) for ( unsigned int i = 0; i < val.vector_val->size(); ++i ) { auto v = (*val.vector_val)[i]->Clone(state); - vv->val.vector_val->push_back(v.release()); + vv->val.vector_val->push_back(std::move(v)); } return vv; diff --git a/src/Val.h b/src/Val.h index 1a0a7de7e6..b7248f217a 100644 --- a/src/Val.h +++ b/src/Val.h @@ -81,7 +81,7 @@ union BroValUnion { RE_Matcher* re_val; PDict* table_val; std::vector>* record_val; - std::vector* vector_val; + std::vector>* vector_val; BroValUnion() = default; @@ -114,9 +114,6 @@ union BroValUnion { constexpr BroValUnion(PDict* value) noexcept : table_val(value) {} - - constexpr BroValUnion(std::vector *value) noexcept - : vector_val(value) {} }; class Val : public BroObj { @@ -221,7 +218,7 @@ public: CONST_ACCESSOR(TYPE_RECORD, std::vector>*, record_val, AsRecord) CONST_ACCESSOR(TYPE_FILE, BroFile*, file_val, AsFile) CONST_ACCESSOR(TYPE_PATTERN, RE_Matcher*, re_val, AsPattern) - CONST_ACCESSOR(TYPE_VECTOR, std::vector*, vector_val, AsVector) + CONST_ACCESSOR(TYPE_VECTOR, std::vector>*, vector_val, AsVector) const IPPrefix& AsSubNet() const { @@ -255,7 +252,7 @@ public: ACCESSOR(TYPE_FUNC, Func*, func_val, AsFunc) ACCESSOR(TYPE_FILE, BroFile*, file_val, AsFile) ACCESSOR(TYPE_PATTERN, RE_Matcher*, re_val, AsPattern) - ACCESSOR(TYPE_VECTOR, std::vector*, vector_val, AsVector) + ACCESSOR(TYPE_VECTOR, std::vector>*, vector_val, AsVector) IntrusivePtr AsFuncPtr() const; diff --git a/src/strings.bif b/src/strings.bif index bfc5853bc7..b22fb894c6 100644 --- a/src/strings.bif +++ b/src/strings.bif @@ -705,7 +705,7 @@ function str_smith_waterman%(s1: string, s2: string, params: sw_params%) : sw_su ## .. zeek:see:: split_string split_string1 split_string_all split_string_n function str_split%(s: string, idx: index_vec%): string_vec %{ - vector* idx_v = idx->AsVector(); + auto idx_v = idx->AsVector(); BroString::IdxVec indices(idx_v->size()); unsigned int i; diff --git a/src/zeek.bif b/src/zeek.bif index 51a95cbaf3..37f7af26d2 100644 --- a/src/zeek.bif +++ b/src/zeek.bif @@ -1327,10 +1327,10 @@ function all_set%(v: any%) : bool %} %%{ -static Func* sort_function_comp = 0; -static Val** index_map = 0; // used for indirect sorting to support order() +static Func* sort_function_comp = nullptr; +static std::vector*> index_map; // used for indirect sorting to support order() -bool sort_function(Val* a, Val* b) +bool sort_function(const IntrusivePtr& a, const IntrusivePtr& b) { // Sort missing values as "high". if ( ! a ) @@ -1338,8 +1338,7 @@ bool sort_function(Val* a, Val* b) if ( ! b ) return 1; - auto result = sort_function_comp->operator()(IntrusivePtr{NewRef{}, a}, - IntrusivePtr{NewRef{}, b}); + auto result = sort_function_comp->operator()(a, b); int int_result = result->CoerceToInt(); return int_result < 0; @@ -1347,10 +1346,10 @@ bool sort_function(Val* a, Val* b) bool indirect_sort_function(size_t a, size_t b) { - return sort_function(index_map[a], index_map[b]); + return sort_function(*index_map[a], *index_map[b]); } -bool signed_sort_function (Val* a, Val* b) +bool signed_sort_function (const IntrusivePtr& a, const IntrusivePtr& b) { if ( ! a ) return 0; @@ -1363,7 +1362,7 @@ bool signed_sort_function (Val* a, Val* b) return ia < ib; } -bool unsigned_sort_function (Val* a, Val* b) +bool unsigned_sort_function (const IntrusivePtr& a, const IntrusivePtr& b) { if ( ! a ) return 0; @@ -1378,12 +1377,12 @@ bool unsigned_sort_function (Val* a, Val* b) bool indirect_signed_sort_function(size_t a, size_t b) { - return signed_sort_function(index_map[a], index_map[b]); + return signed_sort_function(*index_map[a], *index_map[b]); } bool indirect_unsigned_sort_function(size_t a, size_t b) { - return unsigned_sort_function(index_map[a], index_map[b]); + return unsigned_sort_function(*index_map[a], *index_map[b]); } %%} @@ -1431,7 +1430,7 @@ function sort%(v: any, ...%) : any if ( ! comp && ! IsIntegral(elt_type->Tag()) ) builtin_error("comparison function required for sort() with non-integral types"); - vector& vv = *v->AsVector(); + auto& vv = *v->AsVector(); if ( comp ) { @@ -1501,18 +1500,18 @@ function order%(v: any, ...%) : index_vec if ( ! comp && ! IsIntegral(elt_type->Tag()) ) builtin_error("comparison function required for order() with non-integral types"); - vector& vv = *v->AsVector(); + auto& vv = *v->AsVector(); auto n = vv.size(); // Set up initial mapping of indices directly to corresponding // elements. vector ind_vv(n); - index_map = new Val*[n]; + index_map.reserve(n); size_t i; for ( i = 0; i < n; ++i ) { ind_vv[i] = i; - index_map[i] = vv[i]; + index_map.emplace_back(&vv[i]); } if ( comp ) @@ -1537,8 +1536,7 @@ function order%(v: any, ...%) : index_vec sort(ind_vv.begin(), ind_vv.end(), indirect_signed_sort_function); } - delete [] index_map; - index_map = 0; + index_map = {}; // Now spin through ind_vv to read out the rearrangement. for ( i = 0; i < n; ++i )