-
Notifications
You must be signed in to change notification settings - Fork 40
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
ffi array type interop with opaque types #64
Comments
When doing a type assertion (cast) in go, if you don't provide a (discarded) failure bool variable it will panic (with a description) if it fails. In other words, you would do this: xs := xs_.([]Any) There's no current way to do it from spago, but if you do the
(you could also just do Let me know if that helps with your debugging. |
I see a lot of casts in the ffi definitions using the two-result value variant. There's a note in the readme on removing this once you're confident of the types. Is this something that should be cleaned up, and is there some overhead? |
The idea was to end up with the (allegedly faster) two-result (ignored failure) assertions in ffi implementations. In my readme I intended to convey that an author should use the debug-friendly (one-result) version while testing. I say "allegedly faster" because TBH I don't know if in recent versions of golang the non-panic assertions are still faster. Providing |
Ah I misunderstood. I probably read it too fast while frantically debugging. Writing and maintaining two files is definitely error prone. I guess you could provide a few casting helper functions that are generated in debug or release mode based on a flag and the ffi could target those. Or there could be a preprocessor step. Also, couldn't the runtime be generated, since everything is built from source anyway? Is there a benefit to have it as a separate library? |
Well, if I end up getting a definitive answer that the panics don't cause any overhead I would just get rid of the two-result versions everywhere. I'd like to avoid any kind of preprocessor and keep things simple and fast-building. Benefit to separate runtime library is simply cleaner and easier maintenance. It's not set in stone, though. |
It looks like you've gone with using the checked versions andyarvanitis/purescript-native-go-ffi@8512b65, so that answers the silent failure part of my question. The other part was whether boxing arrays into Having to re-allocate arrays to box them is not great in my opinion, even if it is only a constant factor difference, since it immediately adds overhead to using purescript-native over writing the code in go. This can lead people to push more logic into the ffi for performance reasons, getting less use our of purescript's abstractions. |
I don't think there's a way around arrays needing to be of type |
I was trying to return an array from a function and use it with common Foldable and Traversable functions, and hit a wall of silent failures. Luckily, since the build doesn't detect changes in the output directory, I was able to throw some printlns in there and rebuild without it being overwritten. I found that casts in functions like
Data.Array.length
likereturn an empty array when the cast to interface fails.
To fix this, the type cannot simply be assigned to interface:
so it needs to be copied, as this go wiki page points out:
https://github.com/golang/go/wiki/InterfaceSlice#what-can-i-do-instead
I'm wondering whether there's any way to get around this. Maybe define Traversable instances for an opaque type? I haven't tried this and not sure how much boilerplate it would require, but if there is some boilerplate on the go ffi side, maybe it could be provided in the small runtime library where Fn1-10 reside.
Below is the ffi function I ended up with that copies the array:
The text was updated successfully, but these errors were encountered: