Skip to content
This repository was archived by the owner on Nov 3, 2021. It is now read-only.

Fix various TODOs #72

Merged
merged 4 commits into from
Jan 16, 2020
Merged

Fix various TODOs #72

merged 4 commits into from
Jan 16, 2020

Conversation

rossberg
Copy link
Member

After rebasing on the bulk proposal, this PR fixes all open todos in interpreter and spec:

  • Move to small-step semantics for table instructions [interpreter].
  • Add back-compat sugar for table instructions omitting table index [spec/interpreter].
  • Add optional item keyword to syntactically represent multi-instr element expressions, analogous to syntax for offset. [spec/interpreter]
  • Generalise bulk table ops to multiple tables [spec].
  • A few smaller todos in spec (segment instantiation, removing funcelems).

Two things still missing after this (modulo new design questions):

  • Adding declaration segments (Inadvertent exporting of Wasm functions #31) -- I already have a branch for that.
  • Actual tests for bulk table ops involving multiple tables, which is in the intersection of the two proposals -- the respective tests are generated, so I could use some help with extending the generators. ;)


6. Assert: due to :ref:`validation <valid-table.copy>`, a value of :ref:`value type <syntax-valtype>` |I32| is on the top of the stack.
7. Assert: due to :ref:`validation <valid-table.copy>`, :math:`F.\AMODULE.\MITABLES[y]` exists.
Copy link
Contributor

Choose a reason for hiding this comment

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

Why did you remove 6.?

Copy link
Member Author

Choose a reason for hiding this comment

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

Oops, numbering glitch. Fixed.


3. Let :math:`\X{ta}` be the :ref:`table address <syntax-tableaddr>` :math:`F.\AMODULE.\MITABLES[0]`.
3. Let :math:`\X{ta}_d` be the :ref:`table address <syntax-tableaddr>` :math:`F.\AMODULE.\MITABLES[x]`.
Copy link
Contributor

Choose a reason for hiding this comment

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

Why do you use _d and _s here instead of _x and _y? It seems to me that d and s are used below, but with different meanings.

Copy link
Member Author

Choose a reason for hiding this comment

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

Used that at first, but since almost everything is expressed in terms of s and d from here on, it seemed clearer to focus on the relation to those names. Changed back.

....................

.. todo:: TODO: multi tables
:math:`\TABLEINIT~x~y`
Copy link
Contributor

Choose a reason for hiding this comment

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

Should we use s like segment here instead of y? Or could we even use seg?

Copy link
Member Author

Choose a reason for hiding this comment

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

These are in the syntactic class of indices, which are ranged over by x and y per the convention established in the respective section.

@rossberg
Copy link
Member Author

@lars-t-hansen, are you eager to review as well or shall I land with @gahaas's approval?

@lars-t-hansen
Copy link
Contributor

Oh, I missed the request. I'm fine if you just want to land it.

@rossberg rossberg merged commit 3e32403 into master Jan 16, 2020
@binji binji deleted the smallstep branch January 16, 2020 18:17
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants