Change Attributes to store std:vector<IntrusivePtr<Attr>>

This also changes the return type of Attributes::Attrs() from attr_list*

NEWS

@@ -94,6 +94,9 @@ Changed Functionality
 
 - ``AsVector()`` has changed to return ``std::vector<IntrusivePtr<Val>>*``.
 
+- ``Attributes::Attrs()`` now returns ``const std::vector<IntrusivePtr<Attr>>&``
+  instead of ``attr_list*``
+
 Removed Functionality
 ---------------------
 

src/Attr.cc

@@ -139,7 +139,7 @@ void Attr::AddTag(ODesc* d) const
 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());
+	attrs.reserve(a->length());
 	in_record = arg_in_record;
 	global_var = is_global;
 
@@ -155,22 +155,11 @@ Attributes::Attributes(attr_list* a, IntrusivePtr<BroType> t, bool arg_in_record
 	delete a;
 	}
 
-Attributes::~Attributes()
-	{
-	for ( const auto& attr : *attrs )
-		Unref(attr);
-
-	delete attrs;
-	}
-
 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(IntrusivePtr{attr}.release());
+	attrs.emplace_back(attr);
 
 	// We only check the attribute after we've added it, to facilitate
 	// generating error messages via Attributes::Describe.
@@ -180,71 +169,69 @@ void Attributes::AddAttr(IntrusivePtr<Attr> attr)
 	// those attributes only have meaning for a redefinable value.
 	if ( (attr->Tag() == ATTR_ADD_FUNC || attr->Tag() == ATTR_DEL_FUNC) &&
 	     ! FindAttr(ATTR_REDEF) )
-		attrs->push_back(new Attr(ATTR_REDEF));
+		attrs.emplace_back(make_intrusive<Attr>(ATTR_REDEF));
 
 	// For DEFAULT, add an implicit OPTIONAL if it's not a global.
 	if ( ! global_var && attr->Tag() == ATTR_DEFAULT &&
 	     ! FindAttr(ATTR_OPTIONAL) )
-		attrs->push_back(new Attr(ATTR_OPTIONAL));
+		attrs.emplace_back(make_intrusive<Attr>(ATTR_OPTIONAL));
 	}
 
 void Attributes::AddAttrs(Attributes* a)
 	{
-	attr_list* as = a->Attrs();
-	for ( const auto& attr : *as )
-		AddAttr({NewRef{}, attr});
+	for ( const auto& attr : a->Attrs() )
+		AddAttr(attr);
 
 	Unref(a);
 	}
 
 Attr* Attributes::FindAttr(attr_tag t) const
 	{
-	if ( ! attrs )
-		return nullptr;
-
-	for ( const auto& a : *attrs )
-		{
+	for ( const auto& a : attrs )
 		if ( a->Tag() == t )
-			return a;
-		}
+			return a.get();
 
 	return nullptr;
 	}
 
 void Attributes::RemoveAttr(attr_tag t)
 	{
-	for ( int i = 0; i < attrs->length(); i++ )
-		if ( (*attrs)[i]->Tag() == t )
-			attrs->remove_nth(i--);
+	for ( auto it = attrs.begin(); it != attrs.end(); )
+		{
+		if ( (*it)->Tag() == t )
+			it = attrs.erase(it);
+		else
+			++it;
+		}
 	}
 
 void Attributes::Describe(ODesc* d) const
 	{
-	if ( ! attrs )
+	if ( attrs.empty() )
 		{
 		d->AddCount(0);
 		return;
 		}
 
-	d->AddCount(attrs->length());
+	d->AddCount(static_cast<uint64_t>(attrs.size()));
 
-	loop_over_list(*attrs, i)
+	for ( size_t i = 0; i < attrs.size(); ++i )
 		{
 		if ( (d->IsReadable() || d->IsPortable()) && i > 0 )
 			d->Add(", ");
 
-		(*attrs)[i]->Describe(d);
+		attrs[i]->Describe(d);
 		}
 	}
 
 void Attributes::DescribeReST(ODesc* d, bool shorten) const
 	{
-	loop_over_list(*attrs, i)
+	for ( size_t i = 0; i < attrs.size(); ++i )
 		{
 		if ( i > 0 )
 			d->Add(" ");
 
-		(*attrs)[i]->DescribeReST(d, shorten);
+		attrs[i]->DescribeReST(d, shorten);
 		}
 	}
 
@@ -415,15 +402,13 @@ void Attributes::CheckAttr(Attr* a)
 			}
 
 		int num_expires = 0;
