js: fix libyaml bindings #7918
js: fix libyaml bindings #7918
Conversation
|
Could be worth asking on discuss.ocaml.org if there's an easier way to fix the issue. |
|
|
||
| let fix_libyaml_field_offsets_for_wasm () = | ||
| (* mark *) | ||
| override_yaml_ctypes_field_offset Yaml_types.M.Mark.line 4; |
There was a problem hiding this comment.
How did you end up with all those magic constants below?
you disassembled the resulting WASM?
If tomorrow the bus hit you, how someone else can update this code if say the libyaml C library changes a little bit?
There was a problem hiding this comment.
Yep, when I was investigating this I created a macro that I used to print the offset of every struct field. I'll restructure and add more comments to make it clearer what's going on here.
Good idea, if folks have a cleaner solution I'll apply it in another PR 👍 edit: posted question here: https://discuss.ocaml.org/t/using-ocaml-ctypes-with-webassembly/12279 |
| (* event *) | ||
| override_yaml_ctypes_field_offset Yaml_types.M.Event.data 4; | ||
| override_yaml_ctypes_field_offset Yaml_types.M.Event.start_mark 32; | ||
| override_yaml_ctypes_field_offset Yaml_types.M.Event.start_mark 44; |
YAML parsing was fixed in #7918 and #7931, but I didn't catch that node start/end positions were incorrect. This was because in `ctypes_read()` we were returning a JS int when we should have been returning `UInt32` instead. test plan: - confirmed locally that the playground works properly w/ a yaml pattern - added unit test to confirm yaml check works and returns correct positions - `dune build && make test` succeeded
This PR fixes memory alignment issues that was preventing YAML parsing from working properly. TL;DR:
ocaml-yamlusesocaml-ctypesto generate bindings for thelibyamllibrary. Since this is done at compile time, the memory offsets are calculated based off of the build machine (64-bit address space --> 8 byte pointer size). WebAssembly, however, is 32-bit (4 bytes), and this mismatch prevented us from interfacing with the library properly.To fix, we override the memory offsets in JavaScript (exploiting its inherent mutability) at startup. This is a little weird, is much faster than figuring out how to cross-compile
ocaml-yamlagainst a 32-bit architecture.There is a weird off-by-one issue with the index of the end token in Yaml_to_generic, but I'll fix in a separate PR if it proves to be an issue.
test plan:
js/examples/yaml.htmlworks properly