Skip to content
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

Add wrapper shared library #8

Merged
merged 8 commits into from
Feb 17, 2023
Merged

Add wrapper shared library #8

merged 8 commits into from
Feb 17, 2023

Conversation

berchn
Copy link
Contributor

@berchn berchn commented Feb 15, 2023

This creates a wrapper shared library to hide the pass-by-value struct parameters from lisp. This was done to reduce the number of times C structs were being converted to lisp representations and vice versa. The result is that this has cut the conversion time by about half.

There is no longer a dependency on the CFFI fork due to these changes. If there is still an interest in getting this into quicklisp, it may be more feasible now.

@berchn berchn mentioned this pull request Feb 15, 2023
@death
Copy link
Owner

death commented Feb 16, 2023

Nice, I got this to work by re-adding ts-language as the return type argument to foreign-funcall. Otherwise a call to language-module would return no values (which is then interpreted as a nil and passed to ts-parser-set-language, resulting in a type error since a pointer is expected). Why was it removed?

Few more things that may be non-issues in practice but caught my eye are:

  • bool (expands to _Bool) is used in the C file. Then type :boolean is used in the CFFI defcfun form. The latter type assumes :int representation by default, but the former is not guaranteed to be so. I would probably replace bool in the C file with int.
  • Returned values from malloc calls are not checked for nullness.
  • The wrapper file names are inconsistent: the C file uses underscores, while the shared object uses hyphens. I would use hyphens everywhere (Lisper after all).

@berchn
Copy link
Contributor Author

berchn commented Feb 16, 2023

Yeah, that's my bad on removing ts-language. I couldn't figure out what it was there for when I was reading the definition of foreign-funcall and evidently didn't dig far enough or look up the documentation online. I added it back in.

I switched bool over to int, but I'm of the opinion that it's a bug with CFFI if they were to ever not match on some system.

@death
Copy link
Owner

death commented Feb 17, 2023

I see that Lisp code was added to check for null pointer result, which is fine, but the malloc results are not checked in the C code. I will merge this and add the checks. Thanks!

@death death merged commit 371f685 into death:master Feb 17, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants