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

va_list type support (stdargs.h) #41

Closed
olbat opened this issue Feb 8, 2017 · 10 comments
Closed

va_list type support (stdargs.h) #41

olbat opened this issue Feb 8, 2017 · 10 comments

Comments

@olbat
Copy link
Contributor

olbat commented Feb 8, 2017

Hello,

I'm trying to port a library that uses a va_list type (from stdargs.h) using crystal_lib.

The problem is that it generates an alias to LibC::VaList that doesn't seems to exists in any of crystal's lib C bindings so I get an undefined constant LibC::VaList at compile time.

Example:

$ cat /tmp/test.h
#include <stdarg.h>
void test(va_list vl);

$ cat test.cr
require "lib_c"
@[Include("/tmp/test.h", prefix: "test", remove_prefix: false)]
lib LibTest
end

$ crystal src/main.cr -- test.cr
require "lib_c"
lib LibTest
  fun test(vl : VaList)
  alias X__GnucVaList = LibC::VaList
  alias VaList = X__GnucVaList
end

Am I doing something wrong ?

@mverzilli
Copy link

I investigated this (by the way, thank you so much for including a self contained example in the issue ❤️ ), and I think you're doing nothing wrong.

If, for example, in the generated code you replace LibC::VaList with LibC::ULongLong, it compiles.

I think the problem is Crystal's LibC bindings don't define VaList: https://github.com/crystal-lang/crystal/blob/master/src/lib_c.cr

At the same time crystal_lib asumes that it's going to be defined somewhere:

path ["LibC", type.kind.to_s]

So a workaround would be to define it (you can reopen lib LibC as many times as you want)

@olbat
Copy link
Contributor Author

olbat commented Feb 10, 2017

Thank you for your answer !

I'm not sure that replacing LibC::VaList by LibC::ULongLong is the best way to solve this issue since it depends on the system's lib C implementation (so it might not work on every platform) but I think I'll do so as a temporary fix for sure.

If I understand it properly, I think that crystal's LibC lib is generated using @ysbaddaden's posix generator that does not support stdargs.h yet (maybe because there is only macro and types definition in the header ?).

A way to solve this may be to add / ask for it's support in the crystal's LibC/posix binding ?

@mverzilli
Copy link

Sorry, maybe my reply wasn't clear. I'm not proposing to replace LibC::Valist with LibC::ULongLong, I just used that as a way of checking against the bindings that come with Crystal. Anyway, I think we're on the same page with that :).

About the LibC lib being generated with posix, I don't really know, so we can wait for him to confirm. If that's the case, maybe we can move this issue there.

There's quite a lot of types that crystal_lib still doesn't support, maybe we should throw a NotImplementedException when we stumble upon them?

@ysbaddaden
Copy link
Contributor

We don't support va_list. We don't use it in Crystal, and it's interface is a bunch of macros that would have to be rewritten for each platform.

You may use ... in fun definitions instead, which will be handled by LLVM.

@mverzilli
Copy link

mverzilli commented Feb 11, 2017

Thanks for the context info @ysbaddaden! The thing that got me is that it is explicitly listed here:

PrimitiveType::Kind::VaList

So we need to translate it to a variadic function

@ysbaddaden
Copy link
Contributor

I believe the VaList type is for the ... syntax, not the va_list definitions that would be interpreted as C (functions or macros) by clang, which is exactly what happens in your initial example.

We can translate/map the implicit ... but we can't map the explicit va_list. I'm afraid that won't change. Are there no other ways to call the C functions?

@mverzilli
Copy link

I see! Watching crystal_lib issues is really forcing me out of my comfort zone with C, so I really appreciate your guidance 🙇 (and your issues @olbat !).

I've read a bit more about va_list and I now understand what you mean: it doesn't make any sense for a header other than stdargs.h itself to use va_list, given it's actually an implementation detail to support .... That makes me wonder, @olbat, what lib are you trying to bind to? Can you share that?

@olbat
Copy link
Contributor Author

olbat commented Feb 13, 2017

Are there no other ways to call the C functions?

Sadly, not for the functions I've checked ...

Anyway I think I'll leave them for now since it's not the most important functions of the library and I still have other problems to fix (such as symbols renaming/suffixing in shared libraries, I'll probably post something on the google group for this one).

That makes me wonder, @olbat, what lib are you trying to bind to? Can you share that?

Ofc: I'm working on a binding to the ICU library. If it goes well, I'll probably write a crystal wrapper too (at least for the main features since the lib is pretty big).

@Fryguy
Copy link
Contributor

Fryguy commented Jul 19, 2018

@olbat Can this be closed now that we have crystal-lang/crystal#5103 ?

@olbat
Copy link
Contributor Author

olbat commented Jul 20, 2018

Yes, I think it can be closed.

I'll try to run it on the ICU binding to see how it goes :)

@RX14 RX14 closed this as completed Jul 21, 2018
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

No branches or pull requests

5 participants