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

Incorrect handling of init expression on set of sets #3933

Open
bbannier opened this issue Sep 16, 2024 · 0 comments · May be fixed by #4137
Open

Incorrect handling of init expression on set of sets #3933

bbannier opened this issue Sep 16, 2024 · 0 comments · May be fixed by #4137
Assignees
Labels
Area: Scripting Type: Bug 🐛 Unexpected behavior or output.

Comments

@bbannier
Copy link
Member

I would expect the following code to either work at runtime or be outright rejected:

# foo.zeek
event zeek_init()
	{
	local vs: set[set[string]] = { [ set() ] };
	assert |vs| == 1;
	}
$ zeek -b foo.zeek
error in foo.zeek, line 4: assertion failure: sizeof vs == 1
fatal error: errors occurred while initializing

I originally saw this with an initialization of the form

local vs: set[count, set[string, bool]] = { [ 1, set() ] };

In a Slack discussion there was an argument that this should not be supported at all and rejected since values in a set should be immutable (which a set is not). I'd argue that the usage here which does not modify the contained values at all could still be supported; we could still reject mutating operations on vs at runtime.

@bbannier bbannier added Type: Bug 🐛 Unexpected behavior or output. Area: Scripting labels Sep 16, 2024
@evantypanski evantypanski self-assigned this Dec 18, 2024
evantypanski added a commit that referenced this issue Dec 18, 2024
Closes #3933

There are a couple ways to get a list, sometimes with `{}`, sometimes
with `[]`. You can use these to append to sets and vectors. But,
sometimes it's useful to expand the elements of a list, like if you pass
in a global when appending a set. So, those elements are evaluated and
possibly change the list.

This can, however, end up with an empty list that seems like it should
be impactful for construction. It seems useful to say that an empty list
is intentional, since the initial check is not able to catch those
cases. So, keep that in the list if it's empty.
@evantypanski evantypanski linked a pull request Dec 18, 2024 that will close this issue
evantypanski added a commit that referenced this issue Jan 9, 2025
Closes #3933

There are a couple ways to get a list, sometimes with `{}`, sometimes
with `[]`. You can use these to append to sets and vectors. But,
sometimes it's useful to expand the elements of a list, like if you pass
in a global when appending a set. So, those elements are evaluated and
possibly change the list.

This can, however, end up with an empty list that seems like it should
be impactful for construction. It seems useful to say that an empty list
is intentional, since the initial check is not able to catch those
cases. So, keep that in the list if it's empty.
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.

2 participants