Merge remote-tracking branch 'origin/topic/jsiwek/misc-lambda-fixes'

* origin/topic/jsiwek/misc-lambda-fixes:
  GH-725: fix logic for finding a lambda's usage of outer IDs
  Change record field anonymous functions to use lambda expressions

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)

VERSION

@@ -1 +1 @@
-3.2.0-dev.329
+3.2.0-dev.332

src/Expr.cc

@@ -4311,6 +4311,11 @@ LambdaExpr::LambdaExpr(std::unique_ptr<function_ingredients> arg_ing,
 	id->SetConst();
 	}
 
+Scope* LambdaExpr::GetScope() const
+	{
+	return ingredients->scope.get();
+	}
+
 IntrusivePtr<Val> LambdaExpr::Eval(Frame* f) const
 	{
 	auto lamb = make_intrusive<BroFunc>(

src/Expr.h

@@ -64,6 +64,7 @@ extern const char* expr_name(BroExprTag t);
 template <class T> class IntrusivePtr;
 class Stmt;
 class Frame;
+class Scope;
 class ListExpr;
 class NameExpr;
 class IndexExpr;
@@ -815,6 +816,8 @@ public:
 	IntrusivePtr<Val> Eval(Frame* f) const override;
 	TraversalCode Traverse(TraversalCallback* cb) const override;
 
+	Scope* GetScope() const;
+
 protected:
 	void ExprDescribe(ODesc* d) const override;
 

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"

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<Scope*> scopes;
 	vector<const NameExpr*> 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<const LambdaExpr*>(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<Stmt> body)
 	{
 	auto ingredients = std::make_unique<function_ingredients>(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,

src/parse.y

@@ -57,7 +57,7 @@
 %type <ic> init_class
 %type <expr> opt_init
 %type <val> TOK_CONSTANT
-%type <expr> expr opt_expr init anonymous_function index_slice opt_deprecated
+%type <expr> expr opt_expr init anonymous_function lambda_body index_slice opt_deprecated
 %type <event_expr> event
 %type <stmt> stmt stmt_list func_body for_head
 %type <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<ConstExpr>(
-			            IntrusivePtr<Val>{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<function_ingredients>(IntrusivePtr{NewRef{}, current_scope()}, IntrusivePtr{AdoptRef{}, $5});
+			auto ingredients = std::make_unique<function_ingredients>(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
 			{

testing/btest/Baseline/language.lambda-nested-copy/out

@@ -0,0 +1,2 @@
+106
+106

testing/btest/Baseline/language.lambda-record-field/out

@@ -0,0 +1 @@
+helloworld

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;

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();

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");
+	}