diff --git a/CHANGES b/CHANGES index 6d4eab988f..57be0dd237 100644 --- a/CHANGES +++ b/CHANGES @@ -1,4 +1,23 @@ +3.2.0-dev.332 | 2020-03-31 15:34:52 -0700 + + * GH-725: fix logic for finding a lambda's usage of outer IDs (Jon Siwek, Corelight) + + * Change record field anonymous functions to use lambda expressions + + There was an alternate syntax to assign anonymous functions to record + fields that was never migrated to use the new lambda expression + machinery (and so didn't allow referencing variables in outer scope): + + type myrec: record { + foo: function(a: string); + }; + + local o = "o"; + local mr = myrec($foo(a: string) = { print a + o; }); + + (Jon Siwek, Corelight) + 3.2.0-dev.329 | 2020-03-31 08:48:10 -0700 * Mark input/output message classes as final, since nothing should be inheriting from them (Tim Wojtulewicz, Corelight) diff --git a/VERSION b/VERSION index 9a0b5de251..ca0908a6d2 100644 --- a/VERSION +++ b/VERSION @@ -1 +1 @@ -3.2.0-dev.329 +3.2.0-dev.332 diff --git a/src/Expr.cc b/src/Expr.cc index e1a12db019..0c99f01012 100644 --- a/src/Expr.cc +++ b/src/Expr.cc @@ -4311,6 +4311,11 @@ LambdaExpr::LambdaExpr(std::unique_ptr arg_ing, id->SetConst(); } +Scope* LambdaExpr::GetScope() const + { + return ingredients->scope.get(); + } + IntrusivePtr LambdaExpr::Eval(Frame* f) const { auto lamb = make_intrusive( diff --git a/src/Expr.h b/src/Expr.h index ed2d953416..1ed01a57a9 100644 --- a/src/Expr.h +++ b/src/Expr.h @@ -64,6 +64,7 @@ extern const char* expr_name(BroExprTag t); template class IntrusivePtr; class Stmt; class Frame; +class Scope; class ListExpr; class NameExpr; class IndexExpr; @@ -815,6 +816,8 @@ public: IntrusivePtr Eval(Frame* f) const override; TraversalCode Traverse(TraversalCallback* cb) const override; + Scope* GetScope() const; + protected: void ExprDescribe(ODesc* d) const override; diff --git a/src/ID.cc b/src/ID.cc index bd7b536419..e0caea3b2d 100644 --- a/src/ID.cc +++ b/src/ID.cc @@ -12,7 +12,6 @@ #include "Scope.h" #include "Type.h" #include "File.h" -#include "Scope.h" #include "Traverse.h" #include "Val.h" #include "zeekygen/Manager.h" diff --git a/src/Var.cc b/src/Var.cc index cbbf6fbbf0..406e8ab54b 100644 --- a/src/Var.cc +++ b/src/Var.cc @@ -419,32 +419,25 @@ void begin_func(ID* id, const char* module_name, function_flavor flavor, class OuterIDBindingFinder : public TraversalCallback { public: OuterIDBindingFinder(Scope* s) - : scope(s) { } + { + scopes.emplace_back(s); + } TraversalCode PreExpr(const Expr*) override; TraversalCode PostExpr(const Expr*) override; - Scope* scope; + std::vector scopes; vector outer_id_references; - int lambda_depth = 0; - // Note: think we really ought to toggle this to false to prevent - // considering locals within inner-lambdas as "outer", but other logic - // for "selective cloning" and locating IDs in the closure chain may - // depend on current behavior and also needs to be changed. - bool search_inner_lambdas = true; }; TraversalCode OuterIDBindingFinder::PreExpr(const Expr* expr) { if ( expr->Tag() == EXPR_LAMBDA ) - ++lambda_depth; - - if ( lambda_depth > 0 && ! search_inner_lambdas ) - // Don't inspect the bodies of inner lambdas as they will have their - // own traversal to find outer IDs and we don't want to detect - // references to local IDs inside and accidentally treat them as - // "outer" since they can't be found in current scope. + { + auto le = static_cast(expr); + scopes.emplace_back(le->GetScope()); return TC_CONTINUE; + } if ( expr->Tag() != EXPR_NAME ) return TC_CONTINUE; @@ -454,8 +447,11 @@ TraversalCode OuterIDBindingFinder::PreExpr(const Expr* expr) if ( e->Id()->IsGlobal() ) return TC_CONTINUE; - if ( scope->Lookup(e->Id()->Name()) ) - return TC_CONTINUE; + for ( const auto& scope : scopes ) + if ( scope->Lookup(e->Id()->Name()) ) + // Shadowing is not allowed, so if it's found at inner scope, it's + // not something we have to worry about also being at outer scope. + return TC_CONTINUE; outer_id_references.push_back(e); return TC_CONTINUE; @@ -464,10 +460,7 @@ TraversalCode OuterIDBindingFinder::PreExpr(const Expr* expr) TraversalCode OuterIDBindingFinder::PostExpr(const Expr* expr) { if ( expr->Tag() == EXPR_LAMBDA ) - { - --lambda_depth; - assert(lambda_depth >= 0); - } + scopes.pop_back(); return TC_CONTINUE; } @@ -476,16 +469,6 @@ void end_func(IntrusivePtr body) { auto ingredients = std::make_unique(pop_scope(), std::move(body)); - if ( streq(ingredients->id->Name(), "anonymous-function") ) - { - OuterIDBindingFinder cb(ingredients->scope.get()); - ingredients->body->Traverse(&cb); - - for ( size_t i = 0; i < cb.outer_id_references.size(); ++i ) - cb.outer_id_references[i]->Error( - "referencing outer function IDs not supported"); - } - if ( ingredients->id->HasVal() ) ingredients->id->ID_Val()->AsFunc()->AddBody( ingredients->body, diff --git a/src/parse.y b/src/parse.y index 7f379d44ec..dd22fbee04 100644 --- a/src/parse.y +++ b/src/parse.y @@ -57,7 +57,7 @@ %type init_class %type opt_init %type TOK_CONSTANT -%type expr opt_expr init anonymous_function index_slice opt_deprecated +%type expr opt_expr init anonymous_function lambda_body index_slice opt_deprecated %type event %type stmt stmt_list func_body for_head %type type opt_type enum_body @@ -504,7 +504,7 @@ expr: $$ = new FieldAssignExpr($2, {AdoptRef{}, $4}); } - | '$' TOK_ID func_params '=' + | '$' TOK_ID func_params '=' { func_hdr_location = @1; func_id = current_scope()->GenerateTemporary("anonymous-function"); @@ -512,11 +512,9 @@ expr: begin_func(func_id, current_module.c_str(), FUNC_FLAVOR_FUNCTION, 0, {AdoptRef{}, $3}); } - func_body + lambda_body { - $$ = new FieldAssignExpr($2, - make_intrusive( - IntrusivePtr{NewRef{}, func_id->ID_Val()})); + $$ = new FieldAssignExpr($2, IntrusivePtr{AdoptRef{}, $6}); Unref(func_id); } @@ -1237,9 +1235,7 @@ func_body: } ; -anonymous_function: - TOK_FUNCTION begin_func - +lambda_body: '{' { saved_in_init.push_back(in_init); @@ -1254,20 +1250,25 @@ anonymous_function: '}' { - set_location(@1, @7); + set_location(@1, @5); // Code duplication here is sad but needed. end_func actually instantiates the function // and associates it with an ID. We perform that association later and need to return // a lambda expression. // Gather the ingredients for a BroFunc from the current scope - auto ingredients = std::make_unique(IntrusivePtr{NewRef{}, current_scope()}, IntrusivePtr{AdoptRef{}, $5}); + auto ingredients = std::make_unique(IntrusivePtr{NewRef{}, current_scope()}, IntrusivePtr{AdoptRef{}, $3}); id_list outer_ids = gather_outer_ids(pop_scope().get(), ingredients->body.get()); $$ = new LambdaExpr(std::move(ingredients), std::move(outer_ids)); } ; +anonymous_function: + TOK_FUNCTION begin_func lambda_body + { $$ = $3; } + ; + begin_func: func_params { diff --git a/testing/btest/Baseline/language.lambda-nested-copy/out b/testing/btest/Baseline/language.lambda-nested-copy/out new file mode 100644 index 0000000000..b963e085f3 --- /dev/null +++ b/testing/btest/Baseline/language.lambda-nested-copy/out @@ -0,0 +1,2 @@ +106 +106 diff --git a/testing/btest/Baseline/language.lambda-record-field/out b/testing/btest/Baseline/language.lambda-record-field/out new file mode 100644 index 0000000000..31e0fce560 --- /dev/null +++ b/testing/btest/Baseline/language.lambda-record-field/out @@ -0,0 +1 @@ +helloworld diff --git a/testing/btest/Baseline/scripts.base.frameworks.input.reread/out b/testing/btest/Baseline/scripts.base.frameworks.input.reread/out index 19d323afcb..13adcdd31d 100644 --- a/testing/btest/Baseline/scripts.base.frameworks.input.reread/out +++ b/testing/btest/Baseline/scripts.base.frameworks.input.reread/out @@ -39,7 +39,7 @@ print A::outfile, Left; print A::outfile, A::left; print A::outfile, Right; print A::outfile, A::right; -}, pred=anonymous-function +}, pred=lambda_<10906653936191056190> { print A::outfile, ============PREDICATE============; print A::outfile, A::typ; @@ -134,7 +134,7 @@ print A::outfile, Left; print A::outfile, A::left; print A::outfile, Right; print A::outfile, A::right; -}, pred=anonymous-function +}, pred=lambda_<10906653936191056190> { print A::outfile, ============PREDICATE============; print A::outfile, A::typ; @@ -241,7 +241,7 @@ print A::outfile, Left; print A::outfile, A::left; print A::outfile, Right; print A::outfile, A::right; -}, pred=anonymous-function +}, pred=lambda_<10906653936191056190> { print A::outfile, ============PREDICATE============; print A::outfile, A::typ; @@ -468,7 +468,7 @@ print A::outfile, Left; print A::outfile, A::left; print A::outfile, Right; print A::outfile, A::right; -}, pred=anonymous-function +}, pred=lambda_<10906653936191056190> { print A::outfile, ============PREDICATE============; print A::outfile, A::typ; @@ -593,7 +593,7 @@ print A::outfile, Left; print A::outfile, A::left; print A::outfile, Right; print A::outfile, A::right; -}, pred=anonymous-function +}, pred=lambda_<10906653936191056190> { print A::outfile, ============PREDICATE============; print A::outfile, A::typ; @@ -718,7 +718,7 @@ print A::outfile, Left; print A::outfile, A::left; print A::outfile, Right; print A::outfile, A::right; -}, pred=anonymous-function +}, pred=lambda_<10906653936191056190> { print A::outfile, ============PREDICATE============; print A::outfile, A::typ; @@ -843,7 +843,7 @@ print A::outfile, Left; print A::outfile, A::left; print A::outfile, Right; print A::outfile, A::right; -}, pred=anonymous-function +}, pred=lambda_<10906653936191056190> { print A::outfile, ============PREDICATE============; print A::outfile, A::typ; @@ -968,7 +968,7 @@ print A::outfile, Left; print A::outfile, A::left; print A::outfile, Right; print A::outfile, A::right; -}, pred=anonymous-function +}, pred=lambda_<10906653936191056190> { print A::outfile, ============PREDICATE============; print A::outfile, A::typ; @@ -1198,7 +1198,7 @@ print A::outfile, Left; print A::outfile, A::left; print A::outfile, Right; print A::outfile, A::right; -}, pred=anonymous-function +}, pred=lambda_<10906653936191056190> { print A::outfile, ============PREDICATE============; print A::outfile, A::typ; @@ -1251,7 +1251,7 @@ print A::outfile, Left; print A::outfile, A::left; print A::outfile, Right; print A::outfile, A::right; -}, pred=anonymous-function +}, pred=lambda_<10906653936191056190> { print A::outfile, ============PREDICATE============; print A::outfile, A::typ; @@ -1304,7 +1304,7 @@ print A::outfile, Left; print A::outfile, A::left; print A::outfile, Right; print A::outfile, A::right; -}, pred=anonymous-function +}, pred=lambda_<10906653936191056190> { print A::outfile, ============PREDICATE============; print A::outfile, A::typ; @@ -1357,7 +1357,7 @@ print A::outfile, Left; print A::outfile, A::left; print A::outfile, Right; print A::outfile, A::right; -}, pred=anonymous-function +}, pred=lambda_<10906653936191056190> { print A::outfile, ============PREDICATE============; print A::outfile, A::typ; @@ -1410,7 +1410,7 @@ print A::outfile, Left; print A::outfile, A::left; print A::outfile, Right; print A::outfile, A::right; -}, pred=anonymous-function +}, pred=lambda_<10906653936191056190> { print A::outfile, ============PREDICATE============; print A::outfile, A::typ; @@ -1463,7 +1463,7 @@ print A::outfile, Left; print A::outfile, A::left; print A::outfile, Right; print A::outfile, A::right; -}, pred=anonymous-function +}, pred=lambda_<10906653936191056190> { print A::outfile, ============PREDICATE============; print A::outfile, A::typ; diff --git a/testing/btest/language/lambda-nested-copy.zeek b/testing/btest/language/lambda-nested-copy.zeek new file mode 100644 index 0000000000..6cf0e6952a --- /dev/null +++ b/testing/btest/language/lambda-nested-copy.zeek @@ -0,0 +1,19 @@ +# @TEST-EXEC: zeek -b %INPUT >out +# @TEST-EXEC: btest-diff out + +local outer = 100; + +local lambda = function() + { + local inner = function(a: count, b: count, c: count, d: count, e: count, f: count) + { + print outer + f; + }; + + inner(1, 2, 3, 4, 5, 6); + }; + +lambda(); + +local copyLambda = copy(copy(copy(lambda))); +copyLambda(); diff --git a/testing/btest/language/lambda-record-field.zeek b/testing/btest/language/lambda-record-field.zeek new file mode 100644 index 0000000000..bcb7e1bc75 --- /dev/null +++ b/testing/btest/language/lambda-record-field.zeek @@ -0,0 +1,13 @@ +# @TEST-EXEC: zeek -b %INPUT >out +# @TEST-EXEC: btest-diff out + +type myrec: record { + foo: function(a: string); +}; + +event zeek_init() + { + local w = "world"; + local mr = myrec($foo(a: string) = { print a + w; }); + mr$foo("hello"); + }