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

Bytes vector compiles without termination condition for bytes field. #1832

Closed
sethhall opened this issue Aug 8, 2024 · 3 comments
Closed
Assignees
Labels
Bug Something isn't working Diagnostics Good first issue Good for newcomers

Comments

@sethhall
Copy link
Member

sethhall commented Aug 8, 2024

This toy parser is applying size to the total size of the vector but the bytes in the vector actually have no termination condition.

module test;

public type X = unit {
    a: bytes[10] &size=5;
};
@rsmmr rsmmr added Bug Something isn't working Good first issue Good for newcomers Diagnostics labels Aug 9, 2024
@rsmmr
Copy link
Member

rsmmr commented Aug 9, 2024

Looks like there's validation missing rejecting the inner field: it should use the same logic that we use for bytes fields outside of vectors to enforce that there's some termination condition (e.g, &eod, &size, etc.).

@evantypanski evantypanski self-assigned this Sep 3, 2024
@evantypanski
Copy link
Contributor

evantypanski commented Sep 3, 2024

Well it seems like the validator is trying, but it passes the vector's field through during validation:

else if ( const auto& x = pt->type()->tryAs<hilti::type::Vector>() ) {
if ( auto rc = isParseableType(x->elementType(), f); ! rc )
return rc;
return hilti::Nothing();
}

If you surround bytes in parentheses, this actually gets you the error properly:

module test;

public type X = unit {
    a: (bytes)[10] &size=5;
};

The only change is parentheses around bytes. That gets me:

[error] test.spicy:4:9-4:13: bytes field requires one of &eod, &parse_at, &parse_from, &size, &until, &until-including
[error] spicyc: aborting after errors

I'm not quite sure how to get the attributes down to the inner bytes type since the attributes seem applied to fields not types, but I'll look into that :)

@bbannier
Copy link
Member

bbannier commented Sep 6, 2024

Fixed with f9bbf82.

@bbannier bbannier closed this as completed Sep 6, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Bug Something isn't working Diagnostics Good first issue Good for newcomers
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants