Attr: use class IntrusivePtr

src/Attr.cc

@@ -23,18 +23,20 @@ const char* attr_name(attr_tag t)
 	return attr_names[int(t)];
 	}
 
-Attr::Attr(attr_tag t, Expr* e)
+Attr::Attr(attr_tag t, IntrusivePtr<Expr> e)
+	:expr(std::move(e))
 	{
 	tag = t;
-	expr = e;
 	SetLocationInfo(&start_location, &end_location);
 	}
 
-Attr::~Attr()
+Attr::Attr(attr_tag t)
+	:Attr(t, nullptr)
 	{
-	Unref(expr);
 	}
 
+Attr::~Attr() = default;
+
 void Attr::Describe(ODesc* d) const
 	{
 	AddTag(d);
@@ -131,10 +133,10 @@ void Attr::AddTag(ODesc* d) const
 		d->Add(attr_name(Tag()));
 	}
 
-Attributes::Attributes(attr_list* a, BroType* t, bool arg_in_record, bool is_global)
+Attributes::Attributes(attr_list* a, IntrusivePtr<BroType> t, bool arg_in_record, bool is_global)
+	:type(std::move(t))
 	{
 	attrs = new attr_list(a->length());
-	type = t->Ref();
 	in_record = arg_in_record;
 	global_var = is_global;
 
@@ -145,7 +147,7 @@ Attributes::Attributes(attr_list* a, BroType* t, bool arg_in_record, bool is_glo
 	// the necessary checking gets done.
 
 	for ( const auto& attr : *a )
-		AddAttr(attr);
+		AddAttr({NewRef{}, attr});
 
 	delete a;
 	}
@@ -156,23 +158,20 @@ Attributes::~Attributes()
 		Unref(attr);
 
 	delete attrs;
-
-	Unref(type);
 	}
 
-void Attributes::AddAttr(Attr* attr)
+void Attributes::AddAttr(IntrusivePtr<Attr> attr)
 	{
 	if ( ! attrs )
 		attrs = new attr_list(1);
 
 	// We overwrite old attributes by deleting them first.
 	RemoveAttr(attr->Tag());
-	attrs->push_back(attr);
+	attrs->push_back(IntrusivePtr{attr}.release());
-	Ref(attr);
 
 	// We only check the attribute after we've added it, to facilitate
 	// generating error messages via Attributes::Describe.
-	CheckAttr(attr);
+	CheckAttr(attr.get());
 
 	// For ADD_FUNC or DEL_FUNC, add in an implicit REDEF, since
 	// those attributes only have meaning for a redefinable value.
@@ -190,7 +189,7 @@ void Attributes::AddAttrs(Attributes* a)
 	{
 	attr_list* as = a->Attrs();
 	for ( const auto& attr : *as )
-		AddAttr(attr);
+		AddAttr({NewRef{}, attr});
 
 	Unref(a);
 	}
@@ -274,7 +273,7 @@ void Attributes::CheckAttr(Attr* a)
 			}
 
 		FuncType* aft = at->AsFuncType();
-		if ( ! same_type(aft->YieldType(), type) )
+		if ( ! same_type(aft->YieldType(), type.get()) )
 			{
 			a->AttrExpr()->Error(
 				is_add ?
@@ -299,7 +298,7 @@ void Attributes::CheckAttr(Attr* a)
 
 		if ( type->Tag() != TYPE_TABLE || (type->IsSet() && ! in_record) )
 			{
-			if ( same_type(atype, type) )
+			if ( same_type(atype, type.get()) )
 				// Ok.
 				break;
 
@@ -315,15 +314,15 @@ void Attributes::CheckAttr(Attr* a)
 				// Ok.
 				break;
 
-			auto e = check_and_promote_expr(a->AttrExpr(), type);
+			auto e = check_and_promote_expr(a->AttrExpr(), type.get());
 			if ( e )
 				{
-				a->SetAttrExpr(e.release());
+				a->SetAttrExpr(std::move(e));
 				// Ok.
 				break;
 				}
 
-			a->AttrExpr()->Error("&default value has inconsistent type", type);
+			a->AttrExpr()->Error("&default value has inconsistent type", type.get());
 			return;
 			}
 
@@ -357,7 +356,7 @@ void Attributes::CheckAttr(Attr* a)
 				auto e = check_and_promote_expr(a->AttrExpr(), ytype);
 				if ( e )
 					{
-					a->SetAttrExpr(e.release());
+					a->SetAttrExpr(std::move(e));
 					// Ok.
 					break;
 					}
@@ -373,16 +372,16 @@ void Attributes::CheckAttr(Attr* a)
 			{
 			// &default applies to record field.
 
-			if ( same_type(atype, type) )
+			if ( same_type(atype, type.get()) )
 				// Ok.
 				break;
 
 			if ( (atype->Tag() == TYPE_TABLE && atype->AsTableType()->IsUnspecifiedTable()) )
 				{
-				auto e = check_and_promote_expr(a->AttrExpr(), type);
+				auto e = check_and_promote_expr(a->AttrExpr(), type.get());
 				if ( e )
 					{
-					a->SetAttrExpr(e.release());
+					a->SetAttrExpr(std::move(e));
 					break;
 					}
 				}
@@ -573,7 +572,7 @@ void Attributes::CheckAttr(Attr* a)
 		break;
 
 	case ATTR_LOG:
-		if ( ! threading::Value::IsCompatibleType(type) )
+		if ( ! threading::Value::IsCompatibleType(type.get()) )
 			Error("&log applied to a type that cannot be logged");
 		break;
 

src/Attr.h

@@ -4,6 +4,7 @@
 
 #include "Obj.h"
 #include "BroList.h"
+#include "IntrusivePtr.h"
 
 class Expr;
 
@@ -35,18 +36,20 @@ typedef enum {
 
 class Attr : public BroObj {
 public:
-	explicit Attr(attr_tag t, Expr* e = 0);
+	Attr(attr_tag t, IntrusivePtr<Expr> e);
+	explicit Attr(attr_tag t);
 	~Attr() override;
 
 	attr_tag Tag() const	{ return tag; }
-	Expr* AttrExpr() const	{ return expr; }
+	Expr* AttrExpr() const	{ return expr.get(); }
 
 	// Up to the caller to decide if previous expr can be unref'd since it may
 	// not always be safe; e.g. expressions (at time of writing) don't always
 	// keep careful track of referencing their operands, so doing something
 	// like SetAttrExpr(coerce(AttrExpr())) must not completely unref the
 	// previous expr as the new expr depends on it.
-	void SetAttrExpr(Expr* e) { expr = e; }
+	template<typename E>
+	void SetAttrExpr(E &&e) { expr = std::forward<E>(e); }
 
 	void Describe(ODesc* d) const override;
 	void DescribeReST(ODesc* d, bool shorten = false) const;
@@ -69,16 +72,16 @@ protected:
 	void AddTag(ODesc* d) const;
 
 	attr_tag tag;
-	Expr* expr;
+	IntrusivePtr<Expr> expr;
 };
 
 // Manages a collection of attributes.
 class Attributes : public BroObj {
 public:
-	Attributes(attr_list* a, BroType* t, bool in_record, bool is_global);
+	Attributes(attr_list* a, IntrusivePtr<BroType> t, bool in_record, bool is_global);
 	~Attributes() override;
 
-	void AddAttr(Attr* a);
+	void AddAttr(IntrusivePtr<Attr> a);
 	void AddAttrs(Attributes* a);	// Unref's 'a' when done
 
 	Attr* FindAttr(attr_tag t) const;
@@ -93,10 +96,9 @@ public:
 	bool operator==(const Attributes& other) const;
 
 protected:
-	Attributes() : type(), attrs(), in_record()	{ }
 	void CheckAttr(Attr* attr);
 
-	BroType* type;
+	IntrusivePtr<BroType> type;
 	attr_list* attrs;
 	bool in_record;
 	bool global_var;

src/Expr.cc

@@ -3123,7 +3123,7 @@ TableConstructorExpr::TableConstructorExpr(IntrusivePtr<ListExpr> constructor_li
 			}
 		}
 
-	attrs = arg_attrs ? new Attributes(arg_attrs, type.get(), false, false) : 0;
+	attrs = arg_attrs ? new Attributes(arg_attrs, type, false, false) : 0;
 
 	type_list* indices = type->AsTableType()->Indices()->Types();
 	const expr_list& cle = op->AsListExpr()->Exprs();
@@ -3238,7 +3238,7 @@ SetConstructorExpr::SetConstructorExpr(IntrusivePtr<ListExpr> constructor_list,
 	else if ( type->Tag() != TYPE_TABLE || ! type->AsTableType()->IsSet() )
 		SetError("values in set(...) constructor do not specify a set");
 
-	attrs = arg_attrs ? new Attributes(arg_attrs, type.get(), false, false) : 0;
+	attrs = arg_attrs ? new Attributes(arg_attrs, type, false, false) : 0;
 
 	type_list* indices = type->AsTableType()->Indices()->Types();
 	expr_list& cle = op->AsListExpr()->Exprs();

src/ID.cc

@@ -188,9 +188,9 @@ void ID::UpdateValAttrs()
 				TypeDecl* fd = rt->FieldDecl(i);
 
 				if ( ! fd->attrs )
-					fd->attrs = make_intrusive<Attributes>(new attr_list, rt->FieldType(i), true, IsGlobal());
+					fd->attrs = make_intrusive<Attributes>(new attr_list, IntrusivePtr{NewRef{}, rt->FieldType(i)}, true, IsGlobal());
 
-				fd->attrs->AddAttr(new Attr(ATTR_LOG));
+				fd->attrs->AddAttr(make_intrusive<Attr>(ATTR_LOG));
 				}
 			}
 		}
@@ -206,13 +206,13 @@ bool ID::IsDeprecated() const
 	return FindAttr(ATTR_DEPRECATED) != 0;
 	}
 
-void ID::MakeDeprecated(Expr* deprecation)
+void ID::MakeDeprecated(IntrusivePtr<Expr> deprecation)
 	{
 	if ( IsDeprecated() )
 		return;
 
-	attr_list* attr = new attr_list{new Attr(ATTR_DEPRECATED, deprecation)};
+	attr_list* attr = new attr_list{new Attr(ATTR_DEPRECATED, std::move(deprecation))};
-	AddAttrs(make_intrusive<Attributes>(attr, Type(), false, IsGlobal()));
+	AddAttrs(make_intrusive<Attributes>(attr, IntrusivePtr{NewRef{}, Type()}, false, IsGlobal()));
 	}
 
 string ID::GetDeprecationWarning() const
@@ -262,7 +262,7 @@ void ID::SetOption()
 	if ( ! IsRedefinable() )
 		{
 		attr_list* attr = new attr_list{new Attr(ATTR_REDEF)};
-		AddAttrs(make_intrusive<Attributes>(attr, Type(), false, IsGlobal()));
+		AddAttrs(make_intrusive<Attributes>(attr, IntrusivePtr{NewRef{}, Type()}, false, IsGlobal()));
 		}
 	}
 

