Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

@if block parsing problem when located within lambda return value #2075

Closed
0xxon opened this issue Apr 28, 2022 · 7 comments · Fixed by #2346
Closed

@if block parsing problem when located within lambda return value #2075

0xxon opened this issue Apr 28, 2022 · 7 comments · Fixed by #2346
Assignees
Labels
Area: Scripting Type: Bug 🐛 Unexpected behavior or output.

Comments

@0xxon
Copy link
Member

0xxon commented Apr 28, 2022

I tried making some script-code backwards compatible and notices that this code...

function test()
	{
	local make_epoch_result = function(pass_name: string): function(ts: time)
		{
@if ( Version::at_least("4.1") )
		return function [pass_name] (ts: time)
@else
		return function (ts: time)
@endif
			{
			print pass_name;
			};
		};

	}

Throws the following error when run with a current master version of Zeek:

error in ./test2.zeek, line 7: syntax error, at or near "@else"

I think that this is supposed to work fine - but someone please correct me if I am incorrect. One can work around this problem by changing the placement of the { after the return statement.

@0xxon 0xxon added Type: Bug 🐛 Unexpected behavior or output. Area: Scripting labels Apr 28, 2022
@awelzel
Copy link
Contributor

awelzel commented Aug 16, 2022

This appears to be fixable via the following hunk, but not sure #2289 is that easy, too.

 anonymous_function:
-               TOK_FUNCTION begin_lambda lambda_body
-                       { $$ = $3; }
+               TOK_FUNCTION begin_lambda conditional_list lambda_body
+                       { $$ = $4; }
        ;

@vpax
Copy link
Contributor

vpax commented Aug 16, 2022

@awelzel will that change really work? The { for the body that precedes the @if in the example is part of the lambda_body, so that conditional will be prior to any elements of the body.

@awelzel
Copy link
Contributor

awelzel commented Aug 16, 2022

@awelzel will that change really work? The { for the body that precedes the @if in the example is part of the lambda_body, so that conditional will be prior to any elements of the body.

I will take a closer look tomorrow. I added a btest with above example and it seemed to work. Will double check and get back to you. Thanks for looking.

@awelzel
Copy link
Contributor

awelzel commented Aug 17, 2022

@vpax - the example of this ticket involves a lamda returning a lambda. It's the @else following function [pass_name] (ts: time) (begin_lambda) what upsets the parsing. The first @if within the lambda_body seems to work out as it's part of the first lambda_body's stmt_list.

The proposed fix is similar to what we do for func_hdr / func_body:

zeek/src/parse.y

Lines 1401 to 1406 in fad18cb

| func_hdr
{
func_hdr_location = @1;
func_hdr_cond_epoch = conditional_epoch;
}
conditional_list func_body

I've opened a PR, please chime in if I misunderstood or you have further concerns/thoughts. Thanks!

@vpax
Copy link
Contributor

vpax commented Aug 17, 2022

@awelzel I see. Given that change, does the snippet at the top now work correctly?

@awelzel
Copy link
Contributor

awelzel commented Aug 17, 2022

@awelzel I see. Given that change, does the snippet at the top now work correctly?

Yep. I've added it here as a test-case:

https://github.com/zeek/zeek/pull/2346/files#diff-c7537e2ab0b02836a74f51fd55edd9a3fee02a971df499131030bd24b12f7dc1R8

@vpax
Copy link
Contributor

vpax commented Aug 17, 2022

Yep. I've added it here as a test-case:

Great!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Area: Scripting Type: Bug 🐛 Unexpected behavior or output.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants