From 0c434ca4f8f8deb8599fcca8dafd6849045a0f0e Mon Sep 17 00:00:00 2001 From: Vern Paxson Date: Sun, 2 Apr 2023 11:34:16 -0700 Subject: [PATCH 1/5] changed function_ingredients struct to FunctionIngredients class with accessors --- src/Expr.cc | 24 ++++++++++++------------ src/Expr.h | 8 ++++---- src/Func.cc | 18 ++++++++---------- src/Func.h | 19 +++++++++++++++---- src/Var.cc | 25 +++++++++++++------------ src/parse.y | 4 ++-- src/script_opt/CPP/DeclFunc.cc | 4 ++-- src/script_opt/CPP/Driver.cc | 4 ++-- src/script_opt/CPP/GenFunc.cc | 4 ++-- src/script_opt/Expr.cc | 4 ++-- src/script_opt/ProfileFunc.cc | 2 +- 11 files changed, 63 insertions(+), 53 deletions(-) diff --git a/src/Expr.cc b/src/Expr.cc index 9df823c1a8..f37669eae1 100644 --- a/src/Expr.cc +++ b/src/Expr.cc @@ -4624,14 +4624,14 @@ void CallExpr::ExprDescribe(ODesc* d) const args->Describe(d); } -LambdaExpr::LambdaExpr(std::unique_ptr arg_ing, IDPList arg_outer_ids, +LambdaExpr::LambdaExpr(std::unique_ptr 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 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(ingredients->id); - dummy_func->AddBody(ingredients->body, ingredients->inits, ingredients->frame_size, - ingredients->priority); + auto dummy_func = make_intrusive(ingredients->GetID()); + dummy_func->AddBody(ingredients->Body(), ingredients->Inits(), ingredients->FrameSize(), + ingredients->Priority()); dummy_func->SetOuterIDs(outer_ids); @@ -4678,7 +4678,7 @@ LambdaExpr::LambdaExpr(std::unique_ptr arg_ing, IDPList ar auto v = make_intrusive(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(ingredients->id); - lamb->AddBody(ingredients->body, ingredients->inits, ingredients->frame_size, - ingredients->priority); + auto lamb = make_intrusive(ingredients->GetID()); + lamb->AddBody(ingredients->Body(), ingredients->Inits(), ingredients->FrameSize(), + 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); diff --git a/src/Expr.h b/src/Expr.h index f73b8f30d8..73658b6e47 100644 --- a/src/Expr.h +++ b/src/Expr.h @@ -27,7 +27,7 @@ namespace detail class Frame; class Scope; -struct function_ingredients; +class FunctionIngredients; using IDPtr = IntrusivePtr; using ScopePtr = IntrusivePtr; @@ -1454,12 +1454,12 @@ protected: class LambdaExpr final : public Expr { public: - LambdaExpr(std::unique_ptr ingredients, IDPList outer_ids, + LambdaExpr(std::unique_ptr 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 ingredients; + std::unique_ptr ingredients; IDPtr lambda_id; IDPList outer_ids; diff --git a/src/Func.cc b/src/Func.cc index c64928fbc9..7cbfaf764a 100644 --- a/src/Func.cc +++ b/src/Func.cc @@ -862,16 +862,18 @@ static std::set get_func_groups(const std::vector& attrs return groups; } -function_ingredients::function_ingredients(ScopePtr scope, StmtPtr body, - const std::string& module_name) +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 = this->scope->GetID(); + id = 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()->Flavor(); if ( flavor == FUNC_FLAVOR_EVENT || flavor == FUNC_FLAVOR_HOOK ) { - auto module_group = event_registry->RegisterGroup(EventGroupKind::Module, - this->module_name); + auto module_group = event_registry->RegisterGroup(EventGroupKind::Module, module_name); groups.insert(module_group); } } diff --git a/src/Func.h b/src/Func.h index ef6151829b..a27eb892b7 100644 --- a/src/Func.h +++ b/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 inits; size_t frame_size = 0; int priority = 0; diff --git a/src/Var.cc b/src/Var.cc index 497cda8e60..587ab9caf4 100644 --- a/src/Var.cc +++ b/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(pop_scope(), std::move(body), - module_name); - if ( ! ingredients->id->HasVal() ) + auto ingredients = std::make_unique(pop_scope(), std::move(body), + module_name); + auto id = ingredients->GetID(); + if ( ! id->HasVal() ) { - auto f = make_intrusive(ingredients->id); - ingredients->id->SetVal(make_intrusive(std::move(f))); - ingredients->id->SetConst(); + auto f = make_intrusive(id); + id->SetVal(make_intrusive(std::move(f))); + id->SetConst(); } - ingredients->id->GetVal()->AsFunc()->AddBody(ingredients->body, ingredients->inits, - ingredients->frame_size, ingredients->priority, - ingredients->groups); + id->GetVal()->AsFunc()->AddBody(ingredients->Body(), ingredients->Inits(), + ingredients->FrameSize(), ingredients->Priority(), + ingredients->Groups()); - auto func_ptr = cast_intrusive(ingredients->id->GetVal())->AsFuncPtr(); + auto func_ptr = cast_intrusive(id->GetVal())->AsFuncPtr(); auto func = cast_intrusive(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)); diff --git a/src/parse.y b/src/parse.y index 92a22c81d7..e7fe98c074 100644 --- a/src/parse.y +++ b/src/parse.y @@ -1572,9 +1572,9 @@ lambda_body: // Gather the ingredients for a Func from the // current scope. - auto ingredients = std::make_unique( + auto ingredients = std::make_unique( 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)); } diff --git a/src/script_opt/CPP/DeclFunc.cc b/src/script_opt/CPP/DeclFunc.cc index 177e8af91d..f46fe6b76c 100644 --- a/src/script_opt/CPP/DeclFunc.cc +++ b/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 l_id = l->Ingredients().id; + auto body = l->Ingredients().Body(); + auto l_id = l->Ingredients().GetID(); auto& ids = l->OuterIDs(); for ( auto id : ids ) diff --git a/src/script_opt/CPP/Driver.cc b/src/script_opt/CPP/Driver.cc index 236baeb0d2..de6396a19a 100644 --- a/src/script_opt/CPP/Driver.cc +++ b/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(); diff --git a/src/script_opt/CPP/GenFunc.cc b/src/script_opt/CPP/GenFunc.cc index f4071c7deb..d6076453cc 100644 --- a/src/script_opt/CPP/GenFunc.cc +++ b/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 l_id = l->Ingredients().id; + auto body = l->Ingredients().Body(); + auto l_id = l->Ingredients().GetID(); auto& ids = l->OuterIDs(); DefineBody(l_id->GetType(), pf, lname, body, &ids, FUNC_FLAVOR_FUNCTION); diff --git a/src/script_opt/Expr.cc b/src/script_opt/Expr.cc index ec62756e86..c8ef874880 100644 --- a/src/script_opt/Expr.cc +++ b/src/script_opt/Expr.cc @@ -2415,8 +2415,8 @@ StmtPtr CallExpr::ReduceToSingletons(Reducer* c) ExprPtr LambdaExpr::Duplicate() { - auto ingr = std::make_unique(*ingredients); - ingr->body = ingr->body->Duplicate(); + auto ingr = std::make_unique(*ingredients); + ingr->SetBody(ingr->Body()->Duplicate()); return SetSucc(new LambdaExpr(std::move(ingr), outer_ids)); } diff --git a/src/script_opt/ProfileFunc.cc b/src/script_opt/ProfileFunc.cc index 745ab48f88..1da75df4f0 100644 --- a/src/script_opt/ProfileFunc.cc +++ b/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 From 4af6b52876f42714cd157ac8204cdada34d100e8 Mon Sep 17 00:00:00 2001 From: Vern Paxson Date: Sun, 2 Apr 2023 11:35:15 -0700 Subject: [PATCH 2/5] introduced notion of light-weight Frame clones --- src/Frame.cc | 11 +++++++++++ src/Frame.h | 8 ++++++++ 2 files changed, 19 insertions(+) diff --git a/src/Frame.cc b/src/Frame.cc index 44fb4697f7..87ba277427 100644 --- a/src/Frame.cc +++ b/src/Frame.cc @@ -120,6 +120,17 @@ Frame* Frame::Clone() const return other; } +Frame* Frame::LightClone() 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 ) diff --git a/src/Frame.h b/src/Frame.h index d403b2ddad..77a5e89a30 100644 --- a/src/Frame.h +++ b/src/Frame.h @@ -157,6 +157,14 @@ public: */ Frame* Clone() const; + /** + * Creates a copy of the frame that doesn't include its values, + * just its trigger context. + * + * @return a partial copy of this frame. + */ + Frame* LightClone() const; + /** * Serializes the frame in support of copy semantics for lambdas: * From 84906171bad42c5f27003d758ca36f759cb5480a Mon Sep 17 00:00:00 2001 From: Vern Paxson Date: Sun, 2 Apr 2023 11:36:39 -0700 Subject: [PATCH 3/5] simplify WhenInfo and Trigger classes given removal of old capture semantics --- src/Stmt.cc | 173 ++++++++++++------------------------------------- src/Stmt.h | 28 +++----- src/Trigger.cc | 32 +++------ src/Trigger.h | 8 --- 4 files changed, 62 insertions(+), 179 deletions(-) diff --git a/src/Stmt.cc b/src/Stmt.cc index 1dad9ef5d1..1150740a9d 100644 --- a/src/Stmt.cc +++ b/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,20 +1882,17 @@ 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 ) - { - is_present = true; - break; - } - - if ( ! is_present ) + for ( auto& c : *cl ) + if ( c.id == wl ) { - IDPtr wl_ptr = {NewRef{}, const_cast(wl)}; - cl->emplace_back(FuncType::Capture{wl_ptr, false}); + is_present = true; + break; } + + if ( ! is_present ) + { + IDPtr wl_ptr = {NewRef{}, const_cast(wl)}; + cl->emplace_back(FuncType::Capture{wl_ptr, false}); } // In addition, don't treat them as external locals that @@ -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(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(do_test, do_bodies, dummy_return); - auto ingredients = std::make_unique(current_scope(), shebang, - current_module); - auto outer_ids = gather_outer_ids(pop_scope(), ingredients->body); + auto ingredients = std::make_unique(current_scope(), shebang, + current_module); + auto outer_ids = gather_outer_ids(pop_scope(), ingredients->Body()); lambda = make_intrusive(std::move(ingredients), std::move(outer_ids), ws); } void WhenInfo::Instantiate(Frame* f) { - if ( cl ) - Instantiate(lambda->Eval(f)); + Instantiate(lambda->Eval(f)); } void WhenInfo::Instantiate(ValPtr func) { - if ( cl ) - curr_lambda = make_intrusive(std::move(func)); + curr_lambda = make_intrusive(std::move(func)); } ExprPtr WhenInfo::Cond() { - if ( ! curr_lambda ) - return cond; - return make_intrusive(curr_lambda, invoke_cond); } StmtPtr WhenInfo::WhenBody() { - if ( ! curr_lambda ) - return s; - auto invoke = make_intrusive(curr_lambda, invoke_s); return make_intrusive(invoke, true); } @@ -2046,61 +2016,10 @@ double WhenInfo::TimeoutVal(Frame* f) StmtPtr WhenInfo::TimeoutStmt() { - if ( ! curr_lambda ) - return timeout_s; - auto invoke = make_intrusive(curr_lambda, invoke_timeout); return make_intrusive(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(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,32 +2067,24 @@ ValPtr WhenStmt::Exec(Frame* f, StmtFlowType& flow) auto timeout = wi->TimeoutVal(f); - if ( wi->Captures() ) + std::vector local_aggrs; + for ( auto& l : wi->WhenExprLocals() ) { - std::vector local_aggrs; - for ( auto& l : wi->WhenExprLocals() ) - { - IDPtr l_ptr = {NewRef{}, const_cast(l)}; - auto v = f->GetElementByID(l_ptr); - if ( v && v->Modifiable() ) - local_aggrs.emplace_back(std::move(v)); - } - - new trigger::Trigger(wi, timeout, wi->WhenExprGlobals(), local_aggrs, f, location); + IDPtr l_ptr = {NewRef{}, const_cast(l)}; + auto v = f->GetElementByID(l_ptr); + if ( v && v->Modifiable() ) + local_aggrs.emplace_back(std::move(v)); } - else - // The new trigger object will take care of its own deletion. - new trigger::Trigger(wi->Cond(), wi->WhenBody(), wi->TimeoutStmt(), timeout, f, - wi->IsReturn(), location); + // The new trigger object will take care of its own deletion. + new trigger::Trigger(wi, timeout, wi->WhenExprGlobals(), local_aggrs, f, location); return nullptr; } bool WhenStmt::IsPure() const { - return wi->Cond()->IsPure() && wi->WhenBody()->IsPure() && - (! wi->TimeoutStmt() || wi->TimeoutStmt()->IsPure()); + return false; } 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->WhenBody()->Describe(d); + wi->OrigBody()->AccessStats(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->TimeoutStmt()->Describe(d); + wi->OrigTimeoutStmt()->AccessStats(d); + wi->OrigTimeoutStmt()->Describe(d); d->PopIndent(); } else { - wi->TimeoutExpr()->Describe(d); - wi->TimeoutStmt()->Describe(d); + wi->OrigTimeout()->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); } diff --git a/src/Stmt.h b/src/Stmt.h index be23b8786e..5625b54502 100644 --- a/src/Stmt.h +++ b/src/Stmt.h @@ -550,8 +550,7 @@ private: class WhenInfo { public: - // Takes ownership of the CaptureList, which if nil signifies - // old-style frame semantics. + // Takes ownership of the CaptureList. 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 - // individual "when" components. For capture semantics, however, - // these instead return different invocations of a lambda that - // manages the captures. + // Return the original components used to construct the "when". + const ExprPtr& OrigCond() const { return cond; } + const StmtPtr& OrigBody() const { return s; } + 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> prior_vars; }; class WhenStmt final : public Stmt diff --git a/src/Trigger.cc b/src/Trigger.cc index e23509088a..5015952499 100644 --- a/src/Trigger.cc +++ b/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 _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 = ""; - if ( arg_frame ) - frame = arg_frame->Clone(); + if ( f ) + frame = f->LightClone(); 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->LightClone(); } catch ( InterpreterException& ) { @@ -364,7 +352,7 @@ void Trigger::Timeout() if ( timeout_stmts ) { StmtFlowType flow; - FramePtr f{AdoptRef{}, frame->Clone()}; + FramePtr f{AdoptRef{}, frame->LightClone()}; ValPtr v; try diff --git a/src/Trigger.h b/src/Trigger.h index 819979351f..e1c228fe33 100644 --- a/src/Trigger.h +++ b/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 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 index_expr_results); void Register(const ID* id); From 61891e615a28e6d73cab6a245b2b818893fc4837 Mon Sep 17 00:00:00 2001 From: Vern Paxson Date: Sun, 2 Apr 2023 11:37:36 -0700 Subject: [PATCH 4/5] removed skeletal (non-functioning) "when" support from ZAM --- src/script_opt/ZAM/Compile.h | 1 - src/script_opt/ZAM/Ops.in | 22 ----------- src/script_opt/ZAM/Stmt.cc | 77 ------------------------------------ 3 files changed, 100 deletions(-) diff --git a/src/script_opt/ZAM/Compile.h b/src/script_opt/ZAM/Compile.h index 071521b6a7..e1cd35bf2b 100644 --- a/src/script_opt/ZAM/Compile.h +++ b/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()); } diff --git a/src/script_opt/ZAM/Ops.in b/src/script_opt/ZAM/Ops.in index c0d756d97d..b6b09c7645 100644 --- a/src/script_opt/ZAM/Ops.in +++ b/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(this, z.v2); - auto timeout_body = make_intrusive(this, z.v3); - ExprPtr when_cond = {NewRef{}, const_cast(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(this, z.v1); - auto timeout_body = make_intrusive(this, z.v2); - ExprPtr when_cond = {NewRef{}, const_cast(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(this, z.v2); - ExprPtr when_cond = {NewRef{}, const_cast(z.e)}; - new trigger::Trigger(when_cond, when_body, nullptr, -1.0, f, z.v1, z.loc); - op CheckAnyLen op1-read type Vi diff --git a/src/script_opt/ZAM/Stmt.cc b/src/script_opt/ZAM/Stmt.cc index 1e3daedee8..d35c279c38 100644 --- a/src/script_opt/ZAM/Stmt.cc +++ b/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(s)); - case STMT_CHECK_ANY_LEN: { auto cs = static_cast(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 From 910b50ef0d256ce6de22c7674b49fcf1100b29d2 Mon Sep 17 00:00:00 2001 From: Vern Paxson Date: Sun, 2 Apr 2023 11:38:30 -0700 Subject: [PATCH 5/5] test suite update for minor change in "when" error messages --- testing/btest/Baseline/language.when-capture-errors/out | 2 +- testing/btest/language/when-capture-errors.zeek | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/testing/btest/Baseline/language.when-capture-errors/out b/testing/btest/Baseline/language.when-capture-errors/out index f33e6bc1ca..d73d761602 100644 --- a/testing/btest/Baseline/language.when-capture-errors/out +++ b/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}) diff --git a/testing/btest/language/when-capture-errors.zeek b/testing/btest/language/when-capture-errors.zeek index 85b0abb393..dccd4c9383 100644 --- a/testing/btest/language/when-capture-errors.zeek +++ b/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;