switched RecordVal's to use std::optional for tracking missing fields

src/Val.cc

@@ -2846,11 +2846,9 @@ RecordVal::RecordVal(RecordTypePtr t, bool init_fields)
 
 	int n = rt->NumFields();
 
-	record_val = new std::vector<ZVal>;
+	record_val = new std::vector<std::optional<ZVal>>;
 	record_val->reserve(n);
 
-	is_in_record = new std::vector<bool>(n, false);
-
 	if ( run_state::is_parsing )
 		parse_time_records[rt.get()].emplace_back(NewRef{}, this);
 
@@ -2876,7 +2874,6 @@ RecordVal::RecordVal(RecordTypePtr t, bool init_fields)
 					parse_time_records[rt.get()].pop_back();
 
 				delete record_val;
-				delete is_in_record;
 				throw;
 				}
 
@@ -2908,15 +2905,9 @@ RecordVal::RecordVal(RecordTypePtr t, bool init_fields)
 			}
 
 		if ( def )
-			{
 			record_val->emplace_back(ZVal(def, def->GetType()));
-			(*is_in_record)[i] = true;
-			}
 		else
-			{
+			record_val->emplace_back(std::nullopt);
-			record_val->emplace_back(ZVal());
-			(*is_in_record)[i] = false;
-			}
 		}
 	}
 
@@ -2926,10 +2917,9 @@ RecordVal::~RecordVal()
 
 	for ( unsigned int i = 0; i < n; ++i )
 		if ( HasField(i) && IsManaged(i) )
-			ZVal::DeleteManagedType((*record_val)[i]);
+			ZVal::DeleteManagedType(*(*record_val)[i]);
 
 	delete record_val;
-	delete is_in_record;
 	}
 
 ValPtr RecordVal::SizeVal() const
@@ -2945,7 +2935,6 @@ void RecordVal::Assign(int field, ValPtr new_val)
 
 		auto t = rt->GetFieldType(field);
 		(*record_val)[field] = ZVal(new_val, t);
-		(*is_in_record)[field] = true;
 		Modified();
 		}
 	else
@@ -2957,10 +2946,9 @@ void RecordVal::Remove(int field)
 	if ( HasField(field) )
 		{
 		if ( IsManaged(field) )
-			ZVal::DeleteManagedType((*record_val)[field]);
+			ZVal::DeleteManagedType(*(*record_val)[field]);
 
-		(*record_val)[field] = ZVal();
+		(*record_val)[field] = std::nullopt;
-		(*is_in_record)[field] = false;
 
 		Modified();
 		}
@@ -2992,8 +2980,6 @@ void RecordVal::ResizeParseTimeRecords(RecordType* revised_rt)
 
 		if ( required_length > current_length )
 			{
-			rv->Reserve(required_length);
-
 			for ( auto i = current_length; i < required_length; ++i )
 				rv->AppendField(revised_rt->FieldDefault(i));
 			}
@@ -3203,13 +3189,6 @@ unsigned int RecordVal::MemoryAllocation() const
 	size += util::pad_size(record_val->capacity() * sizeof(ZVal));
 	size += padded_sizeof(*record_val);
 
-	// It's tricky sizing is_in_record since it's a std::vector
-	// specialization.  We approximate this by not scaling capacity()
-	// by sizeof(bool) but just using its raw value.  That's still
-	// presumably going to be an overestimate.
-	size += util::pad_size(is_in_record->capacity());
-	size += padded_sizeof(*is_in_record);
-
 	return size + padded_sizeof(*this);
 	}
 

src/Val.h

@@ -1103,13 +1103,13 @@ public:
 	// The following provide efficient record field assignments.
 	void Assign(int field, bool new_val)
 		{
-		(*record_val)[field].int_val = int(new_val);
+		(*record_val)[field] = ZVal(bro_int_t(new_val));
 		AddedField(field);
 		}
 
 	void Assign(int field, int new_val)
 		{
-		(*record_val)[field].int_val = new_val;
+		(*record_val)[field] = ZVal(bro_int_t(new_val));
 		AddedField(field);
 		}
 
@@ -1118,18 +1118,18 @@ public:
 	// than the other.
 	void Assign(int field, uint32_t new_val)
 		{
-		(*record_val)[field].uint_val = new_val;
+		(*record_val)[field] = ZVal(bro_uint_t(new_val));
 		AddedField(field);
 		}
 	void Assign(int field, uint64_t new_val)
 		{
-		(*record_val)[field].uint_val = new_val;
+		(*record_val)[field] = ZVal(bro_uint_t(new_val));
 		AddedField(field);
 		}
 
 	void Assign(int field, double new_val)
 		{
-		(*record_val)[field].double_val = new_val;
+		(*record_val)[field] = ZVal(new_val);
 		AddedField(field);
 		}
 
@@ -1144,8 +1144,9 @@ public:
 
 	void Assign(int field, StringVal* new_val)
 		{
-		ZVal::DeleteManagedType((*record_val)[field]);
+		if ( HasField(field) )
-		(*record_val)[field].string_val = new_val;
+			ZVal::DeleteManagedType(*(*record_val)[field]);
+		(*record_val)[field] = ZVal(new_val);
 		AddedField(field);
 		}
 	void Assign(int field, const char* new_val)
@@ -1177,29 +1178,9 @@ public:
 	void AppendField(ValPtr v)
 		{
 		if ( v )
-			{
-			(*is_in_record)[record_val->size()] = true;
 			record_val->emplace_back(ZVal(v, v->GetType()));
-			}
 		else
-			{
+			record_val->emplace_back(std::nullopt);
-			(*is_in_record)[record_val->size()] = false;
-			record_val->emplace_back(ZVal());
-			}
-		}
-
-	/**
-	 * Ensures that the record has enough internal storage for the
-	 * given number of fields.
-	 * @param n  The number of fields.
-	 */
-	void Reserve(unsigned int n)
-		{
-		record_val->reserve(n);
-		is_in_record->reserve(n);
-
-		for ( auto i = is_in_record->size(); i < n; ++i )
-			is_in_record->emplace_back(false);
 		}
 
 	/**
@@ -1217,7 +1198,7 @@ public:
 	 */
 	bool HasField(int field) const
 		{
-		return (*is_in_record)[field];
+		return (*record_val)[field] ? true : false;
 		}
 
 	/**
@@ -1230,7 +1211,7 @@ public:
 		if ( ! HasField(field) )
 			return nullptr;
 
-		return (*record_val)[field].ToVal(rt->GetFieldType(field));
+		return (*record_val)[field]->ToVal(rt->GetFieldType(field));
 		}
 
 	/**
@@ -1304,33 +1285,33 @@ public:
 		if constexpr ( std::is_same_v<T, BoolVal> ||
 		               std::is_same_v<T, IntVal> ||
 		               std::is_same_v<T, EnumVal> )
-			return record_val->operator[](field).int_val;
+			return record_val->operator[](field)->int_val;
 		else if constexpr ( std::is_same_v<T, CountVal> )
-			return record_val->operator[](field).uint_val;
+			return record_val->operator[](field)->uint_val;
 		else if constexpr ( std::is_same_v<T, DoubleVal> ||
 		                    std::is_same_v<T, TimeVal> ||
 		                    std::is_same_v<T, IntervalVal> )
-			return record_val->operator[](field).double_val;
+			return record_val->operator[](field)->double_val;
 		else if constexpr ( std::is_same_v<T, PortVal> )
-			return val_mgr->Port(record_val->at(field).uint_val);
+			return val_mgr->Port(record_val->at(field)->uint_val);
 		else if constexpr ( std::is_same_v<T, StringVal> )
-			return record_val->operator[](field).string_val->Get();
+			return record_val->operator[](field)->string_val->Get();
 		else if constexpr ( std::is_same_v<T, AddrVal> )
-			return record_val->operator[](field).addr_val->Get();
+			return record_val->operator[](field)->addr_val->Get();
 		else if constexpr ( std::is_same_v<T, SubNetVal> )
-			return record_val->operator[](field).subnet_val->Get();
+			return record_val->operator[](field)->subnet_val->Get();
 		else if constexpr ( std::is_same_v<T, File> )
-			return *(record_val->operator[](field).file_val);
+			return *(record_val->operator[](field)->file_val);
 		else if constexpr ( std::is_same_v<T, Func> )
-			return *(record_val->operator[](field).func_val);
+			return *(record_val->operator[](field)->func_val);
 		else if constexpr ( std::is_same_v<T, PatternVal> )
-			return record_val->operator[](field).re_val->Get();
+			return record_val->operator[](field)->re_val->Get();
 		else if constexpr ( std::is_same_v<T, RecordVal> )
-			return record_val->operator[](field).record_val;
+			return record_val->operator[](field)->record_val;
 		else if constexpr ( std::is_same_v<T, VectorVal> )
-			return record_val->operator[](field).vector_val;
+			return record_val->operator[](field)->vector_val;
 		else if constexpr ( std::is_same_v<T, TableVal> )
-			return record_val->operator[](field).table_val->Get();
+			return record_val->operator[](field)->table_val->Get();
 		else
 			{
 			// It's an error to reach here, although because of
@@ -1345,12 +1326,12 @@ public:
 	T GetFieldAs(int field) const
 		{
 		if constexpr ( std::is_integral_v<T> && std::is_signed_v<T> )
-			return record_val->operator[](field).int_val;
+			return record_val->operator[](field)->int_val;
 		else if constexpr ( std::is_integral_v<T> &&
 					std::is_unsigned_v<T> )
-			return record_val->operator[](field).uint_val;
+			return record_val->operator[](field)->uint_val;
 		else if constexpr ( std::is_floating_point_v<T> )
-			return record_val->operator[](field).double_val;
+			return record_val->operator[](field)->double_val;
 
 		// Note: we could add other types here using type traits,
 		// such as is_same_v<T, std::string>, etc.
@@ -1415,7 +1396,6 @@ protected:
 
 	void AddedField(int field)
 		{
-		(*is_in_record)[field] = true;
 		Modified();
 		}
 
@@ -1428,7 +1408,7 @@ private:
 	void DeleteFieldIfManaged(unsigned int field)
 		{
 		if ( HasField(field) && IsManaged(field) )
-			ZVal::DeleteManagedType((*record_val)[field]);
+			ZVal::DeleteManagedType(*(*record_val)[field]);
 		}
 
 	bool IsManaged(unsigned int offset) const
@@ -1441,12 +1421,7 @@ private:
 	RecordTypePtr rt;
 
 	// Low-level values of each of the fields.
-	std::vector<ZVal>* record_val;
+	std::vector<std::optional<ZVal>>* record_val;
-
-	// Whether a given field exists - for optional fields, and because
-	// Zeek does not enforce that non-optional fields are actually
-	// present.
-	std::vector<bool>* is_in_record;
 
 	// Whether a given field requires explicit memory management.
 	const std::vector<bool>& is_managed;