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

Error when mixing when with preprocessor macros #30

Open
GTrunSec opened this issue Jul 19, 2022 · 5 comments
Open

Error when mixing when with preprocessor macros #30

GTrunSec opened this issue Jul 19, 2022 · 5 comments

Comments

@GTrunSec
Copy link

instance: https://github.com/dopheide-esnet/zeek-known-hosts-with-dns/blob/master/scripts/known-hosts-with-dns.zeek#L171

}timeout Known::host_store_timeout { }
@GTrunSec
Copy link
Author

GTrunSec commented Jul 19, 2022

second one: line 178: syntax error, at or near "}"

The @if boundaries are not properly parsed.

@bbannier
Copy link
Member

This seems to be zeek/tree-sitter-zeek#6 again. Small reproducer:

@if ( Version::at_least("5.0") )
when ( ( T ) )
@else
when ( ( F ) )
@endif
	{ }
timeout 5 sec
	{ }

The preprocessor macros mixed with a declaration here cannot be represented in the syntax tree of the used grammar. A workaround is to e.g., instead use preprocessor macros around in this case full declarations,

@if ( Version::at_least("5.0") )
when ( ( T ) )
	{ }
timeout 5 sec
	{ }
@else
when ( ( F ) )
	{ }
timeout 5 sec
	{ }
@endif

Note that the error message is reported on stderr while e.g., zeek-format emits the formatted input on stdout (which in this case would be unchanged since the input cannot be parsed). It would still return with an error code though.

I am wondering whether it wouldn't make sense to make syntax checking an opt-in feature of zeekscript (either extra flag, or just debug information) so that in this case it would return the unformatted input (unparseable input) and a success error code.

@bbannier bbannier changed the title formatting error with timeout Error when mixing when with preprocessor macros Jul 19, 2022
@vpax
Copy link

vpax commented Jul 19, 2022

From a quick skim, looks like yet another instance of the problem I mention in https://zeekorg.slack.com/archives/CSZBXF6TH/p1658207113074829?thread_ts=1658131529.935799&cid=CSZBXF6TH. I'll file an issue when I get a chance (likely not till later this week.)

@bbannier
Copy link
Member

@vpax, instead of filing a new one I think we could just use this issue or potentially a reopened zeek/tree-sitter-zeek#6.

For that issue I did some digging how other tree-sitter grammars handle preprocessor macros (see e.g., here), and it seems even the upstream C grammar doesn't handle preprocessor macros in arbitrary places in the syntax. This is due to the fact that most of the time one probably would want to represent preprocessor macros in the parse result (e.g., to not have to deal with resolving them), but would need to explicitly allow for this everywhere in the syntax (which is very awkward).

@vpax
Copy link

vpax commented Jul 19, 2022

What I'm referring to isn't a tree-sitter issue, it's a basic Zeek issue: zeek/zeek#2289.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants