-
Notifications
You must be signed in to change notification settings - Fork 691
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
reference-types: add final test: br_table #1264
Conversation
This requires some slighly different validation rules for MVP vs interface-types. There is a test in unreachable-invalid.wast that was removed in the reference-types repo: type-br_table-label-num-vs-label-num-after-unreachable This test depends on the old validation rules and still exists in the master testsuite so we need to continue to support both sets of rules for now.
Can you link to the spec of the updated validation rules? |
The change that removed the test in question was: The new validation algorithm is at: I think this change relates to how the new bottom type and how the unreachable value stack works, but I don't fully understand the change. |
Ah the arity check is specified here: |
result |= Result::Error; | ||
PrintError("br_table labels have inconsistent arity: expected %" PRIzd | ||
" got %" PRIzd, | ||
br_table_sig_->size(), label_sig.size()); |
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'm confused about where we check that the types are compatible beyond their arity. In the algorithm.rst doc, this is done by a call to pop_vals
, but where is it done here?
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.
That already happens above in the call to CheckSignature
.. that basically ensure that the value stack contains the values that match the label. That part doesn't change.
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!
14f578a
to
86cfb31
Compare
This requires some slighly different validation rules for MVP vs
interface-types.
There is a test in unreachable-invalid.wast that was removed in the
reference-types repo:
type-br_table-label-num-vs-label-num-after-unreachable
This test depends on the old validation rules and still exists in the
master testsuite so we need to continue to support both sets of rules
for now.