Merge remote-tracking branch 'origin/topic/vern/when-cleanup'

* origin/topic/vern/when-cleanup:
  test suite update for minor change in "when" error messages
  removed skeletal (non-functioning) "when" support from ZAM
  simplify WhenInfo and Trigger classes given removal of old capture semantics
  introduced notion of light-weight Frame clones
  changed function_ingredients struct to FunctionIngredients class with accessors

Renamed Frame::LightClone() to Frame::CloneForTrigger() during merge.

CHANGES

@@ -1,3 +1,15 @@
+6.0.0-dev.310 | 2023-04-04 10:00:24 +0200
+
+  * test suite update for minor change in "when" error messages (Vern Paxson, Corelight)
+
+  * removed skeletal (non-functioning) "when" support from ZAM (Vern Paxson, Corelight)
+
+  * simplify WhenInfo and Trigger classes given removal of old capture semantics (Vern Paxson, Corelight)
+
+  * introduced notion of light-weight Frame clones (Vern Paxson, Corelight)
+
+  * changed function_ingredients struct to FunctionIngredients class with accessors (Vern Paxson, Corelight)
+
 6.0.0-dev.303 | 2023-04-03 16:37:02 +0200
 
   * addressed static analysis concern about possible null pointer (Vern Paxson, Corelight)

VERSION

@@ -1 +1 @@
-6.0.0-dev.303
+6.0.0-dev.310

src/Expr.cc

@@ -4624,14 +4624,14 @@ void CallExpr::ExprDescribe(ODesc* d) const
 		args->Describe(d);
 	}
 
-LambdaExpr::LambdaExpr(std::unique_ptr<function_ingredients> arg_ing, IDPList arg_outer_ids,
+LambdaExpr::LambdaExpr(std::unique_ptr<FunctionIngredients> arg_ing, IDPList arg_outer_ids,
                        StmtPtr when_parent)
 	: Expr(EXPR_LAMBDA)
 	{
 	ingredients = std::move(arg_ing);
 	outer_ids = std::move(arg_outer_ids);
 
-	SetType(ingredients->id->GetType());
+	SetType(ingredients->GetID()->GetType());
 
 	if ( ! CheckCaptures(when_parent) )
 		{
@@ -4641,9 +4641,9 @@ LambdaExpr::LambdaExpr(std::unique_ptr<function_ingredients> arg_ing, IDPList ar
 
 	// Install a dummy version of the function globally for use only
 	// when broker provides a closure.
-	auto dummy_func = make_intrusive<ScriptFunc>(ingredients->id);
+	auto dummy_func = make_intrusive<ScriptFunc>(ingredients->GetID());
-	dummy_func->AddBody(ingredients->body, ingredients->inits, ingredients->frame_size,
+	dummy_func->AddBody(ingredients->Body(), ingredients->Inits(), ingredients->FrameSize(),
-	                    ingredients->priority);
+	                    ingredients->Priority());
 
 	dummy_func->SetOuterIDs(outer_ids);
 
@@ -4678,7 +4678,7 @@ LambdaExpr::LambdaExpr(std::unique_ptr<function_ingredients> arg_ing, IDPList ar
 
 	auto v = make_intrusive<FuncVal>(std::move(dummy_func));
 	lambda_id->SetVal(std::move(v));
-	lambda_id->SetType(ingredients->id->GetType());
+	lambda_id->SetType(ingredients->GetID()->GetType());
 	lambda_id->SetConst();
 	}
 
@@ -4766,14 +4766,14 @@ bool LambdaExpr::CheckCaptures(StmtPtr when_parent)
 
 ScopePtr LambdaExpr::GetScope() const
 	{
-	return ingredients->scope;
+	return ingredients->Scope();
 	}
 
 ValPtr LambdaExpr::Eval(Frame* f) const
 	{
-	auto lamb = make_intrusive<ScriptFunc>(ingredients->id);
+	auto lamb = make_intrusive<ScriptFunc>(ingredients->GetID());
-	lamb->AddBody(ingredients->body, ingredients->inits, ingredients->frame_size,
+	lamb->AddBody(ingredients->Body(), ingredients->Inits(), ingredients->FrameSize(),
-	              ingredients->priority);
+	              ingredients->Priority());
 
 	lamb->CreateCaptures(f);
 
@@ -4787,7 +4787,7 @@ ValPtr LambdaExpr::Eval(Frame* f) const
 void LambdaExpr::ExprDescribe(ODesc* d) const
 	{
 	d->Add(expr_name(Tag()));
-	ingredients->body->Describe(d);
+	ingredients->Body()->Describe(d);
 	}
 
 TraversalCode LambdaExpr::Traverse(TraversalCallback* cb) const
@@ -4802,7 +4802,7 @@ TraversalCode LambdaExpr::Traverse(TraversalCallback* cb) const
 	tc = lambda_id->Traverse(cb);
 	HANDLE_TC_EXPR_PRE(tc);
 
-	tc = ingredients->body->Traverse(cb);
+	tc = ingredients->Body()->Traverse(cb);
 	HANDLE_TC_EXPR_PRE(tc);
 
 	tc = cb->PostExpr(this);

src/Expr.h

@@ -27,7 +27,7 @@ namespace detail
 
 class Frame;
 class Scope;
-struct function_ingredients;
+class FunctionIngredients;
 using IDPtr = IntrusivePtr<ID>;
 using ScopePtr = IntrusivePtr<Scope>;
 
@@ -1454,12 +1454,12 @@ protected:
 class LambdaExpr final : public Expr
 	{
 public:
-	LambdaExpr(std::unique_ptr<function_ingredients> ingredients, IDPList outer_ids,
+	LambdaExpr(std::unique_ptr<FunctionIngredients> ingredients, IDPList outer_ids,
 	           StmtPtr when_parent = nullptr);
 
 	const std::string& Name() const { return my_name; }
 	const IDPList& OuterIDs() const { return outer_ids; }
-	const function_ingredients& Ingredients() const { return *ingredients; }
+	const FunctionIngredients& Ingredients() const { return *ingredients; }
 
 	ValPtr Eval(Frame* f) const override;
 	TraversalCode Traverse(TraversalCallback* cb) const override;
@@ -1478,7 +1478,7 @@ protected:
 private:
 	bool CheckCaptures(StmtPtr when_parent);
 
-	std::unique_ptr<function_ingredients> ingredients;
+	std::unique_ptr<FunctionIngredients> ingredients;
 	IDPtr lambda_id;
 	IDPList outer_ids;
 

src/Frame.cc

@@ -120,6 +120,17 @@ Frame* Frame::Clone() const
 	return other;
 	}
 
+Frame* Frame::CloneForTrigger() const
+	{
+	Frame* other = new Frame(0, function, func_args);
+
+	other->call = call;
+	other->assoc = assoc;
+	other->trigger = trigger;
+
+	return other;
+	}
+
 static bool val_is_func(const ValPtr& v, ScriptFunc* func)
 	{
 	if ( v->GetType()->Tag() != TYPE_FUNC )

src/Frame.h

@@ -157,6 +157,13 @@ public:
 	 */
 	Frame* Clone() const;
 
+	/**
+	 * Creates a copy of the frame that just includes its trigger context.
+	 *
+	 * @return a partial copy of this frame.
+	 */
+	Frame* CloneForTrigger() const;
+
 	/**
 	 * Serializes the frame in support of copy semantics for lambdas:
 	 *

src/Func.cc

@@ -862,16 +862,18 @@ static std::set<EventGroupPtr> get_func_groups(const std::vector<AttrPtr>& attrs
 	return groups;
 	}
 
-function_ingredients::function_ingredients(ScopePtr scope, StmtPtr body,
+FunctionIngredients::FunctionIngredients(ScopePtr _scope, StmtPtr _body,
                                          const std::string& module_name)
 	{
+	scope = std::move(_scope);
+	body = std::move(_body);
+
 	frame_size = scope->Length();
 	inits = scope->GetInits();
 
-	this->scope = std::move(scope);
+	id = scope->GetID();
-	id = this->scope->GetID();
 
-	const auto& attrs = this->scope->Attrs();
+	const auto& attrs = scope->Attrs();
 
 	if ( attrs )
 		{
@@ -890,15 +892,11 @@ function_ingredients::function_ingredients(ScopePtr scope, StmtPtr body,
 	else
 		priority = 0;
 
-	this->body = std::move(body);
-	this->module_name = module_name;
-
 	// Implicit module event groups for events and hooks.
 	auto flavor = id->GetType<zeek::FuncType>()->Flavor();
 	if ( flavor == FUNC_FLAVOR_EVENT || flavor == FUNC_FLAVOR_HOOK )
 		{
-		auto module_group = event_registry->RegisterGroup(EventGroupKind::Module,
+		auto module_group = event_registry->RegisterGroup(EventGroupKind::Module, module_name);
-		                                                  this->module_name);
 		groups.insert(module_group);
 		}
 	}

src/Func.h

@@ -334,16 +334,27 @@ struct CallInfo
 
 // Struct that collects all the specifics defining a Func. Used for ScriptFuncs
 // with closures.
-struct function_ingredients
+class FunctionIngredients
 	{
-
+public:
 	// Gathers all of the information from a scope and a function body needed
 	// to build a function.
-	function_ingredients(ScopePtr scope, StmtPtr body, const std::string& module_name);
+	FunctionIngredients(ScopePtr scope, StmtPtr body, const std::string& module_name);
 
+	const IDPtr& GetID() const { return id; }
+
+	const StmtPtr& Body() const { return body; }
+	void SetBody(StmtPtr _body) { body = std::move(_body); }
+
+	const auto& Inits() const { return inits; }
+	size_t FrameSize() const { return frame_size; }
+	int Priority() const { return priority; }
+	const ScopePtr& Scope() const { return scope; }
+	const auto& Groups() const { return groups; }
+
+private:
 	IDPtr id;
 	StmtPtr body;
-	std::string module_name; // current module name where function body is defined
 	std::vector<IDPtr> inits;
 	size_t frame_size = 0;
 	int priority = 0;

src/Stmt.cc

@@ -1867,7 +1867,8 @@ TraversalCode NullStmt::Traverse(TraversalCallback* cb) const
 WhenInfo::WhenInfo(ExprPtr arg_cond, FuncType::CaptureList* arg_cl, bool arg_is_return)
 	: cond(std::move(arg_cond)), cl(arg_cl), is_return(arg_is_return)
 	{
-	prior_vars = current_scope()->Vars();
+	if ( ! cl )
+		cl = new zeek::FuncType::CaptureList;
 
 	ProfileFunc cond_pf(cond.get());
 
@@ -1881,8 +1882,6 @@ WhenInfo::WhenInfo(ExprPtr arg_cond, FuncType::CaptureList* arg_cl, bool arg_is_
 		{
 		bool is_present = false;
 
-		if ( cl )
-			{
 		for ( auto& c : *cl )
 			if ( c.id == wl )
 				{
@@ -1895,7 +1894,6 @@ WhenInfo::WhenInfo(ExprPtr arg_cond, FuncType::CaptureList* arg_cl, bool arg_is_
 			IDPtr wl_ptr = {NewRef{}, const_cast<ID*>(wl)};
 			cl->emplace_back(FuncType::Capture{wl_ptr, false});
 			}
-			}
 
 		// In addition, don't treat them as external locals that
 		// existed at the onset.
@@ -1926,32 +1924,12 @@ WhenInfo::WhenInfo(ExprPtr arg_cond, FuncType::CaptureList* arg_cl, bool arg_is_
 
 WhenInfo::WhenInfo(bool arg_is_return) : is_return(arg_is_return)
 	{
-	// This won't be needed once we remove the deprecated semantics.
 	cl = new zeek::FuncType::CaptureList;
 	BuildInvokeElems();
 	}
 
 void WhenInfo::Build(StmtPtr ws)
 	{
-	// This will call ws->Error() if it's deprecated and we can
-	// short-circuit.
-	if ( IsDeprecatedSemantics(ws) )
-		return;
-
-	if ( ! cl )
-		{
-		// This instance is compatible with new-style semantics,
-		// so create a capture list for it and populate with any
-		// when-locals.
-		cl = new zeek::FuncType::CaptureList;
-
-		for ( auto& wl : when_new_locals )
-			{
-			IDPtr wl_ptr = {NewRef{}, const_cast<ID*>(wl)};
-			cl->emplace_back(FuncType::Capture{wl_ptr, false});
-			}
-		}
-
 	lambda_ft->SetCaptures(*cl);
 
 	// Our general strategy is to construct a single lambda (so that
@@ -1996,38 +1974,30 @@ void WhenInfo::Build(StmtPtr ws)
 
 	auto shebang = make_intrusive<StmtList>(do_test, do_bodies, dummy_return);
 
-	auto ingredients = std::make_unique<function_ingredients>(current_scope(), shebang,
+	auto ingredients = std::make_unique<FunctionIngredients>(current_scope(), shebang,
 	                                                         current_module);
-	auto outer_ids = gather_outer_ids(pop_scope(), ingredients->body);
+	auto outer_ids = gather_outer_ids(pop_scope(), ingredients->Body());
 
 	lambda = make_intrusive<LambdaExpr>(std::move(ingredients), std::move(outer_ids), ws);
 	}
 
 void WhenInfo::Instantiate(Frame* f)
 	{
-	if ( cl )
 	Instantiate(lambda->Eval(f));
 	}
 
 void WhenInfo::Instantiate(ValPtr func)
 	{
-	if ( cl )
 	curr_lambda = make_intrusive<ConstExpr>(std::move(func));
 	}
 
 ExprPtr WhenInfo::Cond()
 	{
-	if ( ! curr_lambda )
-		return cond;
-
 	return make_intrusive<CallExpr>(curr_lambda, invoke_cond);
 	}
 
 StmtPtr WhenInfo::WhenBody()
 	{
-	if ( ! curr_lambda )
-		return s;
-
 	auto invoke = make_intrusive<CallExpr>(curr_lambda, invoke_s);
 	return make_intrusive<ReturnStmt>(invoke, true);
 	}
@@ -2046,61 +2016,10 @@ double WhenInfo::TimeoutVal(Frame* f)
 
 StmtPtr WhenInfo::TimeoutStmt()
 	{
-	if ( ! curr_lambda )
-		return timeout_s;
-
 	auto invoke = make_intrusive<CallExpr>(curr_lambda, invoke_timeout);
 	return make_intrusive<ReturnStmt>(invoke, true);
 	}
 
-bool WhenInfo::IsDeprecatedSemantics(StmtPtr ws)
-	{
-	if ( cl )
-		return false;
-
-	// Which locals of the outer function are used in any of the "when"
-	// elements.
-	IDSet locals;
-
-	for ( auto& wl : when_new_locals )
-		prior_vars.erase(wl->Name());
-
-	for ( auto& bl : when_expr_locals )
-		if ( prior_vars.count(bl->Name()) > 0 )
-			locals.insert(bl);
-
-	ProfileFunc body_pf(s.get());
-	for ( auto& bl : body_pf.Locals() )
-		if ( prior_vars.count(bl->Name()) > 0 )
-			locals.insert(bl);
-
-	if ( timeout_s )
-		{
-		ProfileFunc to_pf(timeout_s.get());
-		for ( auto& tl : to_pf.Locals() )
-			if ( prior_vars.count(tl->Name()) > 0 )
-				locals.insert(tl);
-		}
-
-	if ( locals.empty() )
-		return false;
-
-	std::string vars;
-	for ( auto& l : locals )
-		{
-		if ( ! vars.empty() )
-			vars += ", ";
-		vars += l->Name();
-		}
-
-	std::string msg = util::fmt("\"when\" statement referring to locals without an "
-	                            "explicit [] capture: %s",
-	                            vars.c_str());
-	ws->Error(msg.c_str());
-
-	return true;
-	}
-
 void WhenInfo::BuildInvokeElems()
 	{
 	one_const = make_intrusive<ConstExpr>(val_mgr->Count(1));
@@ -2116,12 +2035,12 @@ WhenStmt::WhenStmt(WhenInfo* arg_wi) : Stmt(STMT_WHEN), wi(arg_wi)
 	{
 	wi->Build(ThisPtr());
 
-	auto cond = wi->Cond();
+	auto cond = wi->OrigCond();
 
 	if ( ! cond->IsError() && ! IsBool(cond->GetType()->Tag()) )
 		cond->Error("conditional in test must be boolean");
 
-	auto te = wi->TimeoutExpr();
+	auto te = wi->OrigTimeout();
 
 	if ( te )
 		{
@@ -2148,8 +2067,6 @@ ValPtr WhenStmt::Exec(Frame* f, StmtFlowType& flow)
 
 	auto timeout = wi->TimeoutVal(f);
 
-	if ( wi->Captures() )
-		{
 	std::vector<ValPtr> local_aggrs;
 	for ( auto& l : wi->WhenExprLocals() )
 		{
@@ -2159,21 +2076,15 @@ ValPtr WhenStmt::Exec(Frame* f, StmtFlowType& flow)
 			local_aggrs.emplace_back(std::move(v));
 		}
 
-		new trigger::Trigger(wi, timeout, wi->WhenExprGlobals(), local_aggrs, f, location);
-		}
-
-	else
 	// The new trigger object will take care of its own deletion.
-		new trigger::Trigger(wi->Cond(), wi->WhenBody(), wi->TimeoutStmt(), timeout, f,
+	new trigger::Trigger(wi, timeout, wi->WhenExprGlobals(), local_aggrs, f, location);
-		                     wi->IsReturn(), location);
 
 	return nullptr;
 	}
 
 bool WhenStmt::IsPure() const
 	{
-	return wi->Cond()->IsPure() && wi->WhenBody()->IsPure() &&
+	return false;
-	       (! wi->TimeoutStmt() || wi->TimeoutStmt()->IsPure());
 	}
 
 void WhenStmt::StmtDescribe(ODesc* d) const
@@ -2183,35 +2094,35 @@ void WhenStmt::StmtDescribe(ODesc* d) const
 	if ( d->IsReadable() )
 		d->Add("(");
 
-	wi->Cond()->Describe(d);
+	wi->OrigCond()->Describe(d);
 
 	if ( d->IsReadable() )
 		d->Add(")");
 
 	d->SP();
 	d->PushIndent();
-	wi->WhenBody()->AccessStats(d);
+	wi->OrigBody()->AccessStats(d);
-	wi->WhenBody()->Describe(d);
+	wi->OrigBody()->Describe(d);
 	d->PopIndent();
 
-	if ( wi->TimeoutExpr() )
+	if ( wi->OrigTimeout() )
 		{
 		if ( d->IsReadable() )
 			{
 			d->SP();
 			d->Add("timeout");
 			d->SP();
-			wi->TimeoutExpr()->Describe(d);
+			wi->OrigTimeout()->Describe(d);
 			d->SP();
 			d->PushIndent();
-			wi->TimeoutStmt()->AccessStats(d);
+			wi->OrigTimeoutStmt()->AccessStats(d);
-			wi->TimeoutStmt()->Describe(d);
+			wi->OrigTimeoutStmt()->Describe(d);
 			d->PopIndent();
 			}
 		else
 			{
-			wi->TimeoutExpr()->Describe(d);
+			wi->OrigTimeout()->Describe(d);
-			wi->TimeoutStmt()->Describe(d);
+			wi->OrigTimeoutStmt()->Describe(d);
 			}
 		}
 	}
@@ -2231,22 +2142,22 @@ TraversalCode WhenStmt::Traverse(TraversalCallback* cb) const
 
 	else
 		{
-		tc = wi->Cond()->Traverse(cb);
+		tc = wi->OrigCond()->Traverse(cb);
 		HANDLE_TC_STMT_PRE(tc);
 
-		tc = wi->WhenBody()->Traverse(cb);
+		tc = wi->OrigBody()->Traverse(cb);
 		HANDLE_TC_STMT_PRE(tc);
 
-		if ( wi->TimeoutStmt() )
+		if ( wi->OrigTimeoutStmt() )
 			{
-			tc = wi->TimeoutStmt()->Traverse(cb);
+			tc = wi->OrigTimeoutStmt()->Traverse(cb);
 			HANDLE_TC_STMT_PRE(tc);
 			}
 		}
 
-	if ( wi->TimeoutExpr() )
+	if ( wi->OrigTimeout() )
 		{
-		tc = wi->TimeoutExpr()->Traverse(cb);
+		tc = wi->OrigTimeout()->Traverse(cb);
 		HANDLE_TC_STMT_PRE(tc);
 		}
 

src/Stmt.h

@@ -550,8 +550,7 @@ private:
 class WhenInfo
 	{
 public:
-	// Takes ownership of the CaptureList, which if nil signifies
+	// Takes ownership of the CaptureList.
-	// old-style frame semantics.
 	WhenInfo(ExprPtr cond, FuncType::CaptureList* cl, bool is_return);
 
 	// Constructor used by script optimization to create a stub.
@@ -582,18 +581,20 @@ public:
 	void Instantiate(Frame* f);
 	void Instantiate(ValPtr func);
 
-	// For old-style semantics, the following simply return the
+	// Return the original components used to construct the "when".
-	// individual "when" components.  For capture semantics, however,
+	const ExprPtr& OrigCond() const { return cond; }
-	// these instead return different invocations of a lambda that
+	const StmtPtr& OrigBody() const { return s; }
-	// manages the captures.
+	const ExprPtr& OrigTimeout() const { return timeout; }
+	const StmtPtr& OrigTimeoutStmt() const { return timeout_s; }
+
+	// Return different invocations of a lambda that manages the captures.
 	ExprPtr Cond();
 	StmtPtr WhenBody();
+	StmtPtr TimeoutStmt();
 
 	ExprPtr TimeoutExpr() const { return timeout; }
 	double TimeoutVal(Frame* f);
 
-	StmtPtr TimeoutStmt();
-
 	FuncType::CaptureList* Captures() { return cl; }
 
 	bool IsReturn() const { return is_return; }
@@ -605,18 +606,13 @@ public:
 	const IDSet& WhenExprGlobals() const { return when_expr_globals; }
 
 private:
-	// True if the "when" statement corresponds to old-style deprecated
-	// semantics (no captures, but needing captures).  Also triggers
-	// an error associated with "ws".
-	bool IsDeprecatedSemantics(StmtPtr ws);
-
 	// Build those elements we'll need for invoking our lambda.
 	void BuildInvokeElems();
 
 	ExprPtr cond;
 	StmtPtr s;
-	ExprPtr timeout;
 	StmtPtr timeout_s;
+	ExprPtr timeout;
 	FuncType::CaptureList* cl;
 
 	bool is_return = false;
@@ -648,10 +644,6 @@ private:
 
 	// Locals introduced via "local" in the "when" clause itself.
 	IDSet when_new_locals;
-
-	// Used for identifying deprecated instances.  Holds all of the local
-	// variables in the scope prior to parsing the "when" statement.
-	std::map<std::string, IDPtr, std::less<>> prior_vars;
 	};
 
 class WhenStmt final : public Stmt

src/Trigger.cc

@@ -99,13 +99,6 @@ protected:
 	double time;
 	};
 
-Trigger::Trigger(ExprPtr cond, StmtPtr body, StmtPtr timeout_stmts, double timeout, Frame* frame,
-                 bool is_return, const Location* location)
-	{
-	timeout_value = timeout;
-	Init(cond, body, timeout_stmts, frame, is_return, location);
-	}
-
 Trigger::Trigger(WhenInfo* wi, double timeout, const IDSet& _globals,
                  std::vector<ValPtr> _local_aggrs, Frame* f, const Location* loc)
 	{
@@ -114,34 +107,29 @@ Trigger::Trigger(WhenInfo* wi, double timeout, const IDSet& _globals,
 	local_aggrs = std::move(_local_aggrs);
 	have_trigger_elems = true;
 
-	Init(wi->Cond(), wi->WhenBody(), wi->TimeoutStmt(), f, wi->IsReturn(), loc);
+	cond = wi->Cond();
-	}
+	body = wi->WhenBody();
+	timeout_stmts = wi->TimeoutStmt();
+	is_return = wi->IsReturn();
 
-void Trigger::Init(ExprPtr arg_cond, StmtPtr arg_body, StmtPtr arg_timeout_stmts, Frame* arg_frame,
-                   bool arg_is_return, const Location* location)
-	{
-	cond = arg_cond;
-	body = arg_body;
-	timeout_stmts = arg_timeout_stmts;
 	timer = nullptr;
 	delayed = false;
 	disabled = false;
 	attached = nullptr;
-	is_return = arg_is_return;
 
 	if ( location )
 		name = util::fmt("%s:%d-%d", location->filename, location->first_line, location->last_line);
 	else
 		name = "<no-trigger-location>";
 
-	if ( arg_frame )
+	if ( f )
-		frame = arg_frame->Clone();
+		frame = f->CloneForTrigger();
 	else
 		frame = nullptr;
 
 	DBG_LOG(DBG_NOTIFIERS, "%s: instantiating", Name());
 
-	if ( is_return && frame && arg_frame )
+	if ( is_return && frame )
 		{
 		Trigger* parent = frame->GetTrigger();
 		if ( ! parent )
@@ -152,7 +140,7 @@ void Trigger::Init(ExprPtr arg_cond, StmtPtr arg_body, StmtPtr arg_timeout_stmts
 			}
 
 		parent->Attach(this);
-		arg_frame->SetDelayed();
+		f->SetDelayed();
 		}
 
 	// Make sure we don't get deleted if somebody calls a method like
@@ -262,7 +250,7 @@ bool Trigger::Eval()
 
 	try
 		{
-		f = frame->Clone();
+		f = frame->CloneForTrigger();
 		}
 	catch ( InterpreterException& )
 		{
@@ -364,7 +352,7 @@ void Trigger::Timeout()
 	if ( timeout_stmts )
 		{
 		StmtFlowType flow;
-		FramePtr f{AdoptRef{}, frame->Clone()};
+		FramePtr f{AdoptRef{}, frame->CloneForTrigger()};
 		ValPtr v;
 
 		try

src/Trigger.h

@@ -48,11 +48,6 @@ public:
 	// statements are executed immediately and the object is deleted
 	// right away.
 
-	// These first constructor is for the deprecated deep-copy semantics.
-	Trigger(ExprPtr cond, StmtPtr body, StmtPtr timeout_stmts, double timeout, Frame* f,
-	        bool is_return, const Location* loc);
-
-	// Used for capture-list semantics.
 	Trigger(WhenInfo* wi, double timeout, const IDSet& globals, std::vector<ValPtr> local_aggrs,
 	        Frame* f, const Location* loc);
 
@@ -112,9 +107,6 @@ public:
 private:
 	friend class TriggerTimer;
 
-	void Init(ExprPtr cond, StmtPtr body, StmtPtr timeout_stmts, Frame* frame, bool is_return,
-	          const Location* location);
-
 	void ReInit(std::vector<ValPtr> index_expr_results);
 
 	void Register(const ID* id);

src/Var.cc

@@ -842,24 +842,25 @@ void end_func(StmtPtr body, const char* module_name, bool free_of_conditionals)
 	oi->num_stmts = Stmt::GetNumStmts();
 	oi->num_exprs = Expr::GetNumExprs();
 
-	auto ingredients = std::make_unique<function_ingredients>(pop_scope(), std::move(body),
+	auto ingredients = std::make_unique<FunctionIngredients>(pop_scope(), std::move(body),
 	                                                         module_name);
-	if ( ! ingredients->id->HasVal() )
+	auto id = ingredients->GetID();
+	if ( ! id->HasVal() )
 		{
-		auto f = make_intrusive<ScriptFunc>(ingredients->id);
+		auto f = make_intrusive<ScriptFunc>(id);
-		ingredients->id->SetVal(make_intrusive<FuncVal>(std::move(f)));
+		id->SetVal(make_intrusive<FuncVal>(std::move(f)));
-		ingredients->id->SetConst();
+		id->SetConst();
 		}
 
-	ingredients->id->GetVal()->AsFunc()->AddBody(ingredients->body, ingredients->inits,
+	id->GetVal()->AsFunc()->AddBody(ingredients->Body(), ingredients->Inits(),
-	                                             ingredients->frame_size, ingredients->priority,
+	                                ingredients->FrameSize(), ingredients->Priority(),
-	                                             ingredients->groups);
+	                                ingredients->Groups());
 
-	auto func_ptr = cast_intrusive<FuncVal>(ingredients->id->GetVal())->AsFuncPtr();
+	auto func_ptr = cast_intrusive<FuncVal>(id->GetVal())->AsFuncPtr();
 	auto func = cast_intrusive<ScriptFunc>(func_ptr);
-	func->SetScope(ingredients->scope);
+	func->SetScope(ingredients->Scope());
 
-	for ( const auto& group : ingredients->groups )
+	for ( const auto& group : ingredients->Groups() )
 		group->AddFunc(func);
 
 	analyze_func(std::move(func));

src/parse.y

@@ -1572,9 +1572,9 @@ lambda_body:
 
 			// Gather the ingredients for a Func from the
 			// current scope.
-			auto ingredients = std::make_unique<function_ingredients>(
+			auto ingredients = std::make_unique<FunctionIngredients>(
 				current_scope(), IntrusivePtr{AdoptRef{}, $3}, current_module.c_str());
-			auto outer_ids = gather_outer_ids(pop_scope(), ingredients->body);
+			auto outer_ids = gather_outer_ids(pop_scope(), ingredients->Body());
 
 			$$ = new LambdaExpr(std::move(ingredients), std::move(outer_ids));
 			}

src/script_opt/CPP/DeclFunc.cc

@@ -29,8 +29,8 @@ void CPPCompile::DeclareLambda(const LambdaExpr* l, const ProfileFunc* pf)
 	ASSERT(is_CPP_compilable(pf));
 
 	auto lname = Canonicalize(l->Name().c_str()) + "_lb";
-	auto body = l->Ingredients().body;
+	auto body = l->Ingredients().Body();
-	auto l_id = l->Ingredients().id;
+	auto l_id = l->Ingredients().GetID();
 	auto& ids = l->OuterIDs();
 
 	for ( auto id : ids )

src/script_opt/CPP/Driver.cc

@@ -150,7 +150,7 @@ void CPPCompile::Compile(bool report_uncompilable)
 	for ( const auto& l : pfs.Lambdas() )
 		{
 		const auto& n = l->Name();
-		const auto body = l->Ingredients().body.get();
+		const auto body = l->Ingredients().Body().get();
 		if ( lambda_ASTs.count(n) > 0 )
 			// Reuse previous body.
 			body_names[body] = body_names[lambda_ASTs[n]];
@@ -176,7 +176,7 @@ void CPPCompile::Compile(bool report_uncompilable)
 			continue;
 
 		CompileLambda(l, pfs.ExprProf(l).get());
-		lambda_ASTs[n] = l->Ingredients().body.get();
+		lambda_ASTs[n] = l->Ingredients().Body().get();
 		}
 
 	NL();

src/script_opt/CPP/GenFunc.cc

@@ -23,8 +23,8 @@ void CPPCompile::CompileFunc(const FuncInfo& func)
 void CPPCompile::CompileLambda(const LambdaExpr* l, const ProfileFunc* pf)
 	{
 	auto lname = Canonicalize(l->Name().c_str()) + "_lb";
-	auto body = l->Ingredients().body;
+	auto body = l->Ingredients().Body();
-	auto l_id = l->Ingredients().id;
+	auto l_id = l->Ingredients().GetID();
 	auto& ids = l->OuterIDs();
 
 	DefineBody(l_id->GetType<FuncType>(), pf, lname, body, &ids, FUNC_FLAVOR_FUNCTION);

src/script_opt/Expr.cc

@@ -2415,8 +2415,8 @@ StmtPtr CallExpr::ReduceToSingletons(Reducer* c)
 
 ExprPtr LambdaExpr::Duplicate()
 	{
-	auto ingr = std::make_unique<function_ingredients>(*ingredients);
+	auto ingr = std::make_unique<FunctionIngredients>(*ingredients);
-	ingr->body = ingr->body->Duplicate();
+	ingr->SetBody(ingr->Body()->Duplicate());
 	return SetSucc(new LambdaExpr(std::move(ingr), outer_ids));
 	}
 

src/script_opt/ProfileFunc.cc

@@ -51,7 +51,7 @@ ProfileFunc::ProfileFunc(const Expr* e, bool _abs_rec_fields)
 		for ( auto oid : func->OuterIDs() )
 			captures.insert(oid);
 
-		Profile(func->GetType()->AsFuncType(), func->Ingredients().body);
+		Profile(func->GetType()->AsFuncType(), func->Ingredients().Body());
 		}
 
 	else

src/script_opt/ZAM/Compile.h

@@ -137,7 +137,6 @@ private:
 	const ZAMStmt CompileCatchReturn(const CatchReturnStmt* cr);
 	const ZAMStmt CompileStmts(const StmtList* sl);
 	const ZAMStmt CompileInit(const InitStmt* is);
-	const ZAMStmt CompileWhen(const WhenStmt* ws);
 
 	const ZAMStmt CompileNext() { return GenGoTo(nexts.back()); }
 	const ZAMStmt CompileBreak() { return GenGoTo(breaks.back()); }

src/script_opt/ZAM/Ops.in

@@ -1845,28 +1845,6 @@ type V
 eval    (*tiv_ptr)[z.v1].Clear();
 
 
-
-op When
-op1-read
-type VVVV
-eval	auto when_body = make_intrusive<ZAMResumption>(this, z.v2);
-	auto timeout_body = make_intrusive<ZAMResumption>(this, z.v3);
-	ExprPtr when_cond = {NewRef{}, const_cast<Expr*>(z.e)};
-	new trigger::Trigger(when_cond, when_body, timeout_body, frame[z.v1].double_val, f, z.v4, z.loc);
-
-op When
-type VVVC
-eval	auto when_body = make_intrusive<ZAMResumption>(this, z.v1);
-	auto timeout_body = make_intrusive<ZAMResumption>(this, z.v2);
-	ExprPtr when_cond = {NewRef{}, const_cast<Expr*>(z.e)};
-	new trigger::Trigger(when_cond, when_body, timeout_body, z.c.double_val, f, z.v3, z.loc);
-
-op When
-type VV
-eval	auto when_body = make_intrusive<ZAMResumption>(this, z.v2);
-	ExprPtr when_cond = {NewRef{}, const_cast<Expr*>(z.e)};
-	new trigger::Trigger(when_cond, when_body, nullptr, -1.0, f, z.v1, z.loc);
-
 op CheckAnyLen
 op1-read
 type Vi

src/script_opt/ZAM/Stmt.cc

@@ -63,9 +63,6 @@ const ZAMStmt ZAMCompiler::CompileStmt(const Stmt* s)
 		case STMT_NULL:
 			return EmptyStmt();
 
-		case STMT_WHEN:
-			return CompileWhen(static_cast<const WhenStmt*>(s));
-
 		case STMT_CHECK_ANY_LEN:
 			{
 			auto cs = static_cast<const CheckAnyLenStmt*>(s);
@@ -1125,78 +1122,4 @@ const ZAMStmt ZAMCompiler::InitTable(IDPtr id, TableType* tt, Attributes* attrs)
 	return AddInst(z);
 	}
 
-const ZAMStmt ZAMCompiler::CompileWhen(const WhenStmt* ws)
-	{
-	auto cond = ws->Cond();
-	auto body = ws->Body();
-	auto timeout = ws->TimeoutExpr();
-	auto timeout_body = ws->TimeoutBody();
-	auto is_return = ws->IsReturn();
-
-	ZInstI z;
-
-	if ( timeout )
-		{
-		// Note, we fill in is_return by hand since it's already
-		// an int_val, doesn't need translation.
-		if ( timeout->Tag() == EXPR_CONST )
-			{
-			z = GenInst(OP_WHEN_VVVC, timeout->AsConstExpr());
-			z.op_type = OP_VVVC_I1_I2_I3;
-			z.v3 = is_return;
-			}
-		else
-			{
-			z = GenInst(OP_WHEN_VVVV, timeout->AsNameExpr());
-			z.op_type = OP_VVVV_I2_I3_I4;
-			z.v4 = is_return;
-			}
-		}
-
-	else
-		{
-		z = GenInst(OP_WHEN_VV);
-		z.op_type = OP_VV_I1_I2;
-		z.v1 = is_return;
-		}
-
-	z.e = cond.get();
-
-	auto when_eval = AddInst(z);
-
-	auto branch_past_blocks = GoToStub();
-
-	auto when_body = CompileStmt(body);
-	auto when_done = ReturnX();
-
-	if ( timeout )
-		{
-		auto t_body = CompileStmt(timeout_body);
-		auto t_done = ReturnX();
-
-		if ( timeout->Tag() == EXPR_CONST )
-			{
-			SetV1(when_eval, GoToTargetBeyond(branch_past_blocks));
-			SetV2(when_eval, GoToTargetBeyond(when_done));
-			}
-		else
-			{
-			SetV2(when_eval, GoToTargetBeyond(branch_past_blocks));
-			SetV3(when_eval, GoToTargetBeyond(when_done));
-			}
-
-		SetGoTo(branch_past_blocks, GoToTargetBeyond(t_done));
-
-		return t_done;
-		}
-
-	else
-		{
-		SetV2(when_eval, GoToTargetBeyond(branch_past_blocks));
-		SetGoTo(branch_past_blocks, GoToTargetBeyond(when_done));
-
-		return when_done;
-		}
-	}
-
 	} // zeek::detail

testing/btest/Baseline/language.when-capture-errors/out

@@ -1,5 +1,5 @@
 ### BTest baseline data generated by btest-diff. Do not edit. Use "btest -U/-u" to update. Requires BTest >= 0.63.
-error in <...>/when-capture-errors.zeek, lines 19-22: "when" statement referring to locals without an explicit [] capture: orig1 (when (0 < g) { print orig1})
+error in <...>/when-capture-errors.zeek, lines 19-22: orig1 is used inside "when" statement but not captured (when (0 < g) { print orig1})
 error in <...>/when-capture-errors.zeek, lines 25-28: orig3 is used inside "when" statement but not captured (when (0 < g || orig3) { print g})
 error in <...>/when-capture-errors.zeek, lines 34-38: orig1 is used inside "when" statement but not captured (when (0 < g) { print g} timeout 1.0 sec { print orig1})
 error in <...>/when-capture-errors.zeek, lines 66-70: orig2 is used inside "when" statement but not captured (when (0 < g) { print orig1} timeout 1.0 sec { print orig2})

testing/btest/language/when-capture-errors.zeek

@@ -15,7 +15,7 @@ event zeek_init()
 		print g;
 		}
 
-	# Should generate a deprecation warning.
+	# Should generate an error.
 	when ( g > 0 )
 		{
 		print orig1;