tov / succinct-rs

Succinct Data Structures for Rust
Apache License 2.0
56 stars 15 forks source link

Fixing compilation for wasm32 targets #9

Closed luizirber closed 4 years ago

luizirber commented 5 years ago

Trying to fix succinct-rs to run in wasm32 (since it's a transitive dependency for me, I'm using pdatastructs.rs). As the name implies, wasm32 is 32-bit and some implementations are missing (where checks for 64-bit pointer size were used).

This is a WIP just doing the bare minimum to make it compile and pass the current tests, but still need to write more tests for 32-bit cases.

tov commented 4 years ago

I’m wondering—I took WIP to mean don’t actually merge yet. Was that wrong?

luizirber commented 4 years ago

Is it OK if I add two github actions workflows for compiling and testing under wasm32? With https://github.com/actions-rs/cargo it is easier to setup than on Travis, and I also don't want to extend the travis build times too much

tov commented 4 years ago

That sounds like a good idea to me. Go for it!

On Fri, Nov 8, 2019 at 2:15 PM Luiz Irber notifications@github.com wrote:

Is it OK if I add two github actions workflows for compiling and testing under wasm32? With https://github.com/actions-rs/cargo it is easier to setup than on Travis, and I also don't want to extend the travis build times too much

— You are receiving this because you commented. Reply to this email directly, view it on GitHub https://github.com/tov/succinct-rs/pull/9?email_source=notifications&email_token=AAFS3BCFVXVEUK4ODKUSTODQSXCFNA5CNFSM4ISHP2FKYY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGOEDTHNWA#issuecomment-551974616, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAFS3BB626SQKWG5P4AQJELQSXCFNANCNFSM4ISHP2FA .

-- Dr. Jesse A. Tov Assistant Professor of Instruction Computer Science McCormick School of Engineering Northwestern University

http://www.cs.northwestern.edu/~jesse/

luizirber commented 4 years ago

Added! The check is not showing here yet (because only members of the repo can add new actions), but here is the output from it running in my fork: https://github.com/luizirber/succinct-rs/runs/295115774

Yasu-umi commented 4 years ago

Hi! @tov I use this library for my nlp library (japanese morphological analyzer), and try to compile for wasm. Is there anything to need to merge this pull req? or anything I can help with?

tov commented 4 years ago

@Yasu-umi. Sorry for being so slow—I just merged it. Would you also like a release?

Yasu-umi commented 4 years ago

@tov thank you for reply. I'm looking forward to release!