-
Notifications
You must be signed in to change notification settings - Fork 72
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
Spec type and module validation, subtyping, and module instantiation. #382
Conversation
document/core/util/macros.def
Outdated
|
||
.. |TFUNC| mathdef:: \xref{syntax/types}{syntax-comptype}{\K{func}} | ||
.. |TSTRUCT| mathdef:: \xref{syntax/types}{syntax-comptype}{\K{struct}} | ||
.. |TARRAY| mathdef:: \xref{syntax/types}{syntax-comptype}{\K{array}} | ||
.. |TSUB| mathdef:: \xref{syntax/types}{syntax-subtype}{\K{sub}} | ||
.. |TREC| mathdef:: \xref{syntax/types}{syntax-rectype}{\K{rec}} | ||
.. |TFINAL| mathdef:: \xref{syntax/types}{syntax-subtype}{\K{filan}} |
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.
Typo: filan
→ final
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.
Oh dear, thanks, fixed.
@conrad-watt, ping. |
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.
Just flagging up that I may fail to catch some semantic errors in matching due to my imperfect understanding - I'm still working to grok.
document/core/valid/conventions.rst
Outdated
i.e., all :ref:`type indices <syntax-typeidx>` have been :ref:`substituted <notation-subst>` with their :ref:`defined type <syntax-deftype>` and all free recursive type indices have been :ref:`unrolled <aux-unroll-rectype>`. | ||
|
||
.. note:: | ||
Recursive type indices are internal to a recursive types. |
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.
nit: "to a recursive types"
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.
Fixed.
document/core/valid/conventions.rst
Outdated
\rollrt_x(\TREC~\subtype^\ast) &=& \TREC~(\subtype[(x + i)^\ast \subst (\REC~i)^\ast])^\ast \\ | ||
&&& (\iff i^\ast = 0 \cdots (|\subtype^\ast| - 1)) \\ | ||
\unrollrt(\TREC~\subtype^\ast) &=& \TREC~(\subtype[(\REC~i)^\ast \subst ((\TREC~\subtype^\ast).i)^\ast])^\ast \\[2ex] | ||
&&& (\iff i^\ast = 0 \cdots (|\subtype^\ast| - 1)) \\ |
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.
indenting here seems messy - it makes sense to put the side-condition on its own line if the previous line is too long to fit it alongside, but the the &&&
then indents the side-condition past the end of the previous line, creating a scroll bar anyway.
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.
Fixed.
document/core/valid/conventions.rst
Outdated
\unrollrt(\TREC~\subtype^\ast) &=& \TREC~(\subtype[(\REC~i)^\ast \subst ((\TREC~\subtype^\ast).i)^\ast])^\ast \\[2ex] | ||
&&& (\iff i^\ast = 0 \cdots (|\subtype^\ast| - 1)) \\ | ||
\rolldt_x(\rectype) &=& (\rectype'.i)^\ast & (\iff \rollrt_x(\rectype) = \rectype' = \TREC~\subtype^\ast \\ | ||
&&& \land i^\ast = 0 \cdots (|\subtype^\ast| - 1)) \\ |
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.
nit: add whitespace after \land
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.
Fixed.
|
||
* Then the type sequence is valid. | ||
|
||
.. math:: |
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.
probably should move each judgement to a separate line, and separate the premises of the first judgement across multiple lines
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.
Moved rules to separate lines.
document/core/valid/types.rst
Outdated
C \vdashcomptype \TSTRUCT~\X{ft}^\ast \ok | ||
} | ||
|
||
:math:`TARRAY~\fieldtype` |
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.
nit: TARRAY
should be \TARRAY
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.
Fixed.
Recursive Types | ||
~~~~~~~~~~~~~~~ | ||
|
||
:ref:`Recursive types <syntax-rectype>` are validated for a specific :ref:`type index <syntax-typeidx>` that denotes the index of the type defined by the recursive group. |
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.
Do we want the additional index to be notated like this? It seems hard to read. I'd expect a judgement of either the form C |- rec subtype* : x
or C, x |- rec subtype* ok
instead of C |- rec subtype* ok (x)
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.
It's essentially the former, but I wanted to keep the "ok" for consistency with the other type wf judgements. Moving it to the left doesn't look right to me, since it is not really a context.
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.
What about C |-_{x} rec subtype* ok
(i.e. a subscript)?
I'm personally quite negative about ok (x)
because it looks unstructured/like a typo, and we don't normally use brackets this way.
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.
Subscripts on turnstiles are usually used for distinguishing different relations or modes of relations. So I wouldn't want to put it there.
Not sure I follow what's so bad about ok(x)
as a classifier, it's like an indexed ok. Note that I deliberately didn't put a space where you put it. :)
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.
Oh, I get it, I just noticed that the macros mess up the spacing in the rendered version. That should be fixed now.
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 think you pushed this on the other PR's branch
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.
Oops, yes, pushed here now.
document/core/util/macros.def
Outdated
.. |vdashpackedtype| mathdef:: \xref{valid/types}{valid-packedtype}{\vdash} | ||
.. |vdashstoragetype| mathdef:: \xref{valid/types}{valid-storagetype}{\vdash} | ||
.. |vdashsubtype| mathdef:: \xref{valid/types}{valid-subtype}{\vdash} | ||
.. |vdashrectype| mathdef:: \xref{valid/types}{valid-retype}{\vdash} |
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.
should this be valid-rectype
instead of valid-retype
?
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.
Fixed.
document/core/valid/matching.rst
Outdated
|
||
* Or :math:`\heaptype_2` is a :ref:`type index <syntax-typeidx>` :math:`x_2`, and :math:`\heaptype_1` :ref:`matches <match-heaptype>` :math:`C.\CTYPES[x_2]`. | ||
* Or :math:`\heaptype_1` is a :ref:`defined type <syntax-deftype>` which :ref:`expands <aux-expand-deftype>` to a :ref:`structure type <syntax-structtype>` and :math:`\heaptype_2` is :math:`STRUCT`. |
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.
should STRUCT
, ARRAY
and FUNC
here be macros?
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.
Fixed.
|
||
* The :ref:`result type <syntax-resulttype>` :math:`[t_{11}^\ast]` :ref:`matches <match-resulttype>` :math:`[t_{21}^\ast]`, and vice versa. | ||
* The :ref:`result type <syntax-resulttype>` :math:`[t_{21}^\ast]` :ref:`matches <match-resulttype>` :math:`[t_{11}^\ast]`. |
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.
Just double checking, this change to allow function subtyping is part of the MVP?
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.
Yes, that's why we introduced final types.
} | ||
|
||
.. note:: | ||
In future versions of WebAssembly, subtyping on function types may be relaxed to support co- and contra-variance. | ||
A :ref:`storage type <syntax-storagetype>` :math:`\storagetype_1` matches a type :math:`\storagetype_2` if and only if: |
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.
This prose doesn't seem to have an accompanying math block
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.
Because it's just injection. We could write out rules like
C ⊢ valtype_1 ≤ valtype_2
--------------------------
C ⊢ valtype_1 ≤ valtype_2
but that seems odd, and I omitted them elsewhere in similar situations.
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
Also, moved internal type extensions from syntax to validation chapter.
Baseline: #377