Fixing a problem with records having optional fields when used as

table/set indices.

This addresses #367. In principle, the fix is quite straightford.
However, it turns out that sometimes record fields lost their
attributes on assignment, and then the hashing can't decide anymore
whether a field is optional or not. So that needed to be fixed as
well.

src/Attr.cc

@@ -327,6 +327,41 @@ void Attributes::CheckAttr(Attr* a)
 	}
 	}
 
+bool Attributes::operator==(const Attributes& other) const
+	{
+	if ( ! attrs )
+		return other.attrs;
+
+	if ( ! other.attrs )
+		return false;
+
+	loop_over_list(*attrs, i)
+		{
+		Attr* a = (*attrs)[i];
+		Attr* o = other.FindAttr(a->Tag());
+
+		if ( ! o )
+			return false;
+
+		if ( ! (*a == *o) )
+			return false;
+		}
+
+	loop_over_list(*other.attrs, j)
+		{
+		Attr* o = (*other.attrs)[j];
+		Attr* a = FindAttr(o->Tag());
+
+		if ( ! a )
+			return false;
+
+		if ( ! (*a == *o) )
+			return false;
+		}
+
+	return true;
+	}
+
 bool Attributes::Serialize(SerialInfo* info) const
 	{
 	return SerialObj::Serialize(info);

src/Attr.h

@@ -52,6 +52,20 @@ public:
 
 	void Describe(ODesc* d) const;
 
+	bool operator==(const Attr& other) const
+		{
+		if ( tag != other.tag )
+			return false;
+
+		if ( expr || other.expr )
+			// If any has an expression and they aren't the same object, we
+			// declare them unequal, as we can't really find out if the two
+			// expressions are equivalent.
+			return (expr == other.expr);
+
+		return true;
+		}
+
 protected:
 	void AddTag(ODesc* d) const;
 
@@ -79,6 +93,8 @@ public:
 	bool Serialize(SerialInfo* info) const;
 	static Attributes* Unserialize(UnserialInfo* info);
 
+	bool operator==(const Attributes& other) const;
+
 protected:
 	Attributes()	{ type = 0; attrs = 0; }
 	void CheckAttr(Attr* attr);

src/CompHash.cc

@@ -65,11 +65,22 @@ CompositeHash::~CompositeHash()
 
 // Computes the piece of the hash for Val*, returning the new kp.
 char* CompositeHash::SingleValHash(int type_check, char* kp0,
-					BroType* bt, Val* v) const
+					BroType* bt, Val* v, bool optional) const
 	{
 	char* kp1 = 0;
 	InternalTypeTag t = bt->InternalType();
 
+	if ( optional )
+		{
+		// Add a marker saying whether the optional field is set.
+		char* kp = AlignAndPadType<char>(kp0);
+		*kp = ( v ? 1 : 0);
+		kp0 = reinterpret_cast<char*>(kp+1);
+
+		if ( ! v ) 
+			return kp0;
+		}
+
 	if ( type_check )
 		{
 		InternalTypeTag vt = v->Type()->InternalType();
@@ -163,12 +174,16 @@ char* CompositeHash::SingleValHash(int type_check, char* kp0,
 			for ( int i = 0; i < num_fields; ++i )
 				{
 				Val* rv_i = rv->Lookup(i);
-				if ( ! rv_i )
+
+				Attributes* a = rt->FieldDecl(i)->attrs;
+				bool optional = (a && a->FindAttr(ATTR_OPTIONAL));
+
+				if ( ! (rv_i || optional) )
 					return 0;
 
 				if ( ! (kp = SingleValHash(type_check, kp,
 							   rt->FieldType(i),
-							   rv_i)) )
+							   rv_i, optional)) )
 					return 0;
 				}
 
@@ -248,7 +263,7 @@ HashKey* CompositeHash::ComputeHash(const Val* v, int type_check) const
 	char* kp = k;
 	loop_over_list(*tl, i)
 		{
-		kp = SingleValHash(type_check, kp, (*tl)[i], (*vl)[i]);
+		kp = SingleValHash(type_check, kp, (*tl)[i], (*vl)[i], false);
 		if ( ! kp )
 			return 0;
 		}
@@ -315,10 +330,13 @@ HashKey* CompositeHash::ComputeSingletonHash(const Val* v, int type_check) const
 	}
 
 int CompositeHash::SingleTypeKeySize(BroType* bt, const Val* v,
-					int type_check, int sz) const
+					int type_check, int sz, bool optional) const
 	{
 	InternalTypeTag t = bt->InternalType();
 
+	if ( optional )
+		sz = SizeAlign(sz, sizeof(char));
+
 	if ( type_check && v )
 		{
 		InternalTypeTag vt = v->Type()->InternalType();
@@ -369,9 +387,12 @@ int CompositeHash::SingleTypeKeySize(BroType* bt, const Val* v,
 
 			for ( int i = 0; i < num_fields; ++i )
 				{
+				Attributes* a = rt->FieldDecl(i)->attrs;
+				bool optional = (a && a->FindAttr(ATTR_OPTIONAL));
+
 				sz = SingleTypeKeySize(rt->FieldType(i),
 							rv ? rv->Lookup(i) : 0,
-							type_check, sz);
+							type_check, sz, optional);
 				if ( ! sz )
 					return 0;
 				}
@@ -418,7 +439,7 @@ int CompositeHash::ComputeKeySize(const Val* v, int type_check) const
 	loop_over_list(*tl, i)
 		{
 		sz = SingleTypeKeySize((*tl)[i], v ? v->AsListVal()->Index(i) : 0,
-				       type_check, sz);
+				       type_check, sz, false);
 		if ( ! sz )
 			return 0;
 		}
@@ -495,20 +516,20 @@ ListVal* CompositeHash::RecoverVals(const HashKey* k) const
 	loop_over_list(*tl, i)
 		{
 		Val* v;
-		kp = RecoverOneVal(k, kp, k_end, (*tl)[i], v);
+		kp = RecoverOneVal(k, kp, k_end, (*tl)[i], v, false);
 		ASSERT(v);
 		l->Append(v);
 		}
 
 	if ( kp != k_end )
-		internal_error("under-ran key in CompositeHash::DescribeKey");
+		internal_error("under-ran key in CompositeHash::DescribeKey %ld", k_end - kp);
 
 	return l;
 	}
 
 const char* CompositeHash::RecoverOneVal(const HashKey* k, const char* kp0,
 					 const char* const k_end, BroType* t,
-					 Val*& pval) const
+					 Val*& pval, bool optional) const
 	{
 	// k->Size() == 0 for a single empty string.
 	if ( kp0 >= k_end && k->Size() > 0 )
@@ -516,9 +537,20 @@ const char* CompositeHash::RecoverOneVal(const HashKey* k, const char* kp0,
 
 	TypeTag tag = t->Tag();
 	InternalTypeTag it = t->InternalType();
-
 	const char* kp1 = 0;
 
+	if ( optional )
+		{
+		const char* kp = AlignType<char>(kp0);
+		kp0 = kp1 = reinterpret_cast<const char*>(kp+1);
+
+		if ( ! *kp )
+			{
+			pval = 0;
+			return kp0;
+			}
+		}
+
 	switch ( it ) {
 	case TYPE_INTERNAL_INT:
 		{
@@ -647,9 +679,13 @@ const char* CompositeHash::RecoverOneVal(const HashKey* k, const char* kp0,
 			for ( i = 0; i < num_fields; ++i )
 				{
 				Val* v;
+
+				Attributes* a = rt->FieldDecl(i)->attrs;
+				bool optional = (a && a->FindAttr(ATTR_OPTIONAL));
+
 				kp = RecoverOneVal(k, kp, k_end,
-				                   rt->FieldType(i), v);
+				                   rt->FieldType(i), v, optional);
-				if ( ! v )
+				if ( ! (v || optional) )
 					{
 					internal_error("didn't recover expected number of fields from HashKey");
 					pval = 0;

src/CompHash.h

@@ -30,7 +30,7 @@ protected:
 	// Computes the piece of the hash for Val*, returning the new kp.
 	// Used as a helper for ComputeHash in the non-singleton case.
 	char* SingleValHash(int type_check, char* kp,
-				BroType* bt, Val* v) const;
+				BroType* bt, Val* v, bool optional) const;
 
 	// Recovers just one Val of possibly many; called from RecoverVals.
 	// Upon return, pval will point to the recovered Val of type t.
@@ -38,7 +38,7 @@ protected:
 	// upon errors, so there is no return value for invalid input.
 	const char* RecoverOneVal(const HashKey* k,
 				  const char* kp, const char* const k_end,
-				  BroType* t, Val*& pval) const;
+				  BroType* t, Val*& pval, bool optional) const;
 
 	// Rounds the given pointer up to the nearest multiple of the
 	// given size, if not already a multiple.
@@ -77,7 +77,7 @@ protected:
 	int ComputeKeySize(const Val* v = 0, int type_check = 1) const;
 
 	int SingleTypeKeySize(BroType*, const Val*,
-				int type_check, int sz) const;
+				int type_check, int sz, bool optional) const;
 
 	TypeList* type;
 	char* key;	// space for composite key

src/Expr.cc

@@ -2531,16 +2531,35 @@ bool AssignExpr::TypeCheck()
 		return true;
 		}
 
+	if ( op1->Type()->Tag() == TYPE_RECORD &&
+		 op2->Type()->Tag() == TYPE_RECORD )
+		{
+		if ( same_type(op1->Type(), op2->Type()) )
+			{
+			RecordType* rt1 = op1->Type()->AsRecordType();
+			RecordType* rt2 = op2->Type()->AsRecordType();
+
+			// Make sure the attributes match as well.
+			for ( int i = 0; i < rt1->NumFields(); ++i )
+				{
+				const TypeDecl* td1 = rt1->FieldDecl(i);
+				const TypeDecl* td2 = rt2->FieldDecl(i);
+
+				if ( same_attrs(td1->attrs, td2->attrs) )
+					// Everything matches.
+					return true;
+				}
+			}
+
+		// Need to coerce.
+		op2 = new RecordCoerceExpr(op2, op1->Type()->AsRecordType());
+		return true;
+		}
+
 	if ( ! same_type(op1->Type(), op2->Type()) )
 		{
-		if ( op1->Type()->Tag() == TYPE_RECORD &&
+		ExprError("type clash in assignment");
-		     op2->Type()->Tag() == TYPE_RECORD )
+		return false;
-			op2 = new RecordCoerceExpr(op2, op1->Type()->AsRecordType());
-		else
-			{
-			ExprError("type clash in assignment");
-			return false;
-			}
 		}
 
 	return true;
@@ -5308,21 +5327,39 @@ int check_and_promote_expr(Expr*& e, BroType* t)
 		return 1;
 		}
 
-	else if ( ! same_type(t, et) )
+	if ( t->Tag() == TYPE_RECORD && et->Tag() == TYPE_RECORD )
 		{
-		if ( t->Tag() == TYPE_RECORD && et->Tag() == TYPE_RECORD )
+		RecordType* t_r = t->AsRecordType();
-			{
+		RecordType* et_r = et->AsRecordType();
-			RecordType* t_r = t->AsRecordType();
-			RecordType* et_r = et->AsRecordType();
 
-			if ( record_promotion_compatible(t_r, et_r) )
+		if ( same_type(t, et) )
+			{
+			// Make sure the attributes match as well.
+			for ( int i = 0; i < t_r->NumFields(); ++i )
 				{
-				e = new RecordCoerceExpr(e, t_r);
+				const TypeDecl* td1 = t_r->FieldDecl(i);
-				return 1;
+				const TypeDecl* td2 = et_r->FieldDecl(i);
+
+				if ( same_attrs(td1->attrs, td2->attrs) )
+					// Everything matches perfectly.
+					return 1;
 				}
 			}
 
-		else if ( t->Tag() == TYPE_TABLE && et->Tag() == TYPE_TABLE &&
+		if ( record_promotion_compatible(t_r, et_r) ) // Note: This is always true currently.
+			{
+			e = new RecordCoerceExpr(e, t_r);
+			return 1;
+			}
+
+		t->Error("incompatible record types", e);
+		return 0;
+		}
+
+
+	if ( ! same_type(t, et) )
+		{
+		if ( t->Tag() == TYPE_TABLE && et->Tag() == TYPE_TABLE &&
 			  et->AsTableType()->IsUnspecifiedTable() )
 			{
 			e = new TableCoerceExpr(e, t->AsTableType());

src/Type.cc

@@ -1430,6 +1430,17 @@ int same_type(const BroType* t1, const BroType* t2, int is_init)
 	return 0;
 	}
 
+int same_attrs(const Attributes* a1, const Attributes* a2)
+	{
+	if ( ! a1 )
+		return (a2 != 0);
+
+	if ( ! a2 )
+		return 0;
+
+	return (*a1 == *a2);
+	}
+
 int record_promotion_compatible(const RecordType* /* super_rec */,
 				const RecordType* /* sub_rec */)
 	{

src/Type.h

@@ -509,6 +509,9 @@ inline BroType* error_type()	{ return base_type(TYPE_ERROR); }
 // test is done in the context of an initialization.
 extern int same_type(const BroType* t1, const BroType* t2, int is_init=0);
 
+// True if the two attribute lists are equivalent.
+extern int same_attrs(const Attributes* a1, const Attributes* a2);
+
 // Returns true if the record sub_rec can be promoted to the record
 // super_rec.
 extern int record_promotion_compatible(const RecordType* super_rec,