From 48f1e4df42189e2196e600f4461b99029d228ffe Mon Sep 17 00:00:00 2001 From: Vern Paxson Date: Thu, 12 May 2022 14:05:20 -0700 Subject: [PATCH] Change "when" statements that don't require closures to use new implementation. Provide hooks for script optimization access to "when" statements. Regularize treatment of naming and timeouts for Triggers. --- src/Stmt.cc | 203 +++++++++++++++++++++++++++++++------------------ src/Stmt.h | 43 +++++++++-- src/Trigger.cc | 49 +++--------- src/Trigger.h | 17 ++--- 4 files changed, 183 insertions(+), 129 deletions(-) diff --git a/src/Stmt.cc b/src/Stmt.cc index fb9005c148..f578553c86 100644 --- a/src/Stmt.cc +++ b/src/Stmt.cc @@ -1808,33 +1808,30 @@ WhenInfo::WhenInfo(ExprPtr _cond, FuncType::CaptureList* _cl, bool _is_return) ProfileFunc cond_pf(cond.get()); - if ( ! cl ) - { - for ( auto& wl : cond_pf.WhenLocals() ) - prior_vars.erase(wl->Name()); - return; - } - when_expr_locals = cond_pf.Locals(); - when_expr_globals = cond_pf.Globals(); + when_expr_globals = cond_pf.AllGlobals(); + when_new_locals = cond_pf.WhenLocals(); // Make any when-locals part of our captures, if not already present, // to enable sharing between the condition and the body/timeout code. - for ( auto& wl : cond_pf.WhenLocals() ) + for ( auto& wl : when_new_locals ) { bool is_present = false; - for ( auto& c : *cl ) - if ( c.id == wl ) - { - is_present = true; - break; - } - - if ( ! is_present ) + if ( cl ) { - IDPtr wl_ptr = {NewRef{}, const_cast(wl)}; - cl->emplace_back(FuncType::Capture{wl_ptr, false}); + for ( auto& c : *cl ) + if ( c.id == wl ) + { + 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 @@ -1851,62 +1848,50 @@ WhenInfo::WhenInfo(ExprPtr _cond, FuncType::CaptureList* _cl, bool _is_return) param_list->push_back(new TypeDecl(util::copy_string(lambda_param_id.c_str()), count_t)); auto params = make_intrusive(param_list); - auto ft = make_intrusive(params, base_type(TYPE_ANY), FUNC_FLAVOR_FUNCTION); - ft->SetCaptures(*cl); + lambda_ft = make_intrusive(params, base_type(TYPE_ANY), FUNC_FLAVOR_FUNCTION); if ( ! is_return ) - ft->SetExpressionlessReturnOkay(true); + lambda_ft->SetExpressionlessReturnOkay(true); auto id = current_scope()->GenerateTemporary("when-internal"); + id->SetType(lambda_ft); + push_scope(std::move(id), nullptr); - // This begin_func will be completed by WhenInfo::Build(). - begin_func(id, current_module.c_str(), FUNC_FLAVOR_FUNCTION, false, ft); + auto arg_id = install_ID(lambda_param_id.c_str(), current_module.c_str(), false, false); + arg_id->SetType(count_t); + } + +WhenInfo::WhenInfo(bool _is_return) : is_return(_is_return) + { + // This won't be needed once we remove the deprecated semantics. + cl = new zeek::FuncType::CaptureList; + BuildInvokeElems(); } void WhenInfo::Build(StmtPtr ws) { - if ( ! cl ) + if ( IsDeprecatedSemantics(ws) ) { - // Old-style semantics. - auto locals = when_expr_locals; - - ProfileFunc cond_pf(cond.get()); - for ( auto& bl : cond_pf.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() ) - { - 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 is deprecated: %s", - vars.c_str()); - ws->Warn(msg.c_str()); - } - + pop_scope(); 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 // the values of captures are shared across all of its elements) // that's used for all three of the "when" components: condition, @@ -1924,14 +1909,9 @@ void WhenInfo::Build(StmtPtr ws) // Build the AST elements of the lambda. // First, the constants we'll need. - auto true_const = make_intrusive(val_mgr->True()); - auto one_const = make_intrusive(val_mgr->Count(1)); - auto two_const = make_intrusive(val_mgr->Count(2)); - auto three_const = make_intrusive(val_mgr->Count(3)); + BuildInvokeElems(); - invoke_cond = make_intrusive(one_const); - invoke_s = make_intrusive(two_const); - invoke_timeout = make_intrusive(three_const); + auto true_const = make_intrusive(val_mgr->True()); // Access to the parameter that selects which action we're doing. auto param_id = lookup_ID(lambda_param_id.c_str(), current_module.c_str()); @@ -1963,7 +1943,13 @@ void WhenInfo::Build(StmtPtr ws) void WhenInfo::Instantiate(Frame* f) { if ( cl ) - curr_lambda = make_intrusive(lambda->Eval(f)); + Instantiate(lambda->Eval(f)); + } + +void WhenInfo::Instantiate(ValPtr func) + { + if ( cl ) + curr_lambda = make_intrusive(std::move(func)); } ExprPtr WhenInfo::Cond() @@ -1983,6 +1969,18 @@ StmtPtr WhenInfo::WhenBody() return make_intrusive(invoke, true); } +double WhenInfo::TimeoutVal(Frame* f) + { + if ( timeout ) + { + auto t = timeout->Eval(f); + if ( t ) + return t->AsDouble(); + } + + return -1.0; // signals "no timeout" + } + StmtPtr WhenInfo::TimeoutStmt() { if ( ! curr_lambda ) @@ -1992,6 +1990,65 @@ StmtPtr WhenInfo::TimeoutStmt() 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 is deprecated: %s", + vars.c_str()); + ws->Warn(msg.c_str()); + + return true; + } + +void WhenInfo::BuildInvokeElems() + { + one_const = make_intrusive(val_mgr->Count(1)); + two_const = make_intrusive(val_mgr->Count(2)); + three_const = make_intrusive(val_mgr->Count(3)); + + invoke_cond = make_intrusive(one_const); + invoke_s = make_intrusive(two_const); + invoke_timeout = make_intrusive(three_const); + } + WhenStmt::WhenStmt(WhenInfo* _wi) : Stmt(STMT_WHEN), wi(_wi) { wi->Build(ThisPtr()); @@ -2026,6 +2083,8 @@ ValPtr WhenStmt::Exec(Frame* f, StmtFlowType& flow) wi->Instantiate(f); + auto timeout = wi->TimeoutVal(f); + if ( wi->Captures() ) { std::vector local_aggrs; @@ -2037,12 +2096,12 @@ ValPtr WhenStmt::Exec(Frame* f, StmtFlowType& flow) local_aggrs.emplace_back(std::move(v)); } - new trigger::Trigger(wi, wi->WhenExprGlobals(), local_aggrs, f, location); + 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(), wi->TimeoutExpr(), f, + new trigger::Trigger(wi->Cond(), wi->WhenBody(), wi->TimeoutStmt(), timeout, f, wi->IsReturn(), location); return nullptr; diff --git a/src/Stmt.h b/src/Stmt.h index f61ffb28b1..462b339a20 100644 --- a/src/Stmt.h +++ b/src/Stmt.h @@ -547,6 +547,10 @@ public: // Takes ownership of the CaptureList, which if nil signifies // old-style frame semantics. WhenInfo(ExprPtr _cond, FuncType::CaptureList* _cl, bool _is_return); + + // Constructor used by script optimization to create a stub. + WhenInfo(bool _is_return); + ~WhenInfo() { delete cl; } void AddBody(StmtPtr _s) { s = std::move(_s); } @@ -559,11 +563,18 @@ public: // Complete construction of the associated internals, including // the (complex) lambda used to access the different elements of - // the statement. - void Build(StmtPtr ws); + // the statement. The optional argument is only for generating + // error messages. + void Build(StmtPtr ws = nullptr); - // Instantiate a new instance. + // This is available after a call to Build(). + const LambdaExprPtr& Lambda() const { return lambda; } + + // Instantiate a new instance, either by evaluating the associated + // lambda, or directly using the given function value (for compiled + // code). void Instantiate(Frame* f); + void Instantiate(ValPtr func); // For old-style semantics, the following simply return the // individual "when" components. For capture semantics, however, @@ -572,15 +583,15 @@ public: ExprPtr Cond(); StmtPtr WhenBody(); - ExprPtr TimeoutExpr() { return timeout; } + ExprPtr TimeoutExpr() const { return timeout; } + double TimeoutVal(Frame* f); + StmtPtr TimeoutStmt(); FuncType::CaptureList* Captures() { return cl; } bool IsReturn() const { return is_return; } - const LambdaExprPtr& Lambda() const { return lambda; } - // The locals and globals used in the conditional expression // (other than newly introduced locals), necessary for registering // the associated triggers for when their values change. @@ -588,6 +599,15 @@ 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 generates + // the corresponding deprecation warnings, which are 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; @@ -599,8 +619,9 @@ private: // The name of parameter passed ot the lambda. std::string lambda_param_id; - // The expression for constructing the lambda. + // The expression for constructing the lambda, and its type. LambdaExprPtr lambda; + FuncTypePtr lambda_ft; // The current instance of the lambda. Created by Instantiate(), // for immediate use via calls to Cond() etc. @@ -612,9 +633,17 @@ private: ListExprPtr invoke_s; ListExprPtr invoke_timeout; + // Helper expressions for calling the lambda / testing within it. + ConstExprPtr one_const; + ConstExprPtr two_const; + ConstExprPtr three_const; + IDSet when_expr_locals; IDSet when_expr_globals; + // 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; diff --git a/src/Trigger.cc b/src/Trigger.cc index 91aaff5003..36e2a7acd4 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, ExprPtr timeout_expr, - Frame* frame, bool is_return, const Location* location) - { - GetTimeout(timeout_expr, frame); - Init(cond, body, timeout_stmts, frame, is_return, location); - } - Trigger::Trigger(ExprPtr cond, StmtPtr body, StmtPtr timeout_stmts, double timeout, Frame* frame, bool is_return, const Location* location) { @@ -113,41 +106,19 @@ Trigger::Trigger(ExprPtr cond, StmtPtr body, StmtPtr timeout_stmts, double timeo Init(cond, body, timeout_stmts, frame, is_return, location); } -Trigger::Trigger(WhenInfo* wi, const IDSet& _globals, std::vector _local_aggrs, Frame* f, - const Location* loc) +Trigger::Trigger(WhenInfo* wi, double timeout, const IDSet& _globals, + std::vector _local_aggrs, Frame* f, const Location* loc) { + timeout_value = timeout; globals = _globals; local_aggrs = std::move(_local_aggrs); have_trigger_elems = true; - GetTimeout(wi->TimeoutExpr(), f); - Init(wi->Cond(), wi->WhenBody(), wi->TimeoutStmt(), f, wi->IsReturn(), loc); } -void Trigger::GetTimeout(const ExprPtr& timeout_expr, Frame* f) - { - timeout_value = -1.0; - - if ( timeout_expr ) - { - ValPtr timeout_val; - - try - { - timeout_val = timeout_expr->Eval(f); - } - catch ( InterpreterException& ) - { /* Already reported */ - } - - if ( timeout_val ) - timeout_value = timeout_val->AsInterval(); - } - } - void Trigger::Init(ExprPtr arg_cond, StmtPtr arg_body, StmtPtr arg_timeout_stmts, Frame* arg_frame, - bool arg_is_return, const Location* arg_location) + bool arg_is_return, const Location* location) { cond = arg_cond; body = arg_body; @@ -157,7 +128,11 @@ void Trigger::Init(ExprPtr arg_cond, StmtPtr arg_body, StmtPtr arg_timeout_stmts disabled = false; attached = nullptr; is_return = arg_is_return; - location = arg_location; + + 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(); @@ -526,12 +501,6 @@ void Trigger::Modified(notifier::detail::Modifiable* m) trigger_mgr->Queue(this); } -const char* Trigger::Name() const - { - assert(location); - return util::fmt("%s:%d-%d", location->filename, location->first_line, location->last_line); - } - Manager::Manager() : iosource::IOSource() { pending = new TriggerList(); diff --git a/src/Trigger.h b/src/Trigger.h index 5b8f84f770..66768a305a 100644 --- a/src/Trigger.h +++ b/src/Trigger.h @@ -47,16 +47,13 @@ public: // statements are executed immediately and the object is deleted // right away. - // These first two constructors are for the deprecated deep-copy - // semantics. - Trigger(ExprPtr cond, StmtPtr body, StmtPtr timeout_stmts, ExprPtr timeout, Frame* f, - bool is_return, const Location* loc); + // 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, const IDSet& globals, std::vector local_aggrs, Frame* f, - const Location* loc); + Trigger(WhenInfo* wi, double timeout, const IDSet& globals, std::vector local_aggrs, + Frame* f, const Location* loc); ~Trigger() override; @@ -108,13 +105,12 @@ public: // for any further progress to be made, so just Unref ourselves. void Terminate() override; - const char* Name() const; + const char* Name() const { return name.c_str(); } + void SetName(const char* new_name) { name = new_name; } private: friend class TriggerTimer; - void GetTimeout(const ExprPtr& timeout_expr, Frame* f); - void Init(ExprPtr cond, StmtPtr body, StmtPtr timeout_stmts, Frame* frame, bool is_return, const Location* location); @@ -131,7 +127,8 @@ private: double timeout_value; Frame* frame; bool is_return; - const Location* location; + + std::string name; TriggerTimer* timer; Trigger* attached;