Sketch-sh / sketch-sh

Sketch.sh - Online Code Editor and ReasonML community
https://sketch.sh
Other
425 stars 37 forks source link

Obj.magic() causes code execution to fail #224

Open jozanza opened 5 years ago

jozanza commented 5 years ago

I'm really loving Sketch 🎉, however, I ran into problems trying to use OCaml's Queue module -- the types all checked out, and code execution ran without issue, until I actually called Queue.create().

After some digging, I realized that Queue uses some unsafe type coercion via Obj.magic() in a number of methods; Queue.create() is one of them.

Further testing revealed that Obj.magic() worked in cases involving bool, int, and float, but broke code execution when used to assign string or record type.

I have a Sketch that illustrates the problem in more detail: https://sketch.sh/s/O5RAG6SC1rDRGzTu3d6GaU/

Right now, it seems like the only possible workaround is to roll my own Queue module. I'm still fairly new when it comes to reason/ocaml, but I'd love to help out if possible.

thangngoc89 commented 5 years ago

Hello, thank you for using and loving Sketch.sh. Queue isn't part of the stdlib, though it's often shipped with the compiler. Currently Sketch only supports the stdlib (at OCaml 4.06). There are plans to support loading external packages.

Regardings your problem, I think this is an upstream issue (with JSOO) or maybe how your Queue is implemented. I will have tho dig deeper to see what the actual issue is.

In the mean time, here is a Queue implementation you can use that works with Sketch.

https://sketch.sh/s/YAdMoaIgTvVcdGGeOs1njq/

Drup commented 5 years ago

Queue is part of the standard library.

thangngoc89 commented 5 years ago

@Drup I'd only consider anythings that's linked with -I +stdlib to be the real stdlib. Anything else isn't. For ex: Str, Unix

Drup commented 5 years ago

Yes, and Queue is part of that. The content of the stdlib is shown here.

thangngoc89 commented 5 years ago

@Drup interesting. I wonder why sketch doesn't have Queue module

jozanza commented 5 years ago

Thanks for the alternate implementation @thangngoc89!

FWIW, I've been using the reasonml docs as a stdlib reference. When I realized Queue references broke my Sketch, I just copy-pasted the vendored ocaml dependency from bsb-native, which appears to have been authored by INRIA back in 2002. That's where the Obj.magic() came from in the first place. I never even knew that was a thing prior 😆

EDIT: I also just rolled my own simple Queue module that is API-compatible with the stdlib Queue 👍 https://sketch.sh/s/ZygnP0cDT7SpYg6YwD4T1q/

Drup commented 5 years ago

FWIW, the current version of Queue doesn't use Obj.magic

jozanza commented 5 years ago

@Drup Thanks that's exactly what I needed! Not sure why bsb-native is using a weird out-of-date version 🤷‍♂