Adjust some reaching-def memory management

E.g. can use unique_ptr or just avoid heap-allocating as minor
simplification of some mem-mgmt logic.

src/script_opt/GenRDs.cc

@@ -10,40 +10,6 @@
 namespace zeek::detail {
 
 
-// Helper class that tracks definitions gathered in a block that either
-// need to be propagated to the beginning of the block or to the end.
-// Used for RD propagation due to altered control flow (next/break/fallthrough).
-// Managed as a stack (vector) to deal with nested loops, switches, etc.
-// Only applies to gathering maximum RDs.
-class BlockDefs {
-public:
-	BlockDefs(bool _is_case)
-		{ is_case = _is_case; }
-
-	void AddPreRDs(RDPtr RDs)	{ pre_RDs.push_back(std::move(RDs)); }
-	void AddPostRDs(RDPtr RDs)	{ post_RDs.push_back(std::move(RDs)); }
-	void AddFutureRDs(RDPtr RDs)	{ future_RDs.push_back(std::move(RDs)); }
-
-	const std::vector<RDPtr>& PreRDs() const	{ return pre_RDs; }
-	const std::vector<RDPtr>& PostRDs() const	{ return post_RDs; }
-	const std::vector<RDPtr>& FutureRDs() const	{ return future_RDs; }
-
-	void Clear()
-		{ pre_RDs.clear(); post_RDs.clear(); future_RDs.clear(); }
-
-	bool IsCase() const	{ return is_case; }
-
-private:
-	std::vector<RDPtr> pre_RDs;
-	std::vector<RDPtr> post_RDs;
-	std::vector<RDPtr> future_RDs;	// RDs for next case block
-
-	// Whether this block is for a switch case.  If not,
-	// it's for a loop body.
-	bool is_case;
-};
-
-
 void RD_Decorate::TraverseFunction(const Func* f, Scope* scope, StmtPtr body)
 	{
 	func_flavor = f->Flavor();
@@ -236,7 +202,7 @@ TraversalCode RD_Decorate::PreStmt(const Stmt* s)
 		// To keep from traversing the loop expression, we just do
 		// the body manually here.
 
-		block_defs.push_back(new BlockDefs(false));
+		block_defs.emplace_back(std::make_unique<BlockDefs>(false));
 
 		body->Traverse(this);
 
@@ -274,7 +240,7 @@ TraversalCode RD_Decorate::PreStmt(const Stmt* s)
 		auto body = w->Body().get();
 		mgr.SetPreFromPre(body, cond);
 
-		block_defs.push_back(new BlockDefs(false));
+		block_defs.emplace_back(std::make_unique<BlockDefs>(false));
 
 		body->Traverse(this);
 
@@ -319,8 +285,8 @@ void RD_Decorate::TraverseSwitch(const SwitchStmt* sw)
 	auto sw_min_pre = mgr.GetPreMinRDs(sw);
 	auto sw_max_pre = mgr.GetPreMaxRDs(sw);
 
-	auto bd = new BlockDefs(true);
+	block_defs.emplace_back(std::make_unique<BlockDefs>(true));
-	block_defs.push_back(bd);
+	auto bd = block_defs.back().get();
 
 	RDPtr sw_post_min_rds = nullptr;
 	RDPtr sw_post_max_rds = nullptr;
@@ -431,7 +397,6 @@ void RD_Decorate::TraverseSwitch(const SwitchStmt* sw)
 	sw_post_max_rds.release();
 
 	block_defs.pop_back();
-	delete bd;
 	}
 
 void RD_Decorate::DoIfStmtConfluence(const IfStmt* i)
@@ -452,7 +417,7 @@ void RD_Decorate::DoIfStmtConfluence(const IfStmt* i)
 void RD_Decorate::DoLoopConfluence(const Stmt* s, const Stmt* top,
 					const Stmt* body)
 	{
-	auto bd = block_defs.back();
+	auto bd = std::move(block_defs.back());
 	block_defs.pop_back();
 
 	auto loop_pre = mgr.GetPreMaxRDs(top);
@@ -493,8 +458,7 @@ void RD_Decorate::DoLoopConfluence(const Stmt* s, const Stmt* top,
 			mgr.MergeIntoPre(body, mgr.GetPostMaxRDs(top));
 			}
 
-		auto bd2 = new BlockDefs(false);
+		block_defs.emplace_back(std::make_unique<BlockDefs>(false));
-		block_defs.push_back(bd2);
 		body->Traverse(this);
 		block_defs.pop_back();
 
@@ -502,7 +466,6 @@ void RD_Decorate::DoLoopConfluence(const Stmt* s, const Stmt* top,
 		// definitions in bd.  This is tricky because the body
 		// itself might not have RDs if it ends in a "break" or
 		// such.
-		delete bd2;
 		}
 
 	DefinitionPoint ds(s);
@@ -532,8 +495,6 @@ void RD_Decorate::DoLoopConfluence(const Stmt* s, const Stmt* top,
 		mgr.SetPostMaxRDs(s, max_post_rds);
 		max_post_rds.release();
 		}
-
-	delete bd;
 	}
 
 TraversalCode RD_Decorate::PostStmt(const Stmt* s)
@@ -635,7 +596,7 @@ void RD_Decorate::AddBlockDefs(const Stmt* s,
 	// match to this one.
 	for ( int i = block_defs.size() - 1; i >= 0; --i )
 		{
-		auto bd = block_defs[i];
+		auto& bd = block_defs[i];
 
 		if ( bd->IsCase() == is_case )
 			{ // This one matches what we're looking for.

src/script_opt/GenRDs.h

@@ -5,6 +5,8 @@
 
 #pragma once
 
+#include <memory>
+
 #include "zeek/script_opt/ReachingDefs.h"
 #include "zeek/script_opt/DefSetsMgr.h"
 #include "zeek/script_opt/ProfileFunc.h"
@@ -14,7 +16,37 @@ namespace zeek::detail {
 
 // Helper class that tracks definitions gathered in a block that either
 // need to be propagated to the beginning of the block or to the end.
-class BlockDefs;
+// Used for RD propagation due to altered control flow (next/break/fallthrough).
+// Managed as a stack (vector) to deal with nested loops, switches, etc.
+// Only applies to gathering maximum RDs.
+class BlockDefs {
+public:
+	BlockDefs(bool _is_case)
+		{ is_case = _is_case; }
+
+	void AddPreRDs(RDPtr RDs)	{ pre_RDs.push_back(std::move(RDs)); }
+	void AddPostRDs(RDPtr RDs)	{ post_RDs.push_back(std::move(RDs)); }
+	void AddFutureRDs(RDPtr RDs)	{ future_RDs.push_back(std::move(RDs)); }
+
+	const std::vector<RDPtr>& PreRDs() const	{ return pre_RDs; }
+	const std::vector<RDPtr>& PostRDs() const	{ return post_RDs; }
+	const std::vector<RDPtr>& FutureRDs() const	{ return future_RDs; }
+
+	void Clear()
+		{ pre_RDs.clear(); post_RDs.clear(); future_RDs.clear(); }
+
+	bool IsCase() const	{ return is_case; }
+
+private:
+	std::vector<RDPtr> pre_RDs;
+	std::vector<RDPtr> post_RDs;
+	std::vector<RDPtr> future_RDs;	// RDs for next case block
+
+	// Whether this block is for a switch case.  If not,
+	// it's for a loop body.
+	bool is_case;
+};
+
 
 class RD_Decorate : public TraversalCallback {
 public:
@@ -113,7 +145,7 @@ private:
 
 	// A stack of definitions associated with (potentially nested) loop
 	// and switch blocks.
-	std::vector<BlockDefs*> block_defs;
+	std::vector<std::unique_ptr<BlockDefs>> block_defs;
 };
 
 } // zeek::detail

src/script_opt/ReachingDefs.cc

@@ -9,7 +9,7 @@ namespace zeek::detail {
 
 ReachingDefs::ReachingDefs()
 	{
-	my_rd_map = new ReachingDefsMap;
+	my_rd_map = std::make_unique<ReachingDefsMap>();
 	const_rd_map = nullptr;
 	}
 
@@ -19,17 +19,6 @@ ReachingDefs::ReachingDefs(RDPtr rd)
 	my_rd_map = nullptr;
 	}
 
-ReachingDefs::~ReachingDefs()
-	{
-	if ( my_rd_map )
-		{
-		for ( auto& one_rd : *my_rd_map )
-			delete one_rd.second;
-
-		delete my_rd_map;
-		}
-	}
-
 void ReachingDefs::AddRD(const DefinitionItem* di, const DefinitionPoint& dp)
 	{
 	if ( HasPair(di, dp) )
@@ -41,14 +30,14 @@ void ReachingDefs::AddRD(const DefinitionItem* di, const DefinitionPoint& dp)
 
 	if ( curr_defs == my_rd_map->end() )
 		{
-		auto dps = new List<DefinitionPoint>();
+		auto dps = std::make_unique<List<DefinitionPoint>>();
 		dps->push_back(dp);
-		(*my_rd_map)[di] = dps;
+		(*my_rd_map)[di] = std::move(dps);
 		}
 
 	else
 		{
-		auto dps = curr_defs->second;
+		auto& dps = curr_defs->second;
 		dps->push_back(dp);
 		}
 	}
@@ -158,7 +147,7 @@ void ReachingDefs::CopyMapIfNeeded()
 	if ( my_rd_map )
 		return;
 
-	my_rd_map = new ReachingDefsMap;
+	my_rd_map = std::make_unique<ReachingDefsMap>();
 	auto old_const_rd_map = const_rd_map;
 	const_rd_map = nullptr;
 	AddRDs(old_const_rd_map);
@@ -179,7 +168,7 @@ void ReachingDefs::DumpMap(const ReachingDefsMap* map) const
 		if ( ++n > 1 )
 			printf(", ");
 
-		PrintRD(r->first, r->second);
+		PrintRD(r->first, r->second.get());
 		}
 
 	printf("\n");
@@ -200,10 +189,10 @@ void ReachingDefs::PrintRD(const DefinitionItem* di, const DefPoints* dps) const
 	}
 
 
-RDPtr& ReachingDefSet::FindRDs(const Obj* o) const
+const RDPtr& ReachingDefSet::FindRDs(const Obj* o) const
 	{
-	auto rd = a_i->find(o);
+	auto rd = a_i.find(o);
-	if ( rd == a_i->end() )
+	if ( rd == a_i.end() )
 		{
 		static RDPtr empty_rds;
 		return empty_rds;
@@ -215,8 +204,8 @@ RDPtr& ReachingDefSet::FindRDs(const Obj* o) const
 void ReachingDefSet::AddOrReplace(const Obj* o, const DefinitionItem* di,
 					const DefinitionPoint& dp)
 	{
-	auto rd = a_i->find(o);
+	auto rd = a_i.find(o);
-	ASSERT(rd != a_i->end());
+	ASSERT(rd != a_i.end());
 	rd->second->AddOrFullyReplace(di, dp);
 	}
 

src/script_opt/ReachingDefs.h

@@ -2,6 +2,8 @@
 
 #pragma once
 
+#include <memory>
+
 #include "zeek/script_opt/DefItem.h"
 
 
@@ -20,7 +22,7 @@ namespace zeek::detail {
 // as (maximally) reaching.
 
 typedef List<DefinitionPoint> DefPoints;
-typedef std::unordered_map<const DefinitionItem*, DefPoints*> ReachingDefsMap;
+typedef std::unordered_map<const DefinitionItem*, std::unique_ptr<DefPoints>> ReachingDefsMap;
 
 
 // The ReachingDefs class tracks all of the RDs associated with a given
@@ -41,8 +43,6 @@ public:
 	// Create a new object, using the RDs from another object.
 	ReachingDefs(RDPtr rd);
 
-	~ReachingDefs();
-
 	// Add in all the definition points from rd into our set, if
 	// we don't already have them.
 	void AddRDs(const RDPtr& rd)	{ AddRDs(rd->RDMap()); }
@@ -69,7 +69,7 @@ public:
 		{
 		auto map = RDMap();
 		auto dps = map->find(di);
-		return dps == map->end() ? nullptr : dps->second;
+		return dps == map->end() ? nullptr : dps->second.get();
 		}
 
 	// Returns true if two sets of definition points are equivalent,
@@ -130,7 +130,7 @@ protected:
 	void AddRDs(const ReachingDefsMap* rd_m);
 
 	const ReachingDefsMap* RDMap() const
-		{ return my_rd_map ? my_rd_map : const_rd_map->RDMap(); }
+		{ return my_rd_map ? my_rd_map.get() : const_rd_map->RDMap(); }
 
 	// If we don't already have our own map, copy the one we're using
 	// so that we then do.
@@ -142,7 +142,7 @@ protected:
 	// If my_rd_map is non-nil, then we use that map.  Otherwise,
 	// we use the map that const_rd_map points to.
 	RDPtr const_rd_map;
-	ReachingDefsMap* my_rd_map;
+	std::unique_ptr<ReachingDefsMap> my_rd_map;
 };
 
 
@@ -157,18 +157,11 @@ typedef std::unordered_map<const Obj*, RDPtr> AnalyInfo;
 class ReachingDefSet : public Obj {
 public:
 	ReachingDefSet(DefItemMap& _item_map) : item_map(_item_map)
-		{
+		{ }
-		a_i = new AnalyInfo;
-		}
-
-	~ReachingDefSet()
-		{
-		delete a_i;
-		}
 
 	// Whether in our collection we have RDs associated with the
 	// given AST node.
-	bool HasRDs(const Obj* o) const	{ return a_i->count(o) > 0; }
+	bool HasRDs(const Obj* o) const	{ return a_i.count(o) > 0; }
 
 	// Whether in our collection we have RDs associated with the
 	// given variable.
@@ -179,8 +172,8 @@ public:
 	// at the given AST node.
 	bool HasSingleRD(const Obj* o, const ID* id) const
 		{
-		auto RDs = a_i->find(o);
+		auto RDs = a_i.find(o);
-		if ( RDs == a_i->end() )
+		if ( RDs == a_i.end() )
 			return false;
 
 		auto di = item_map.GetConstID_DI(id);
@@ -196,8 +189,8 @@ public:
 	// AST node.
 	bool HasRD(const Obj* o, const DefinitionItem* di) const
 		{
-		auto RDs = a_i->find(o);
+		auto RDs = a_i.find(o);
-		if ( RDs == a_i->end() )
+		if ( RDs == a_i.end() )
 			return false;
 
 		return RDs->second->HasDI(di);
@@ -205,13 +198,13 @@ public:
 
 	// Returns the RDs associated with a given AST node, if any.
 	// If none are, returns an empty ReachingDef object.
-	RDPtr& FindRDs(const Obj* o) const;
+	const RDPtr& FindRDs(const Obj* o) const;
 
 	// Associates the given RDs with the given AST node.
 	void SetRDs(const Obj* o, RDPtr rd)
 		{
 		auto new_rd = make_intrusive<ReachingDefs>(std::move(rd));
-		(*a_i)[o] = new_rd;
+		a_i[o] = new_rd;
 		}
 
 	// If the given di is new, add this definition.  If it
@@ -226,7 +219,7 @@ public:
 		if ( HasRDs(o) )
 			MergeRDs(o, rd);
 		else
-			(*a_i)[o] = rd;
+			a_i[o] = rd;
 		}
 
 protected:
@@ -236,11 +229,11 @@ protected:
 	// RDs of 'o'.
 	void MergeRDs(const Obj* o, const RDPtr& rd)
 		{
-		auto curr_rds = a_i->find(o)->second;
+		auto curr_rds = a_i.find(o)->second;
 		curr_rds->AddRDs(rd);
 		}
 
-	AnalyInfo* a_i;
+	AnalyInfo a_i;
 	DefItemMap& item_map;
 };