Skip to content
This repository has been archived by the owner on Jun 24, 2024. It is now read-only.

Consider removing the bindgen dependency #26

Closed
philpax opened this issue Mar 16, 2023 · 4 comments · Fixed by #28
Closed

Consider removing the bindgen dependency #26

philpax opened this issue Mar 16, 2023 · 4 comments · Fixed by #28

Comments

@philpax
Copy link
Collaborator

philpax commented Mar 16, 2023

#22 and #24 show that clang is required as a build dependency because bindgen uses it to read in ggml.h and produce bindings. We should consider just writing the C bindings ourselves - it shouldn't be too difficult as long as we account for the idiosyncrasies (like #6)

@setzer22
Copy link
Collaborator

setzer22 commented Mar 16, 2023

Would it be possible to use bindgen as a dev dependency and just push the generated code?

Manually maintaining C bindings sounds very tedious 😅

@philpax
Copy link
Collaborator Author

philpax commented Mar 16, 2023

Possibly - not sure if there are any concerns with architecture-specific behaviour that would get baked in. Maintaining the bindings ourselves lets us handle that.

Maintaining the C bindings would be tedious, but I can't imagine that ggml's interface will change much over time. We'd have the initial pain, and then we'd have to add or remove a few lines every so often - shouldn't be too bad!

@setzer22
Copy link
Collaborator

setzer22 commented Mar 16, 2023

Hmmm, good point. But one thing that concerns me a bit with doing this manually is that if we mistype something in the bindings, or forget to update something, and then call a function with an incorrect signature, that's UB. Isn't it?

@philpax
Copy link
Collaborator Author

philpax commented Mar 16, 2023

It can be - I hope that doesn't happen, but it's definitely a possibility. We don't depend on too much from ggml as it is now anyway, right?

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants