Switch VectorVal BroValUnion to store std::vector<IntrusivePtr<Val>>

This changes the return type of AsVector() from std::vector<Val*>*

NEWS

@@ -92,6 +92,8 @@ Changed Functionality
 - ``AsRecord()`` and ``AsNonConstRecord()`` have changed to return
   ``std::vector<IntrusivePtr<Val>>*``.
 
+- ``AsVector()`` has changed to return ``std::vector<IntrusivePtr<Val>>*``.
+
 Removed Functionality
 ---------------------
 

src/Val.cc

@@ -3040,14 +3040,11 @@ VectorVal::VectorVal(VectorType* t) : VectorVal({NewRef{}, t})
 
 VectorVal::VectorVal(IntrusivePtr<VectorType> t) : Val(std::move(t))
 	{
-	val.vector_val = new vector<Val*>();
+	val.vector_val = new vector<IntrusivePtr<Val>>();
 	}
 
 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<Val> element)
 	     ! same_type(element->GetType().get(), GetType()->AsVectorType()->Yield().get(), false) )
 		return false;
 
-	Val* val_at_index = nullptr;
+	if ( index >= val.vector_val->size() )
-
-	if ( index < val.vector_val->size() )
-		val_at_index = (*val.vector_val)[index];
-	else
 		val.vector_val->resize(index + 1);
 
-	Unref(val_at_index);
+	(*val.vector_val)[index] = std::move(element);
-
-	// 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();
 
 	Modified();
 	return true;
@@ -3100,17 +3088,14 @@ bool VectorVal::Insert(unsigned int index, IntrusivePtr<Val> element)
 		return false;
 		}
 
-	vector<Val*>::iterator it;
+	vector<IntrusivePtr<Val>>::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.
+	val.vector_val->insert(it, std::move(element));
-	// AssignExpr::Eval() already does this; other callers must remember
-	// to do it similarly.
-	val.vector_val->insert(it, element.release());
 
 	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<Val> 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;

src/Val.h

@@ -81,7 +81,7 @@ union BroValUnion {
 	RE_Matcher* re_val;
 	PDict<TableEntryVal>* table_val;
 	std::vector<IntrusivePtr<Val>>* record_val;
-	std::vector<Val*>* vector_val;
+	std::vector<IntrusivePtr<Val>>* vector_val;
 
 	BroValUnion() = default;
 
@@ -114,9 +114,6 @@ union BroValUnion {
 
 	constexpr BroValUnion(PDict<TableEntryVal>* value) noexcept
 		: table_val(value) {}
-
-	constexpr BroValUnion(std::vector<Val*> *value) noexcept
-		: vector_val(value) {}
 };
 
 class Val : public BroObj {
@@ -221,7 +218,7 @@ public:
 	CONST_ACCESSOR(TYPE_RECORD, std::vector<IntrusivePtr<Val>>*, 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<Val*>*, vector_val, AsVector)
+	CONST_ACCESSOR(TYPE_VECTOR, std::vector<IntrusivePtr<Val>>*, 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<Val*>*, vector_val, AsVector)
+	ACCESSOR(TYPE_VECTOR, std::vector<IntrusivePtr<Val>>*, vector_val, AsVector)
 
 	IntrusivePtr<Func> AsFuncPtr() const;
 

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<Val*>* idx_v = idx->AsVector();
+	auto idx_v = idx->AsVector();
 	BroString::IdxVec indices(idx_v->size());
 	unsigned int i;
 

src/zeek.bif

@@ -1327,10 +1327,10 @@ function all_set%(v: any%) : bool
 	%}
 
 %%{
-static Func* sort_function_comp = 0;
+static Func* sort_function_comp = nullptr;
-static Val** index_map = 0;	// used for indirect sorting to support order()
+static std::vector<const IntrusivePtr<Val>*> index_map;	// used for indirect sorting to support order()
 
-bool sort_function(Val* a, Val* b)
+bool sort_function(const IntrusivePtr<Val>& a, const IntrusivePtr<Val>& 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},
+	auto result = sort_function_comp->operator()(a, b);
-	                                             IntrusivePtr{NewRef{}, 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<Val>& a, const IntrusivePtr<Val>& 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<Val>& a, const IntrusivePtr<Val>& 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<Val*>& 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<Val*>& vv = *v->AsVector();
+	auto& vv = *v->AsVector();
 	auto n = vv.size();
 
 	// Set up initial mapping of indices directly to corresponding
 	// elements.
 	vector<size_t> 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 = {};
-	index_map = 0;
 
 	// Now spin through ind_vv to read out the rearrangement.
 	for ( i = 0; i < n; ++i )