From ee60ab5d5dd863ac76d27a086b0292b16477f98f Mon Sep 17 00:00:00 2001 From: Vern Paxson Date: Wed, 31 Jul 2024 21:07:39 +0100 Subject: [PATCH] fixups for correctly tracking parameters --- src/ID.cc | 11 +++-------- src/ID.h | 17 ++++++++++++++--- src/Stmt.cc | 1 + src/Var.cc | 8 +++++--- src/script_opt/CPP/DeclFunc.cc | 12 +++++------- src/script_opt/Inline.cc | 1 + src/script_opt/ProfileFunc.cc | 20 +++++++++++++++++--- src/script_opt/ProfileFunc.h | 2 ++ src/script_opt/Reduce.cc | 3 +++ 9 files changed, 51 insertions(+), 24 deletions(-) diff --git a/src/ID.cc b/src/ID.cc index 72effdb4a3..8a0716cb19 100644 --- a/src/ID.cc +++ b/src/ID.cc @@ -100,20 +100,15 @@ ID::ID(const char* arg_name, IDScope arg_scope, bool arg_is_export) { name = util::copy_string(arg_name); scope = arg_scope; is_export = arg_is_export; - is_option = false; - is_blank = name && extract_var_name(name) == "_"; - is_const = false; - is_enum_const = false; - is_type = false; offset = 0; - if ( is_blank ) + if ( name && extract_var_name(name) == "_" ) { + is_blank = true; SetType(base_type(TYPE_ANY)); + } opt_info = new IDOptInfo(this); - infer_return_type = false; - SetLocationInfo(&start_location, &end_location); } diff --git a/src/ID.h b/src/ID.h index 75b849b5c1..965d4b4333 100644 --- a/src/ID.h +++ b/src/ID.h @@ -21,12 +21,14 @@ class RecordType; class TableType; class VectorType; class EnumType; +class FuncType; class Type; using TypePtr = IntrusivePtr; using RecordTypePtr = IntrusivePtr; using TableTypePtr = IntrusivePtr; using VectorTypePtr = IntrusivePtr; using EnumTypePtr = IntrusivePtr; +using FuncTypePtr = IntrusivePtr; using ValPtr = IntrusivePtr; using FuncPtr = IntrusivePtr; @@ -81,7 +83,6 @@ public: } bool IsType() const { return is_type; } - void MakeType() { is_type = true; } void SetVal(ValPtr v); @@ -95,6 +96,10 @@ public: void ClearVal(); + // ### + FuncTypePtr IsParam() const { return param_func_type; } + void SetParam(const FuncTypePtr& ft) { param_func_type = ft; } + void SetConst() { is_const = true; } bool IsConst() const { return is_const; } @@ -160,9 +165,15 @@ protected: const char* name; IDScope scope; bool is_export; - bool infer_return_type; TypePtr type; - bool is_const, is_enum_const, is_type, is_option, is_blank; + FuncTypePtr param_func_type; + bool is_capture = false; + bool is_const = false; + bool is_enum_const = false; + bool is_type = false; + bool is_option = false; + bool is_blank = false; + bool infer_return_type = false; int offset; ValPtr val; AttributesPtr attrs; diff --git a/src/Stmt.cc b/src/Stmt.cc index 383884137c..469d2be356 100644 --- a/src/Stmt.cc +++ b/src/Stmt.cc @@ -1786,6 +1786,7 @@ WhenInfo::WhenInfo(ExprPtr arg_cond, FuncType::CaptureList* arg_cl, bool arg_is_ param_id = install_ID(lambda_param_id.c_str(), current_module.c_str(), false, false); param_id->SetType(count_t); + param_id->SetParam(lambda_ft); } WhenInfo::WhenInfo(const WhenInfo* orig) { diff --git a/src/Var.cc b/src/Var.cc index e17e2dc15a..8943bfadad 100644 --- a/src/Var.cc +++ b/src/Var.cc @@ -538,8 +538,10 @@ static auto get_prototype(IDPtr id, FuncTypePtr t) { return prototype; } -static bool check_params(int i, std::optional prototype, const RecordTypePtr& args, +static bool check_params(int i, std::optional prototype, const FuncTypePtr& ft, const RecordTypePtr& canon_args, const char* module_name) { + const auto& args = ft->Params(); + TypeDecl* arg_i; bool hide = false; @@ -577,6 +579,7 @@ static bool check_params(int i, std::optional prototype, co arg_id = install_ID(local_name, module_name, false, false); arg_id->SetType(arg_i->type); + arg_id->SetParam(ft); return true; } @@ -635,13 +638,12 @@ void begin_func(IDPtr id, const char* module_name, FunctionFlavor flavor, bool i if ( IsErrorType(id->GetType()->Tag()) ) reporter->FatalError("invalid definition of '%s' (see previous errors)", id->Name()); - const auto& args = t->Params(); const auto& canon_args = id->GetType()->AsFuncType()->Params(); push_scope(std::move(id), std::move(attrs)); for ( int i = 0; i < canon_args->NumFields(); ++i ) - if ( ! check_params(i, prototype, args, canon_args, module_name) ) + if ( ! check_params(i, prototype, t, canon_args, module_name) ) break; if ( Attr* depr_attr = find_attr(current_scope()->Attrs().get(), ATTR_DEPRECATED) ) diff --git a/src/script_opt/CPP/DeclFunc.cc b/src/script_opt/CPP/DeclFunc.cc index 9ff0eb4134..a85c5f8a2a 100644 --- a/src/script_opt/CPP/DeclFunc.cc +++ b/src/script_opt/CPP/DeclFunc.cc @@ -328,18 +328,16 @@ void CPPCompile::GatherParamNames(vector& p_names, const FuncTypePtr& ft if ( param_id ) { if ( t->Tag() == TYPE_ANY && param_id->GetType()->Tag() != TYPE_ANY ) - // We'll need to translate the parameter - // from its current representation to - // type "any". + // We'll need to translate the parameter from its current + // representation to type "any". p_names.emplace_back(string("any_param__CPP_") + Fmt(i)); else p_names.emplace_back(LocalName(param_id)); } else - // Parameters that are unused don't wind up in the - // ProfileFunc. Rather than dig their name out of - // the function's declaration, we explicitly name - // them to reflect that they're unused. + // Parameters that are unused don't wind up in the ProfileFunc. + // Rather than dig their name out of the function's declaration, + // we explicitly name them to reflect that they're unused. p_names.emplace_back(string("unused_param__CPP_") + Fmt(i)); } diff --git a/src/script_opt/Inline.cc b/src/script_opt/Inline.cc index fb5c3fb61d..90a4f01552 100644 --- a/src/script_opt/Inline.cc +++ b/src/script_opt/Inline.cc @@ -254,6 +254,7 @@ void Inliner::CoalesceEventHandlers(ScriptFuncPtr func, const std::vectorName(), "", false, false); p->SetType(vi->GetType()); + p->SetParam(cast_intrusive(func->GetType())); param_ids.push_back(std::move(p)); } diff --git a/src/script_opt/ProfileFunc.cc b/src/script_opt/ProfileFunc.cc index 1b5feab5b0..44e5747ec3 100644 --- a/src/script_opt/ProfileFunc.cc +++ b/src/script_opt/ProfileFunc.cc @@ -27,8 +27,8 @@ ProfileFunc::ProfileFunc(const Func* func, const StmtPtr& body, bool _abs_rec_fi profiled_body = body.get(); abs_rec_fields = _abs_rec_fields; - auto ft = func->GetType()->AsFuncType(); - auto& fcaps = ft->GetCaptures(); + profiled_func_t = cast_intrusive(func->GetType()); + auto& fcaps = profiled_func_t->GetCaptures(); if ( fcaps ) { int offset = 0; @@ -40,7 +40,7 @@ ProfileFunc::ProfileFunc(const Func* func, const StmtPtr& body, bool _abs_rec_fi } } - Profile(ft, body); + Profile(profiled_func_t.get(), body); } ProfileFunc::ProfileFunc(const Stmt* s, bool _abs_rec_fields) { @@ -196,6 +196,7 @@ TraversalCode ProfileFunc::PreExpr(const Expr* e) { break; } +#if 0 // This is a tad ugly. Unfortunately due to the weird way // that Zeek function *declarations* work, there's no reliable // way to get the list of parameters for a function *definition*, @@ -205,6 +206,12 @@ TraversalCode ProfileFunc::PreExpr(const Expr* e) { // to avoid misconfusing a lambda capture with a low frame offset // as a parameter. if ( captures.count(id) == 0 && id->Offset() < num_params ) + { + ASSERT(id->IsParam()); + params.insert(id); + } +#endif + if ( profiled_func_t && id->IsParam() == profiled_func_t ) params.insert(id); locals.insert(id); @@ -422,9 +429,16 @@ TraversalCode ProfileFunc::PreExpr(const Expr* e) { locals.insert(i); TrackID(i); +#if 0 // See above re EXPR_NAME regarding the following // logic. if ( captures.count(i) == 0 && i->Offset() < num_params ) + { + ASSERT(i->IsParam()); + params.insert(i); + } +#endif + if ( profiled_func_t && i->IsParam() == profiled_func_t ) params.insert(i); } diff --git a/src/script_opt/ProfileFunc.h b/src/script_opt/ProfileFunc.h index 660e24b9e1..fb5aecf048 100644 --- a/src/script_opt/ProfileFunc.h +++ b/src/script_opt/ProfileFunc.h @@ -158,6 +158,8 @@ protected: // The function, body, or expression profiled. Can be null // depending on which constructor was used. const Func* profiled_func = nullptr; + // ### null is okay, means "not in a full function context" + FuncTypePtr profiled_func_t; const Stmt* profiled_body = nullptr; const Expr* profiled_expr = nullptr; diff --git a/src/script_opt/Reduce.cc b/src/script_opt/Reduce.cc index fbf5bc7eb1..172a309623 100644 --- a/src/script_opt/Reduce.cc +++ b/src/script_opt/Reduce.cc @@ -817,6 +817,9 @@ IDPtr Reducer::GenLocal(const IDPtr& orig) { local_id->SetType(orig->GetType()); local_id->SetAttrs(orig->GetAttrs()); + // Don't propagate IsParam() since if we're creating a local for the + // identifier then that's for a context where it's not a parameter. + if ( orig->IsBlank() ) local_id->SetBlank();