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

Add support for field-local %error handlers. #1838

Merged
merged 4 commits into from
Sep 26, 2024

Conversation

rsmmr
Copy link
Member

@rsmmr rsmmr commented Aug 22, 2024

Add support for field-local %error handlers.

We now support attaching an %error handler to an individual field:

    type Test = unit {
        a: b"A";
        b: b"B" %error { print "field B %error", self; }
        c: b"C";
    };

With input AxC, that handler will trigger, whereas with ABx it
won't. If the unit had a unit-wide %error handler as well, that one
would trigger in both cases (i.e., for b, in addition to its field
local handler).

The handler can also be provided separately from the field:

    on b %error { ... }

In that separate version, one can receive the error message as well by
declaring a corresponding string parameter:

    on b(msg: string) %error { ... }

This works externally, from outside the unit, as well:

    on Test::b(msg: string) %error { ... }

This is debased on top of topic/robin/optimize-type-parsing so that
we get the peephole optimizer.

Addresses #1824.

@rsmmr
Copy link
Member Author

rsmmr commented Aug 23, 2024

@sethhall another one to try

@sethhall
Copy link
Member

sethhall commented Aug 26, 2024

This doesn't seem to work if you have any attributes attached to the field...

type Test1 = unit {
    a: b"A";
    b: bytes &size=1 %error { print "field B %error", self; }
    c: b"C";
};
[error] test17.spicy:5:23-5:27: unknown ID 'error'
[error] spicyc: aborting after errors

@rsmmr
Copy link
Member Author

rsmmr commented Aug 27, 2024

b: bytes &size=1 %error { print "field B %error", self; }

Turns out this gets parsed as &size=(1 % error) (i.e., a module expression) ... !

@rsmmr
Copy link
Member Author

rsmmr commented Aug 27, 2024

Turns out this gets parsed as &size=(1 % error) (i.e., a module expression) ... !

... and this is because in the past we have explicitly added support for parsing things like l %k == 10; as a valid expression.

@sethhall
Copy link
Member

sethhall commented Aug 27, 2024

Turns out this gets parsed as &size=(1 % error) (i.e., a module expression) ... !

haha, well I'm glad I bumped across that when I tried to use the feature!

@rsmmr rsmmr force-pushed the topic/robin/gh-1824-field-error-hook branch 2 times, most recently from f0910a6 to 63199fe Compare September 24, 2024 08:45
@rsmmr rsmmr changed the title [WIP] Add support for field-local %error handlers. Add support for field-local %error handlers. Sep 24, 2024
@rsmmr rsmmr force-pushed the topic/robin/gh-1824-field-error-hook branch from 63199fe to 71d0e8b Compare September 24, 2024 11:48
@rsmmr rsmmr marked this pull request as ready for review September 24, 2024 13:55
@rsmmr rsmmr requested a review from bbannier September 24, 2024 13:55
@rsmmr rsmmr force-pushed the topic/robin/gh-1824-field-error-hook branch 2 times, most recently from 7de841d to 1d91bf5 Compare September 26, 2024 08:38
bbannier
bbannier previously approved these changes Sep 26, 2024
Copy link
Member

@bbannier bbannier left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks! 🚀

@rsmmr rsmmr force-pushed the topic/robin/gh-1824-field-error-hook branch from 1d91bf5 to d7dc372 Compare September 26, 2024 10:27
Currently we only have two types of field hooks: standard hooks and
`foreach` hooks. To prepare for more types, this refactors the code
to represent the type with an `enum` instead of a `foreach` boolean. It
also moves validation of permitted attributes from the parser to
the validator.
@rsmmr rsmmr force-pushed the topic/robin/gh-1824-field-error-hook branch from d7dc372 to fec6193 Compare September 26, 2024 12:24
We now support attaching an `%error` handler to an individual field:

    type Test = unit {
        a: b"A";
        b: b"B" %error { print "field B %error", self; }
        c: b"C";
    };

With input `AxC`, that handler will trigger, whereas with `ABx` it
won't. If the unit had a unit-wide `%error` handler as well, that one
would trigger in both cases (i.e., for `b`, in addition to its field
local handler).

The handler can also be provided separately from the field:

    on b %error { ... }

In that separate version, one can receive the error message as well by
declaring a corresponding string parameter:

    on b(msg: string) %error { ... }

This works externally, from outside the unit, as well:

    on Test::b(msg: string) %error { ... }

This is eebased on top of `topic/robin/optimize-type-parsing` so that
we get the peephole optimizer.

Addresses #1824.
The old text was very outdated. This extends the content, documents
the new per-field `%error` handler, and moves it all into the
"parsing" section to have it closer to the error recovery content.
In the past, we had special-cased properties in our Flex/Bison parser
so that when parsing an expression, they wouldn't be recognized.
However, that now led an error field hook of the form `x: bytes
&size=42 %error` to be parsed as `&size=(42 % error)`. We now switch
to white-listing all known properties, just as we already do for
attributes. That way conflicts should be extremely rare.
@rsmmr rsmmr force-pushed the topic/robin/gh-1824-field-error-hook branch from fec6193 to 7fba8bf Compare September 26, 2024 12:41
@rsmmr rsmmr merged commit 15db391 into main Sep 26, 2024
20 checks passed
@rsmmr rsmmr deleted the topic/robin/gh-1824-field-error-hook branch September 26, 2024 15:30
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

Successfully merging this pull request may close these issues.

3 participants