Expr: check_and_promote_expr() returns IntrusivePtr

Instead of returning a pseudo-boolean integer, it now returns a
referenced object or nullptr on error.  The old API was very error
prone because of its obscure reference counting semantics.

src/Attr.cc

@@ -315,10 +315,10 @@ void Attributes::CheckAttr(Attr* a)
 				// Ok.
 				break;
 
-			Expr* e = a->AttrExpr();
+			auto e = check_and_promote_expr(a->AttrExpr(), type);
-			if ( check_and_promote_expr(e, type) )
+			if ( e )
 				{
-				a->SetAttrExpr(e);
+				a->SetAttrExpr(e.release());
 				// Ok.
 				break;
 				}
@@ -354,10 +354,10 @@ void Attributes::CheckAttr(Attr* a)
 					// Ok.
 					break;
 
-				Expr* e = a->AttrExpr();
+				auto e = check_and_promote_expr(a->AttrExpr(), ytype);
-				if ( check_and_promote_expr(e, ytype) )
+				if ( e )
 					{
-					a->SetAttrExpr(e);
+					a->SetAttrExpr(e.release());
 					// Ok.
 					break;
 					}
@@ -379,11 +379,10 @@ void Attributes::CheckAttr(Attr* a)
 
 			if ( (atype->Tag() == TYPE_TABLE && atype->AsTableType()->IsUnspecifiedTable()) )
 				{
-				Expr* e = a->AttrExpr();
+				auto e = check_and_promote_expr(a->AttrExpr(), type);
-
+				if ( e )
-				if ( check_and_promote_expr(e, type) )
 					{
-					a->SetAttrExpr(e);
+					a->SetAttrExpr(e.release());
 					break;
 					}
 				}

src/Expr.cc

@@ -3148,10 +3148,14 @@ TableConstructorExpr::TableConstructorExpr(IntrusivePtr<ListExpr> constructor_li
 			{
 			Expr* idx = idx_exprs[j];
 
-			if ( check_and_promote_expr(idx, (*indices)[j]) )
+			auto promoted_idx = check_and_promote_expr(idx, (*indices)[j]);
+			if ( promoted_idx )
 				{
-				if ( idx != idx_exprs[j] )
+				if ( promoted_idx.get() != idx )
-					idx_exprs.replace(j, idx);
+					{
+					Unref(idx);
+					idx_exprs.replace(j, promoted_idx.release());
+					}
 				continue;
 				}
 
@@ -4671,7 +4675,10 @@ IntrusivePtr<Val> ListExpr::InitVal(const BroType* t, IntrusivePtr<Val> aggr) co
 		loop_over_list(exprs, i)
 			{
 			Expr* e = exprs[i];
-			check_and_promote_expr(e, vec->Type()->AsVectorType()->YieldType());
+			auto promoted_e = check_and_promote_expr(e, vec->Type()->AsVectorType()->YieldType());
+			if (promoted_e)
+				e = promoted_e.get();
+
 			auto v = e->Eval(nullptr);
 
 			if ( ! vec->Assign(i, std::move(v)) )
@@ -4970,35 +4977,34 @@ IntrusivePtr<Expr> get_assign_expr(IntrusivePtr<Expr> op1,
 		                                  is_init);
 	}
 
-int check_and_promote_expr(Expr*& e, BroType* t)
+IntrusivePtr<Expr> check_and_promote_expr(Expr* const e, BroType* t)
 	{
 	BroType* et = e->Type();
 	TypeTag e_tag = et->Tag();
 	TypeTag t_tag = t->Tag();
 
 	if ( t->Tag() == TYPE_ANY )
-		return 1;
+		return {NewRef{}, e};
 
 	if ( EitherArithmetic(t_tag, e_tag) )
 		{
 		if ( e_tag == t_tag )
-			return 1;
+			return {NewRef{}, e};
 
 		if ( ! BothArithmetic(t_tag, e_tag) )
 			{
 			t->Error("arithmetic mixed with non-arithmetic", e);
-			return 0;
+			return nullptr;
 			}
 
 		TypeTag mt = max_type(t_tag, e_tag);
 		if ( mt != t_tag )
 			{
 			t->Error("over-promotion of arithmetic value", e);
-			return 0;
+			return nullptr;
 			}
 
-		e = new ArithCoerceExpr({AdoptRef{}, e}, t_tag);
+		return make_intrusive<ArithCoerceExpr>(IntrusivePtr{NewRef{}, e}, t_tag);
-		return 1;
 		}
 
 	if ( t->Tag() == TYPE_RECORD && et->Tag() == TYPE_RECORD )
@@ -5016,15 +5022,12 @@ int check_and_promote_expr(Expr*& e, BroType* t)
 
 				if ( same_attrs(td1->attrs.get(), td2->attrs.get()) )
 					// Everything matches perfectly.
-					return 1;
+					return {NewRef{}, e};
 				}
 			}
 
 		if ( record_promotion_compatible(t_r, et_r) )
-			{
+			return make_intrusive<RecordCoerceExpr>(IntrusivePtr{NewRef{}, e}, IntrusivePtr{NewRef{}, t_r});
-			e = new RecordCoerceExpr({AdoptRef{}, e}, {NewRef{}, t_r});
-			return 1;
-			}
 
 		t->Error("incompatible record types", e);
 		return 0;
@@ -5035,23 +5038,17 @@ int check_and_promote_expr(Expr*& e, BroType* t)
 		{
 		if ( t->Tag() == TYPE_TABLE && et->Tag() == TYPE_TABLE &&
 			  et->AsTableType()->IsUnspecifiedTable() )
-			{
+			return make_intrusive<TableCoerceExpr>(IntrusivePtr{NewRef{}, e}, IntrusivePtr{NewRef{}, t->AsTableType()});
-			e = new TableCoerceExpr({AdoptRef{}, e}, {NewRef{}, t->AsTableType()});
-			return 1;
-			}
 
 		if ( t->Tag() == TYPE_VECTOR && et->Tag() == TYPE_VECTOR &&
 		     et->AsVectorType()->IsUnspecifiedVector() )
-			{
+			return make_intrusive<VectorCoerceExpr>(IntrusivePtr{NewRef{}, e}, IntrusivePtr{NewRef{}, t->AsVectorType()});
-			e = new VectorCoerceExpr({AdoptRef{}, e}, {NewRef{}, t->AsVectorType()});
-			return 1;
-			}
 
 		t->Error("type clash", e);
-		return 0;
+		return nullptr;
 		}
 
-	return 1;
+	return {NewRef{}, e};
 	}
 
 int check_and_promote_exprs(ListExpr* const elements, TypeList* types)