-		if ( attrs )
+
+		for ( const auto& a : attrs )
 			{
-			for ( const auto& a : *attrs )
-				{
-				if ( a->Tag() == ATTR_EXPIRE_READ ||
-				     a->Tag() == ATTR_EXPIRE_WRITE ||
-				     a->Tag() == ATTR_EXPIRE_CREATE )
-					num_expires++;
-				}
+			if ( a->Tag() == ATTR_EXPIRE_READ ||
+				 a->Tag() == ATTR_EXPIRE_WRITE ||
+				 a->Tag() == ATTR_EXPIRE_CREATE )
+				num_expires++;
 			}
 
 		if ( num_expires > 1 )
@@ -609,13 +594,13 @@ void Attributes::CheckAttr(Attr* a)
 
 bool Attributes::operator==(const Attributes& other) const
 	{
-	if ( ! attrs )
-		return other.attrs;
+	if ( attrs.empty() )
+		return other.attrs.empty();
 
-	if ( ! other.attrs )
+	if ( other.attrs.empty() )
 		return false;
 
-	for ( const auto& a : *attrs )
+	for ( const auto& a : attrs )
 		{
 		Attr* o = other.FindAttr(a->Tag());
 
@@ -626,7 +611,7 @@ bool Attributes::operator==(const Attributes& other) const
 			return false;
 		}
 
-	for ( const auto& o : *other.attrs )
+	for ( const auto& o : other.attrs )
 		{
 		Attr* a = FindAttr(o->Tag());
 

src/Attr.h

@@ -2,6 +2,8 @@
 
 #pragma once
 
+#include <vector>
+
 #include "Obj.h"
 #include "BroList.h"
 #include "IntrusivePtr.h"
@@ -78,7 +80,6 @@ protected:
 class Attributes final : public BroObj {
 public:
 	Attributes(attr_list* a, IntrusivePtr<BroType> t, bool in_record, bool is_global);
-	~Attributes() override;
 
 	void AddAttr(IntrusivePtr<Attr> a);
 	void AddAttrs(Attributes* a);	// Unref's 'a' when done
@@ -90,7 +91,8 @@ public:
 	void Describe(ODesc* d) const override;
 	void DescribeReST(ODesc* d, bool shorten = false) const;
 
-	attr_list* Attrs()	{ return attrs; }
+	const std::vector<IntrusivePtr<Attr>>& Attrs() const
+		{ return attrs; }
 
 	bool operator==(const Attributes& other) const;
 
@@ -98,7 +100,7 @@ protected:
 	void CheckAttr(Attr* attr);
 
 	IntrusivePtr<BroType> type;
-	attr_list* attrs;
+	std::vector<IntrusivePtr<Attr>> attrs;
 	bool in_record;
 	bool global_var;
 };

src/Expr.cc

@@ -2170,9 +2170,14 @@ bool AssignExpr::TypeCheck(attr_list* attrs)
 
 				if ( sce->Attrs() )
 					{
-					attr_list* a = sce->Attrs()->Attrs();
-					attrs = new attr_list(a->length());
-					std::copy(a->begin(), a->end(), std::back_inserter(*attrs));
+					const auto& a = sce->Attrs()->Attrs();
+					attr_copy = new attr_list(a.size());
+
+					for ( const auto& attr : a )
+						{
+						::Ref(attr.get());
+						attrs->push_back(attr.get());
+						}
 					}
 
 				int errors_before = reporter->Errors();

src/Var.cc

@@ -326,10 +326,20 @@ IntrusivePtr<Stmt> add_local(IntrusivePtr<ID> id, IntrusivePtr<BroType> t,
 		        *init->GetLocationInfo() : no_location;
 
 		auto name_expr = make_intrusive<NameExpr>(id, dt == VAR_CONST);
-		auto attrs = id->GetAttrs() ? id->GetAttrs()->Attrs() : nullptr;
+		attr_list* attrs = nullptr;
+
+		if ( id->GetAttrs() )
+			{
+			attrs = new attr_list(id->GetAttrs()->Attrs().size());
+
+			for ( const auto& a : id->GetAttrs()->Attrs() )
+				attrs->push_back(a.get());
+			}
+
 		auto assign_expr = make_intrusive<AssignExpr>(std::move(name_expr),
 		                                              std::move(init), 0,
 		                                              nullptr, attrs);
+		delete attrs;
 		auto stmt = make_intrusive<ExprStmt>(std::move(assign_expr));
 		stmt->SetLocationInfo(&location);
 		return stmt;