WebAssembly / component-model

Repository for design and specification of the Component Model
Other
933 stars 79 forks source link

Update Type Checking compound component example #201

Closed danbev closed 1 year ago

danbev commented 1 year ago

This commit updates the example given in the Type Checking section.

The motivation for this update is that the current example does not seem to be correct:

$ echo '(component $A
  (type $ListString1 (list string))
  (type $ListListString1 (list $ListString1))
  (type $ListListString2 (list $ListString1))
  (component $B
    (type $ListString3 (list string))
    (type $ListListString3 (list $ListString3))
    (type $ListString4 (alias outer $A $ListString))
    (type $ListListString4 (list $ListString4))
    (type $ListListString5 (alias outer $A $ListString2))
  )
)' | wasm-tools parse -t
error: unexpected token, expected one of: `func`, `component`, `instance`, `resource`, `record`, `variant`, `list`, `tuple`, `flags`, `enum`, `union`, `option`, `result`, `own`, `borrow`
     --> <stdin>:8:25
      |
    8 |     (type $ListString4 (alias outer $A $ListString))
      |

With the updates in this commit the output will be:

$ echo '(component $A
  (type $ListString1 (list string))
  (type $ListListString1 (list $ListString1))
  (type $ListListString2 (list $ListString1))
  (component $B
    (type $ListString3 (list string))
    (type $ListListString3 (list $ListString3))
    (alias outer $A $ListString1 (type $ListString4))
    (type $ListListString4 (list $ListString4))
    (alias outer $A $ListListString2 (type $ListString5))
  )
)' | wasm-tools parse -t
(component $A
  (type $ListString1 (;0;) (list string))
  (type $ListListString1 (;1;) (list $ListString1))
  (type $ListListString2 (;2;) (list $ListString1))
  (component $B (;0;)
    (type $ListString3 (;0;) (list string))
    (type $ListListString3 (;1;) (list $ListString3))
    (alias outer $A $ListString1 (type $ListString4 (;2;)))
    (type $ListListString4 (;3;) (list $ListString4))
    (alias outer $A $ListListString2 (type $ListString5 (;4;)))
  )
)

I think this is correct but hopefully others can correct me if I'm wrong.

lukewagner commented 1 year ago

Thanks for trying out the examples! I think this might be a not-yet-implemented case in wasm-tools. In particular, if you search for the phrase "aliases can be written in an inverted form that puts the sort first" in the explainer, it describes an inverted syntax that I believe makes both forms (before and after this PR) equivalent. The inverted syntax is used a lot for functions in the examples (which validate in wasm-tools atm), so maybe it's just missing for types. Is that right @alexcrichton / @peterhuene ?

That being said, having the inverted syntax work for types is not particularly important and it is quite nice to have the examples in the explainer validate in wasm-tools, so if it's not a quick fix, we can merge this PR.

peterhuene commented 1 year ago

Inverted alias forms do not appear to be implemented in the wast crate used from wasm-tools.

I think a better course of action is to open an issue in wasm-tools to implement the inverted forms and leave the explainer examples as they are; we'll use them as test cases for the parser tests.

peterhuene commented 1 year ago

FYI, it appears the only currently supported inverted forms in wast for aliases are for the sorts core func and func.

lukewagner commented 1 year ago

Thanks @peterhuene! sgtm

danbev commented 1 year ago

I think this might be a not-yet-implemented case in wasm-tools. In particular, if you search for the phrase "aliases can be written in an inverted form that puts the sort first" in the explainer, it describes an inverted syntax that I believe makes both forms (before and after this PR) equivalent.

Ah sorry about that, I did not realize this was using the inverted form.

I think a better course of action is to open an issue in wasm-tools to implement the inverted forms and leave the explainer examples as they are; we'll use them as test cases for the parser tests.

Sounds good! I'll open an issue for this :+1:

@lukewagner @peterhuene Thanks for the feedback and sorry about the noise. I'll close this pull request and open an issue in wasm-tools.