-
Notifications
You must be signed in to change notification settings - Fork 505
[spec] Fix and clean up invariants for host functions #563
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
Conversation
binji
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I just did a quick once-over, but it looks good to me.
|
Hi Andreas We briefly discussed the possibility of removing the store constraints from the host function reduction rule, since they're not necessary any more. This is a stylistic choice, but it would make my model slightly prettier if I'm going for "eyeball-close" correspondence with the spec rules. Looks good to me in any case! |
|
@conrad-watt, right. Done, adjusting the text accordingly. PTAL. |
conrad-watt
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM!
document/core/exec/instructions.rst
Outdated
| every :ref:`host function instance <syntax-funcinst>` must be :ref:`valid <valid-hostfuncinst>`, | ||
| which means that it adheres to suitable pre- and post-conditions: | ||
| under a :ref:`valid store <valid-store>` :math:`S`, and given arguments :math:`\val^n` matching the ascribed parameter types :math:`t_1^n`, | ||
| executing the host function must produce a valid store :math:`S'` that is an :ref:`extension <extend-store>` of :math:`S` and a result matching the ascribed result types :math:`t_2^m`. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Possible nitpick: "result types" should be "result type"
Test out-of-bounds element segment indexing for `array.new_elem`
Plus a few random typos.
This change is inspired by discussion with Conrad Watt, who pointed out a problem with the previous formulation.