Switch FuncType capture-list storage to optional<vector<Capture>>

May help clarify overall mem-mgmt/ownership semantics.

src/Expr.cc

@@ -4421,7 +4421,7 @@ LambdaExpr::LambdaExpr(std::unique_ptr<function_ingredients> arg_ing,
 void LambdaExpr::CheckCaptures()
 	{
 	auto ft = type->AsFuncType();
-	auto captures = ft->GetCaptures();
+	const auto& captures = ft->GetCaptures();
 
 	capture_by_ref = false;
 
@@ -4441,9 +4441,9 @@ void LambdaExpr::CheckCaptures()
 	std::set<const ID*> outer_is_matched;
 	std::set<const ID*> capture_is_matched;
 
-	for ( auto c : *captures )
+	for ( const auto& c : *captures )
 		{
-		auto cid = c->id.get();
+		auto cid = c.id.get();
 
 		if ( ! cid )
 			// This happens for undefined/inappropriate
@@ -4470,9 +4470,9 @@ void LambdaExpr::CheckCaptures()
 		if ( outer_is_matched.count(id) == 0 )
 			ExprError(util::fmt("%s is used inside lambda but not captured", id->Name()));
 
-	for ( auto c : *captures )
+	for ( const auto& c : *captures )
 		{
-		auto cid = c->id.get();
+		auto cid = c.id.get();
 		if ( cid && capture_is_matched.count(cid) == 0 )
 			ExprError(util::fmt("%s is captured but not used inside lambda", cid->Name()));
 		}

src/Frame.cc

@@ -419,7 +419,7 @@ broker::expected<broker::data> Frame::SerializeCopyFrame()
 	}
 
 std::pair<bool, FramePtr> Frame::Unserialize(const broker::vector& data,
-				const std::vector<FuncType::Capture*>* captures)
+				const std::optional<FuncType::CaptureList>& captures)
 	{
 	if ( data.size() == 0 )
 		return std::make_pair(true, nullptr);
@@ -505,7 +505,7 @@ std::pair<bool, FramePtr> Frame::Unserialize(const broker::vector& data,
 
 		std::advance(where, 1);
 
-		auto closure_pair = Frame::Unserialize(*has_vec, nullptr);
+		auto closure_pair = Frame::Unserialize(*has_vec, {});
 		if ( ! closure_pair.first )
 			{
 			for ( auto& i : outer_ids )

src/Frame.h

@@ -7,6 +7,7 @@
 #include <utility>
 #include <vector>
 #include <memory>
+#include <optional>
 
 #include <broker/data.hh>
 #include <broker/expected.hh>
@@ -242,7 +243,7 @@ public:
 	 * reference semantics.
 	 */
 	static std::pair<bool, FramePtr> Unserialize(const broker::vector& data,
-			const std::vector<FuncType::Capture*>* captures);
+			const std::optional<FuncType::CaptureList>& captures);
 
 	/**
 	 * Sets the IDs that the frame knows offsets for. These offsets will

src/Func.cc

@@ -477,7 +477,7 @@ ValPtr ScriptFunc::Invoke(zeek::Args* args, Frame* parent) const
 
 void ScriptFunc::CreateCaptures(Frame* f)
 	{
-	auto captures = type->GetCaptures();
+	const auto& captures = type->GetCaptures();
 
 	if ( ! captures )
 		return;
@@ -490,27 +490,26 @@ void ScriptFunc::CreateCaptures(Frame* f)
 	captures_offset_mapping = new OffsetMap;
 
 	int offset = 0;
-	for ( auto c : *captures )
+	for ( const auto& c : *captures )
 		{
-		auto cid = c->id;
-		auto v = f->GetElementByID(cid);
+		auto v = f->GetElementByID(c.id);
 
 		if ( v )
 			{
-			if ( c->deep_copy || ! v->Modifiable() )
+			if ( c.deep_copy || ! v->Modifiable() )
 				v = v->Clone();
 
 			captures_frame->SetElement(offset, std::move(v));
 			}
 
-		(*captures_offset_mapping)[cid->Name()] = offset;
+		(*captures_offset_mapping)[c.id->Name()] = offset;
 		++offset;
 		}
 	}
 
 void ScriptFunc::SetCaptures(Frame* f)
 	{
-	auto captures = type->GetCaptures();
+	const auto& captures = type->GetCaptures();
 	ASSERT(captures);
 
 	delete captures_frame;
@@ -519,10 +518,9 @@ void ScriptFunc::SetCaptures(Frame* f)
 	captures_offset_mapping = new OffsetMap;
 
 	int offset = 0;
-	for ( auto c : *captures )
+	for ( const auto& c : *captures )
 		{
-		auto cid = c->id;
-		(*captures_offset_mapping)[cid->Name()] = offset;
+		(*captures_offset_mapping)[c.id->Name()] = offset;
 		++offset;
 		}
 	}
@@ -600,7 +598,7 @@ void ScriptFunc::SetClosureFrame(Frame* f)
 
 bool ScriptFunc::UpdateClosure(const broker::vector& data)
 	{
-	auto result = Frame::Unserialize(data, nullptr);
+	auto result = Frame::Unserialize(data, {});
 
 	if ( ! result.first )
 		return false;

src/Type.cc

@@ -622,8 +622,6 @@ FuncType::FuncType(RecordTypePtr arg_args,
 		}
 
 	prototypes.emplace_back(Prototype{false, "", args, std::move(offsets)});
-
-	captures = nullptr;
 	}
 
 TypePtr FuncType::ShallowClone()
@@ -657,16 +655,6 @@ string FuncType::FlavorString() const
 	}
 	}
 
-FuncType::~FuncType()
-	{
-	if ( captures )
-		{
-		for ( auto c : *captures )
-			delete c;
-		delete captures;
-		}
-	}
-
 int FuncType::MatchesIndex(detail::ListExpr* const index) const
 	{
 	return check_and_promote_args(index, args.get()) ?
@@ -709,9 +697,9 @@ bool FuncType::CheckArgs(const std::vector<TypePtr>& args,
 	return success;
 	}
 
-void FuncType::SetCaptures(std::vector<Capture*>* _captures)
+void FuncType::SetCaptures(std::optional<CaptureList> _captures)
 	{
-	captures = _captures;
+	captures = std::move(_captures);
 	}
 
 void FuncType::Describe(ODesc* d) const

src/Type.h

@@ -492,8 +492,6 @@ public:
 
 	TypePtr ShallowClone() override;
 
-	~FuncType() override;
-
 	[[deprecated("Remove in v4.1.  Use Params().")]]
 	RecordType* Args() const	{ return args.get(); }
 
@@ -549,19 +547,21 @@ public:
 		bool deep_copy;
 	};
 
+	using CaptureList = std::vector<Capture>;
+
 	/**
 	 * Sets this function's set of captures.  Only valid for lambdas.
 	 *
 	 * @param captures  if non-nil, a list of the lambda's captures
 	 */
-	void SetCaptures(std::vector<Capture*>* captures);
+	void SetCaptures(std::optional<CaptureList> captures);
 
 	/**
 	 * Returns the captures declared for this function, or nil if none.
 	 *
 	 * @return a vector giving the captures
 	 */
-	const std::vector<Capture*>* GetCaptures() const
+	const std::optional<CaptureList>& GetCaptures() const
 		{ return captures; }
 
 protected:
@@ -574,7 +574,7 @@ protected:
 	FunctionFlavor flavor;
 	std::vector<Prototype> prototypes;
 
-	std::vector<Capture*>* captures; // if nil then no captures specified
+	std::optional<CaptureList> captures; // if nil then no captures specified
 };
 
 class TypeType final : public Type {

src/broker/Data.cc

@@ -405,8 +405,7 @@ struct val_converter {
 				if ( ! b )
 					return nullptr;
 
-				auto copy_semantics =
-					b->GetType()->GetCaptures() != nullptr;
+				auto copy_semantics = b->GetType()->GetCaptures().has_value();
 
 				if ( copy_semantics )
 					{

src/parse.y

@@ -1315,7 +1315,24 @@ begin_lambda:
 			{
 			auto id = zeek::detail::current_scope()->GenerateTemporary("anonymous-function");
 			zeek::detail::begin_func(id, zeek::detail::current_module.c_str(), zeek::FUNC_FLAVOR_FUNCTION, false, {zeek::AdoptRef{}, $2});
-			$2->SetCaptures($1);
+
+			std::optional<zeek::FuncType::CaptureList> captures;
+
+			if ( $1 )
+				{
+				captures = zeek::FuncType::CaptureList{};
+				captures->reserve($1->size());
+
+				for ( auto c : *$1 )
+					{
+					captures->emplace_back(*c);
+					delete c;
+					}
+
+				delete $1;
+				}
+
+			$2->SetCaptures(std::move(captures));
 			$$ = id.release();
 			}
 	;