kylegoetz / tree-sitter-unison

Tree Sitter grammar for Unison programming language
MIT License
7 stars 8 forks source link

Remove `fprint`f calls for WASM build. #67

Closed zetashift closed 7 months ago

zetashift commented 7 months ago

Currently the WASM build in Zed's extension pipeline seems to fail because of fwrite being used.

Error: Failed to instantiate wasm module: invalid import 'fwrite'

This might be because of fprintf calls, which Zed had the following to say about: https://github.com/zed-industries/extensions/pull/361#issuecomment-2062441610

Maybe it calls fprintf or similar? If so, maybe that can be wrapped in an ifdef, disabling it for wasm builds? or remove it completely?

Can we do this?

For the fprintf usage: https://github.com/search?q=repo%3Akylegoetz%2Ftree-sitter-unison%20fprintf&type=code

kylegoetz commented 7 months ago

@zetashift I have no easy way to test if my solution fixes this, so could you test the branch 67/fprintf-wasm?

https://github.com/kylegoetz/tree-sitter-unison/tree/67/fprintf-wasm

Some googling suggests that Emscripten defines __EMSCRIPTEN__ for its builds, so I'm using fprintf only #ifndef __EMSCRIPTEN__.

kylegoetz commented 7 months ago

according to https://emscripten.org/docs/compiling/Building-Projects.html#detecting-emscripten-in-preprocessor

The preprocessor define EMSCRIPTEN is always defined when compiling programs with Emscripten.

I think the WASM build process uses Emscripten, which means I should be able to#68

#ifndef __EMSCRIPTEN__
#define LOG things_using_fprintf
#else
#define LOG not_using_fprintf
#endif
kylegoetz commented 7 months ago

update based on talks over on the zed github, they suggested __wasm32__ instead of __EMSCRIPTEN__ so awaiting @zetashift confirmation that it works, then will merge the fix into main