src/ID.h

@@ -86,7 +86,7 @@ public:
 
 	bool IsDeprecated() const;
 
-	void MakeDeprecated(Expr* deprecation);
+	void MakeDeprecated(IntrusivePtr<Expr> deprecation);
 
 	std::string GetDeprecationWarning() const;
 

src/Type.cc

@@ -571,7 +571,7 @@ void FuncType::DescribeReST(ODesc* d, bool roles_only) const
 
 TypeDecl::TypeDecl(IntrusivePtr<BroType> t, const char* i, attr_list* arg_attrs, bool in_record)
 	:type(std::move(t)),
-	 attrs(arg_attrs ? make_intrusive<Attributes>(arg_attrs, type.get(), in_record, false) : nullptr),
+	 attrs(arg_attrs ? make_intrusive<Attributes>(arg_attrs, type, in_record, false) : nullptr),
 	 id(i)
 	{
 	}
@@ -821,9 +821,9 @@ const char* RecordType::AddFields(type_decl_list* others, attr_list* attr)
 		if ( log )
 			{
 			if ( ! td->attrs )
-				td->attrs = make_intrusive<Attributes>(new attr_list, td->type.get(), true, false);
+				td->attrs = make_intrusive<Attributes>(new attr_list, td->type, true, false);
 
-			td->attrs->AddAttr(new Attr(ATTR_LOG));
+			td->attrs->AddAttr(make_intrusive<Attr>(ATTR_LOG));
 			}
 
 		types->push_back(td);
@@ -1122,7 +1122,7 @@ void EnumType::CheckAndAddName(const string& module_name, const char* name,
 		id->SetEnumConst();
 
 		if ( deprecation )
-			id->MakeDeprecated(deprecation);
+			id->MakeDeprecated({NewRef{}, deprecation});
 
 		zeekygen_mgr->Identifier(std::move(id));
 		}

src/Var.cc

@@ -114,7 +114,7 @@ static void make_var(ID* id, IntrusivePtr<BroType> t, init_class c,
 	id->SetType(t);
 
 	if ( attr )
-		id->AddAttrs(make_intrusive<Attributes>(attr, t.get(), false, id->IsGlobal()));
+		id->AddAttrs(make_intrusive<Attributes>(attr, t, false, id->IsGlobal()));
 
 	if ( init )
 		{
@@ -294,7 +294,7 @@ void add_type(ID* id, IntrusivePtr<BroType> t, attr_list* attr)
 	id->MakeType();
 
 	if ( attr )
-		id->SetAttrs(make_intrusive<Attributes>(attr, tnew.get(), false, false));
+		id->SetAttrs(make_intrusive<Attributes>(attr, tnew, false, false));
 	}
 
 static void transfer_arg_defaults(RecordType* args, RecordType* recv)
@@ -312,11 +312,11 @@ static void transfer_arg_defaults(RecordType* args, RecordType* recv)
 		if ( ! recv_i->attrs )
 			{
 			attr_list* a = new attr_list{def};
-			recv_i->attrs = make_intrusive<Attributes>(a, recv_i->type.get(), true, false);
+			recv_i->attrs = make_intrusive<Attributes>(a, recv_i->type, true, false);
 			}
 
 		else if ( ! recv_i->attrs->FindAttr(ATTR_DEFAULT) )
-			recv_i->attrs->AddAttr(def);
+			recv_i->attrs->AddAttr({NewRef{}, def});
 		}
 	}
 
@@ -412,7 +412,7 @@ void begin_func(ID* id, const char* module_name, function_flavor flavor,
 		}
 
 	if ( Attr* depr_attr = find_attr(attrs, ATTR_DEPRECATED) )
-		id->MakeDeprecated(depr_attr->AttrExpr());
+		id->MakeDeprecated({NewRef{}, depr_attr->AttrExpr()});
 	}
 
 class OuterIDBindingFinder : public TraversalCallback {

src/parse.y

@@ -1351,31 +1351,31 @@ attr_list:
 
 attr:
 		TOK_ATTR_DEFAULT '=' expr
-		        { $$ = new Attr(ATTR_DEFAULT, $3); }
+		        { $$ = new Attr(ATTR_DEFAULT, {AdoptRef{}, $3}); }
 	|	TOK_ATTR_OPTIONAL
 			{ $$ = new Attr(ATTR_OPTIONAL); }
 	|	TOK_ATTR_REDEF
 			{ $$ = new Attr(ATTR_REDEF); }
 	|	TOK_ATTR_ADD_FUNC '=' expr
-			{ $$ = new Attr(ATTR_ADD_FUNC, $3); }
+			{ $$ = new Attr(ATTR_ADD_FUNC, {AdoptRef{}, $3}); }
 	|	TOK_ATTR_DEL_FUNC '=' expr
-			{ $$ = new Attr(ATTR_DEL_FUNC, $3); }
+			{ $$ = new Attr(ATTR_DEL_FUNC, {AdoptRef{}, $3}); }
 	|	TOK_ATTR_ON_CHANGE '=' expr
-			{ $$ = new Attr(ATTR_ON_CHANGE, $3); }
+			{ $$ = new Attr(ATTR_ON_CHANGE, {AdoptRef{}, $3}); }
 	|	TOK_ATTR_EXPIRE_FUNC '=' expr
-			{ $$ = new Attr(ATTR_EXPIRE_FUNC, $3); }
+			{ $$ = new Attr(ATTR_EXPIRE_FUNC, {AdoptRef{}, $3}); }
 	|	TOK_ATTR_EXPIRE_CREATE '=' expr
-			{ $$ = new Attr(ATTR_EXPIRE_CREATE, $3); }
+			{ $$ = new Attr(ATTR_EXPIRE_CREATE, {AdoptRef{}, $3}); }
 	|	TOK_ATTR_EXPIRE_READ '=' expr
-			{ $$ = new Attr(ATTR_EXPIRE_READ, $3); }
+			{ $$ = new Attr(ATTR_EXPIRE_READ, {AdoptRef{}, $3}); }
 	|	TOK_ATTR_EXPIRE_WRITE '=' expr
-			{ $$ = new Attr(ATTR_EXPIRE_WRITE, $3); }
+			{ $$ = new Attr(ATTR_EXPIRE_WRITE, {AdoptRef{}, $3}); }
 	|	TOK_ATTR_RAW_OUTPUT
 			{ $$ = new Attr(ATTR_RAW_OUTPUT); }
 	|	TOK_ATTR_PRIORITY '=' expr
-			{ $$ = new Attr(ATTR_PRIORITY, $3); }
+			{ $$ = new Attr(ATTR_PRIORITY, {AdoptRef{}, $3}); }
 	|	TOK_ATTR_TYPE_COLUMN '=' expr
-			{ $$ = new Attr(ATTR_TYPE_COLUMN, $3); }
+			{ $$ = new Attr(ATTR_TYPE_COLUMN, {AdoptRef{}, $3}); }
 	|	TOK_ATTR_LOG
 			{ $$ = new Attr(ATTR_LOG); }
 	|	TOK_ATTR_ERROR_HANDLER
@@ -1385,7 +1385,7 @@ attr:
 	|	TOK_ATTR_DEPRECATED '=' TOK_CONSTANT
 			{
 			if ( IsString($3->Type()->Tag()) )
-				$$ = new Attr(ATTR_DEPRECATED, new ConstExpr({AdoptRef{}, $3}));
+				$$ = new Attr(ATTR_DEPRECATED, make_intrusive<ConstExpr>(IntrusivePtr{AdoptRef{}, $3}));
 			else
 				{
 				ODesc d;