low-level cleanups found by code review

src/script_opt/CPP/Types.cc

@@ -145,7 +145,7 @@ void CPPCompile::ExpandListTypeVar(const TypePtr& t, string& tn)
 	const auto& tl = t->AsTypeList()->GetTypes();
 	auto t_name = tn + "->AsTypeList()";
 
-	for ( auto& tl_i : tl )
+	for ( const auto& tl_i : tl )
 		AddInit(t, t_name + "->Append(" + GenTypeName(tl_i) + ");");
 	}
 

src/script_opt/ZAM/AM-Opt.cc

@@ -753,12 +753,9 @@ void ZAMCompiler::SetLifetimeStart(int slot, const ZInstI* inst)
 		denizen_beginning[slot] = inst;
 
 		if ( inst_beginnings.count(inst) == 0 )
-			{
 			// Need to create a set to track the denizens
 			// beginning at the instruction.
-			std::unordered_set<ID*> denizens;
-			inst_beginnings[inst] = denizens;
-			}
+			inst_beginnings[inst] = {};
 
 		inst_beginnings[inst].insert(frame_denizens[slot]);
 		}
@@ -826,10 +823,7 @@ void ZAMCompiler::ExtendLifetime(int slot, const ZInstI* inst)
 			inst_endings[old_inst].erase(frame_denizens[slot]);
 
 			if ( inst_endings.count(inst) == 0 )
-				{
-				std::unordered_set<ID*> denizens;
-				inst_endings[inst] = denizens;
-				}
+				inst_endings[inst] = {};
 
 			inst_endings[inst].insert(frame_denizens[slot]);
 			denizen_ending.at(slot) = inst;

src/script_opt/ZAM/Compile.h

@@ -25,7 +25,7 @@ class CatchReturnStmt;
 
 class ProfileFunc;
 
-typedef ZInstI* InstLabel;
+using InstLabel = ZInstI*;
 
 // Class representing a single compiled statement.  (This is different from,
 // but related to, the ZAM instruction(s) generated for that compilation.)
@@ -296,8 +296,8 @@ private:
 		}
 
 
-	typedef std::vector<ZAMStmt> GoToSet;
-	typedef std::vector<GoToSet> GoToSets;
+	using GoToSet = std::vector<ZAMStmt>;
+	using GoToSets = std::vector<GoToSet>;
 
 	void PushGoTos(GoToSets& gotos);
 	void ResolveGoTos(GoToSets& gotos, const InstLabel l);
@@ -569,8 +569,8 @@ private:
 
 	// A type for mapping an instruction to a set of locals associated
 	// with it.
-	typedef std::unordered_map<const ZInstI*, std::unordered_set<ID*>>
-	        AssociatedLocals;
+	using AssociatedLocals =
+	      std::unordered_map<const ZInstI*, std::unordered_set<ID*>>;
 
 	// Maps (live) instructions to which frame denizens begin their
 	// lifetime via an initialization at that instruction, if any ...
@@ -584,7 +584,7 @@ private:
 	AssociatedLocals inst_endings;
 
 	// A type for inverse mappings.
-	typedef std::unordered_map<int, const ZInstI*> AssociatedInsts;
+	using AssociatedInsts = std::unordered_map<int, const ZInstI*>;
 
 	// Inverse mappings: for a given frame denizen's slot, where its
 	// lifetime begins and ends.

src/script_opt/ZAM/Driver.cc

@@ -102,9 +102,11 @@ void ZAMCompiler::InitLocals()
 	for ( auto l : pf->Locals() )
 		{
 		auto non_const_l = const_cast<ID*>(l);
-		// ### should check for unused variables.
 		// Don't add locals that were already added because they're
 		// parameters.
+		//
+		// Don't worry about unused variables, those will get
+		// removed during low-level ZAM optimization.
 		if ( ! HasFrameSlot(non_const_l) )
 			(void) AddToFrame(non_const_l);
 		}
@@ -289,24 +291,16 @@ void ZAMCompiler::AdjustBranches()
 		if ( ! inst->live )
 			continue;
 
-		auto t = inst->target;
-
-		if ( ! t )
-			continue;
-
-		inst->target = FindLiveTarget(t);
+		if ( auto t = inst->target )
+			inst->target = FindLiveTarget(t);
 		}
 	}
 
 void ZAMCompiler::RetargetBranches()
 	{
 	for ( auto& inst : insts2 )
-		{
-		if ( ! inst->target )
-			continue;
-
-		ConcretizeBranch(inst, inst->target, inst->target_slot);
-		}
+		if ( inst->target )
+			ConcretizeBranch(inst, inst->target, inst->target_slot);
 	}
 
 void ZAMCompiler::RemapFrameDenizens(const std::vector<int>& inst1_to_inst2)

src/script_opt/ZAM/Expr.cc

@@ -58,8 +58,6 @@ const ZAMStmt ZAMCompiler::CompileIncrExpr(const IncrExpr* e)
 	{
 	auto target = e->Op()->AsRefExpr()->GetOp1()->AsNameExpr();
 
-	auto s = EmptyStmt();
-
 	if ( target->GetType()->Tag() == TYPE_INT )
 		{
 		if ( e->Tag() == EXPR_INCR )
@@ -301,16 +299,16 @@ const ZAMStmt ZAMCompiler::CompileScheduleExpr(const ScheduleExpr* e)
 	auto when = e->When();
 
 	auto event_args = event->Args();
-        auto handler = event->Handler();
+	auto handler = event->Handler();
 
-        bool is_interval = when->GetType()->Tag() == TYPE_INTERVAL;
+	bool is_interval = when->GetType()->Tag() == TYPE_INTERVAL;
 
-        if ( when->Tag() == EXPR_NAME )
-                return ScheduleViHL(when->AsNameExpr(), is_interval,
-		                    handler.Ptr(), event_args);
-        else
-                return ScheduleCiHL(when->AsConstExpr(), is_interval,
-		                    handler.Ptr(), event_args);
+	if ( when->Tag() == EXPR_NAME )
+		return ScheduleViHL(when->AsNameExpr(), is_interval,
+				    handler.Ptr(), event_args);
+	else
+		return ScheduleCiHL(when->AsConstExpr(), is_interval,
+				    handler.Ptr(), event_args);
 	}
 
 const ZAMStmt ZAMCompiler::CompileSchedule(const NameExpr* n,

src/script_opt/ZAM/Gen-ZAM.cc

@@ -8,12 +8,12 @@
 using namespace std;
 
 // Helper functions to convert dashes to underscores or vice versa.
-char dash_to_under(char c)
+static char dash_to_under(char c)
 	{
 	return c == '-' ? '_' : c;
 	}
 
-char under_to_dash(char c)
+static char under_to_dash(char c)
 	{
 	return c == '_' ? '-' : c;
 	}
@@ -1944,7 +1944,7 @@ string TemplateInput::SkipWords(const string& line, int n) const
 	return string(s);
 	}
 
-void TemplateInput::Gripe(const char* msg, const string& input)
+void TemplateInput::Gripe(const char* msg, const string& input) const
 	{
 	auto input_s = input.c_str();
 	int n = strlen(input_s);
@@ -1957,7 +1957,7 @@ void TemplateInput::Gripe(const char* msg, const string& input)
 	exit(1);
 	}
 
-void TemplateInput::Gripe(const char* msg, const InputLoc& l)
+void TemplateInput::Gripe(const char* msg, const InputLoc& l) const
 	{
 	fprintf(stderr, "%s, line %d: %s\n", l.file_name, l.line_num, msg);
 	exit(1);

src/script_opt/ZAM/Gen-ZAM.h

@@ -183,15 +183,15 @@ public:
 	// Returns a string defining the parameters for a declaration;
 	// these have full C++ type information along with the parameter
 	// name.
-	string Decls()			{ return full_decl; }
+	string Decls() const			{ return full_decl; }
 
 	// Returns a string for passing the parameters in a function
 	// call.  This is a comma-separated list of the parameter names,
 	// with no associated C++ types.
-	string Params()			{ return full_params; }
+	string Params() const			{ return full_params; }
 
 	// Returns the name of the given parameter, indexed starting with 0.
-	const string& NthParam(int n)	{ return params[n]; }
+	const string& NthParam(int n) const	{ return params[n]; }
 
 private:
 	// Makes sure that each parameter has a unique name.  For any
@@ -894,8 +894,8 @@ public:
 	void PutBack(const string& line)	{ put_back = line; }
 
 	// Report an error and exit.
-	void Gripe(const char* msg, const string& input);
-	void Gripe(const char* msg, const InputLoc& loc);
+	[[noreturn]] void Gripe(const char* msg, const string& input) const;
+	[[noreturn]] void Gripe(const char* msg, const InputLoc& loc) const;
 
 private:
 	string put_back;	// if non-empty, use this for the next ScanLine
@@ -943,9 +943,9 @@ public:
 	void IndentDown()		{ --indent_level; }
 	void SetNoNL(bool _no_NL)	{ no_NL = _no_NL; }
 
-	void Gripe(const char* msg, const string& input)
+	[[noreturn]] void Gripe(const char* msg, const string& input) const
 		{ ti->Gripe(msg, input); }
-	void Gripe(const char* msg, const InputLoc& loc)
+	[[noreturn]] void Gripe(const char* msg, const InputLoc& loc) const
 		{ ti->Gripe(msg, loc); }
 
 private:

src/script_opt/ZAM/Ops.in

@@ -1463,7 +1463,7 @@ macro EvalScheduleArgs(time, is_delta, build_args)
 	if ( is_delta )
 		dt += run_state::network_time;
 	auto handler = EventHandlerPtr(z.event_handler);
-	val_vec args;
+	ValVec args;
 	build_args
 	auto timer = new ScheduleTimer(handler, std::move(args), dt);
 	timer_mgr->Add(timer);
@@ -1498,26 +1498,26 @@ op Event
 type HL
 op1-read
 custom-method return CompileEvent(h, l);
-eval	val_vec args;
+eval	ValVec args;
 	z.aux->FillValVec(args, frame);
 	event_mgr.Enqueue(z.event_handler, std::move(args));
 
 internal-op Event0
 type X
-eval	val_vec args(0);
+eval	ValVec args(0);
 	event_mgr.Enqueue(z.event_handler, std::move(args));
 
 internal-op Event1
 type V
 op1-read
-eval	val_vec args(1);
+eval	ValVec args(1);
 	args[0] = frame[z.v1].ToVal(z.t);
 	event_mgr.Enqueue(z.event_handler, std::move(args));
 
 internal-op Event2
 type VV
 op1-read
-eval	val_vec args(2);
+eval	ValVec args(2);
 	args[0] = frame[z.v1].ToVal(z.t);
 	args[1] = frame[z.v2].ToVal(z.t2);
 	event_mgr.Enqueue(z.event_handler, std::move(args));
@@ -1525,7 +1525,7 @@ eval	val_vec args(2);
 internal-op Event3
 type VVV
 op1-read
-eval	val_vec args(3);
+eval	ValVec args(3);
 	args[0] = frame[z.v1].ToVal(z.t);
 	args[1] = frame[z.v2].ToVal(z.t2);
 	auto types = z.aux->types;
@@ -1535,7 +1535,7 @@ eval	val_vec args(3);
 internal-op Event4
 type VVVV
 op1-read
-eval	val_vec args(4);
+eval	ValVec args(4);
 	args[0] = frame[z.v1].ToVal(z.t);
 	args[1] = frame[z.v2].ToVal(z.t2);
 	auto types = z.aux->types;

src/script_opt/ZAM/Support.cc

@@ -63,10 +63,10 @@ StringVal* ZAM_to_lower(const StringVal* sv)
 
 StringVal* ZAM_sub_bytes(const StringVal* s, bro_uint_t start, bro_int_t n)
 	{
-        if ( start > 0 )
-                --start;        // make it 0-based
+	if ( start > 0 )
+		--start;        // make it 0-based
 
-        auto ss = s->AsString()->GetSubstring(start, n);
+	auto ss = s->AsString()->GetSubstring(start, n);
 
 	return new StringVal(ss ? ss : new String(""));
 	}

src/script_opt/ZAM/Support.h

@@ -9,7 +9,7 @@
 
 namespace zeek::detail {
 
-typedef std::vector<ValPtr> val_vec;
+using ValVec = std::vector<ValPtr>;
 
 // The (reduced) statement currently being compiled.  Used for both
 // tracking "use" and "reaching" definitions, and for error messages.

src/script_opt/ZAM/ZBody.cc

@@ -98,6 +98,7 @@ static void vec_exec(ZOp op, TypePtr t, VectorVal*& v1, VectorVal* v2,
 		auto res_zv = new VectorVal(yt); \
 		auto n = v.size(); \
 		res_zv->Resize(n); \
+		ASSERT(0); \
 		auto& res = *res_zv->RawVec(); \
 		for ( auto i = 0U; i < n; ++i ) \
 			if ( v[i] ) \
@@ -284,7 +285,8 @@ ValPtr ZBody::DoExec(Frame* f, int start_pc, StmtFlowType& flow)
 
 	flow = FLOW_RETURN;	// can be over-written by a Hook-Break
 
-	while ( pc < end_pc && ! ZAM_error ) {
+	while ( pc < end_pc && ! ZAM_error )
+		{
 		auto& z = insts[pc];
 
 #ifdef DEBUG

src/script_opt/ZAM/ZBody.h

@@ -121,11 +121,8 @@ private:
 class ZAMResumption : public Stmt {
 public:
 	ZAMResumption(ZBody* _am, int _xfer_pc)
-	: Stmt(STMT_ZAM_RESUMPTION)
-		{
-		am = _am;
-		xfer_pc = _xfer_pc;
-		}
+	: Stmt(STMT_ZAM_RESUMPTION), am(_am), xfer_pc(_xfer_pc)
+		{ }
 
 	ValPtr Exec(Frame* f, StmtFlowType& flow) override;
 

src/script_opt/ZAM/ZInst.h

@@ -17,7 +17,7 @@ class Stmt;
 using AttributesPtr = IntrusivePtr<Attributes>;
 
 // Maps ZAM frame slots to associated identifiers.   
-typedef std::vector<ID*> FrameMap;
+using FrameMap = std::vector<ID*>;
 
 // Maps ZAM frame slots to information for sharing the slot across
 // multiple script variables.
@@ -45,7 +45,7 @@ public:
 	bool is_managed;
 };
 
-typedef std::vector<FrameSharingInfo> FrameReMap;
+using FrameReMap = std::vector<FrameSharingInfo>;
 
 class ZInstAux;
 
@@ -358,7 +358,7 @@ public:
 		}
 
 	// Returns the parallel arrays converted to a vector of ValPtr's.
-	const val_vec& ToValVec(const ZVal* frame)
+	const ValVec& ToValVec(const ZVal* frame)
 		{
 		vv.clear();
 		FillValVec(vv, frame);
@@ -367,7 +367,7 @@ public:
 
 	// Populates the given vector of ValPtr's with the conversion
 	// of the parallel arrays.
-	void FillValVec(val_vec& vec, const ZVal* frame) const
+	void FillValVec(ValVec& vec, const ZVal* frame) const
 		{
 		for ( auto i = 0; i < n; ++i )
 			vec.push_back(ToVal(frame, i));
@@ -439,7 +439,7 @@ public:
 	//
 	// If we cared about memory penny-pinching, we could make this
 	// a pointer and only instantiate as needed.
-	val_vec vv;
+	ValVec vv;
 };
 
 // Returns a human-readable version of the given ZAM op-code.