wasmerio / wai

A language binding generator for `wai` (a precursor to WebAssembly interface types)
Apache License 2.0
116 stars 13 forks source link

Make sure wit-bindgen-gen-wasmer-py uses forward references for named types #7

Closed Michael-F-Bryan closed 2 years ago

Michael-F-Bryan commented 2 years ago

I was using wit-bindgen-gen-wasmer-py and noticed that you can get code like this:

class Exports:
  @classmethod
  def from_path(...) -> Expected['Exports', Error]:
    ...

class Error:
  ...

In this case, Exports.from_path()'s return type references Error before the Error class has been defined, so we get a NameError when importing bindings.py.

$ ipython
> import wit_pack_py

---------------------------------------------------------------------------
NameError                                 Traceback (most recent call last)
<ipython-input-1-473065f48e2f> in <module>
----> 1 import wit_pack_py

~/Documents/wasmer/wit-pack/wit_pack_py/__init__.py in <module>
      5 from pathlib import Path
      6
----> 7 from .bindings import WitPack
      8
      9 store = Store()

~/Documents/wasmer/wit-pack/wit_pack_py/bindings.py in <module>
    208
    209
--> 210 class Exports:
    211
    212     _wasm_val: int

~/Documents/wasmer/wit-pack/wit_pack_py/bindings.py in Exports()
    240     def from_wit(
    241         cls, obj: "WitPack", name: str, contents: str
--> 242     ) -> Expected["Exports", Error]:
    243         memory = obj._memory
    244         realloc = obj._canonical_abi_realloc

NameError: name 'Error' is not defined
syrusakbary commented 2 years ago

It would be good to add a test case for the future :)

Michael-F-Bryan commented 2 years ago

It would be good to add a test case for the future :)

Yeah, that's a good point although I'm not sure how much we would gain from it. Almost all of the code generated by wit-bindgen's internals is untested and because of the imperative way it's been written, it's very hard to test the code generated for one component (e.g. a method signature) in isolation.

I could add another integration test, but because the NameError bug depends on the order used to generate items, the test would be subtle and you wouldn't understand what it's checking without reading through this issue.