@@ -5071,14 +5068,18 @@ int check_and_promote_exprs(ListExpr* const elements, TypeList* types)
 	loop_over_list(el, i)
 		{
 		Expr* e = el[i];
-		if ( ! check_and_promote_expr(e, (*tl)[i]) )
+		auto promoted_e = check_and_promote_expr(e, (*tl)[i]);
+		if ( ! promoted_e )
 			{
 			e->Error("type mismatch", (*tl)[i]);
 			return 0;
 			}
 
-		if ( e != el[i] )
+		if ( promoted_e.get() != e )
-			el.replace(i, e);
+			{
+			Unref(e);
+			el.replace(i, promoted_e.release());
+			}
 		}
 
 	return 1;
@@ -5138,14 +5139,18 @@ int check_and_promote_exprs_to_type(ListExpr* const elements, BroType* type)
 	loop_over_list(el, i)
 		{
 		Expr* e = el[i];
-		if ( ! check_and_promote_expr(e, type) )
+		auto promoted_e = check_and_promote_expr(e, type);
+		if ( ! promoted_e )
 			{
 			e->Error("type mismatch", type);
 			return 0;
 			}
 
-		if ( e != el[i] )
+		if ( promoted_e.get() != e )
-			el.replace(i, e);
+			{
+			Unref(e);
+			el.replace(i, promoted_e.release());
+			}
 		}
 
 	return 1;

src/Expr.h

@@ -925,7 +925,13 @@ IntrusivePtr<Expr> get_assign_expr(IntrusivePtr<Expr> op1,
 // types or a single type.
 //
 // Note, the type is not "const" because it can be ref'd.
-extern int check_and_promote_expr(Expr*& e, BroType* t);
+
+/**
+ * Returns nullptr if the expression cannot match or a promoted
+ * expression.
+ */
+extern IntrusivePtr<Expr> check_and_promote_expr(Expr* e, BroType* t);
+
 extern int check_and_promote_exprs(ListExpr* elements, TypeList* types);
 extern int check_and_promote_args(ListExpr* args, RecordType* types);
 extern int check_and_promote_exprs_to_type(ListExpr* elements, BroType* type);

src/Stmt.cc

@@ -1450,9 +1450,9 @@ ReturnStmt::ReturnStmt(IntrusivePtr<Expr> arg_e)
 
 	else
 		{
-		Expr* e_ = e.release();
+		auto promoted_e = check_and_promote_expr(e.get(), yt);
-		(void) check_and_promote_expr(e_, yt);
+		if ( promoted_e )
-		e = {AdoptRef{}, e_};
+			e = std::move(promoted_e);
 		}
 	}