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

Do not generate bindings for functions implemented in the header #120

Closed
armanbilge opened this issue Sep 19, 2022 · 11 comments
Closed

Do not generate bindings for functions implemented in the header #120

armanbilge opened this issue Sep 19, 2022 · 11 comments

Comments

@armanbilge
Copy link

https://github.com/axboe/liburing/blob/0535620c15e1133f9c19aefe9e928e0607e6c2b2/src/include/liburing.h#L283-L309

They will compile but graduate to linking errors.

@keynmol
Copy link
Contributor

keynmol commented Sep 19, 2022

Actually a lot of libraries are defined entirely in an amalgamated header file, so it's not this

I think this is down to static inline.

A function defined with static inline. Stand-alone object code may be emitted if required. You can have multiple definitions in your program, in different translation units, and it will still work. This is the same as the C99 rules.

Need to read this more carefully: https://www.greenend.org.uk/rjk/tech/inline.html

normal functions defined in headers are a fair game, but various inline functions are at compiler's mercy, and as such aren't guaranteed to be present in the object file.

I feel like as a stopgap those shouldn't be generated at all if they are marked "inline" (regardless of static)

Did you find a linking error attempting to use it?

@armanbilge
Copy link
Author

Yes I did, you can see in armanbilge/fs2-io_uring#2.

@keynmol
Copy link
Contributor

keynmol commented Sep 19, 2022

Hmm, so should this be considered internal API then?
Can you achieve what you need without using that function?

Because as it stands it's sort of at compiler's mercy to include/not include it.

If you got a linking error, that means the shared library for io_uring doesn't have an entry for this function - which must mean you're supposed to use some other means of achieving the same effect?

@armanbilge
Copy link
Author

Hmm, so should this be considered internal API then?

No, it's public API.

Because as it stands it's sort of at compiler's mercy to include/not include it.

I'm not sure what you mean.

which must mean you're supposed to use some other means of achieving the same effect?

If I were writing C code, I would include the header and that function would be available to me via that.

Can you achieve what you need without using that function?

Yes I believe so, but now it is much more annoying. My two options are:

  1. implement said utilities myself, in Scala
  2. include the header in a C glue code, and expose the methods to Scala

@keynmol
Copy link
Contributor

keynmol commented Sep 19, 2022

What I'm trying to get at is that inline functions are weird:

$ cat test.c
#include "test.h"

$ cat test.h
static inline int test_inline(int c) {
  return c + 5;
}

extern int text_regular(int c) {
  return c + 11;
}

If we just compile that, you can see that the inline function is elided:

$ clang -c test.h test.c

$ nm ./test.o
0000000000000000 T _text_regular
0000000000000000 t ltmp0
0000000000000018 s ltmp1

And even though text_regular is defined in the header, it's available in the binary artifact.

Now, if you use the inline function in the source that produces your binary artifact (liburing.dylib, for exaple), then inline function can be added there:


$ cat test1.c
#include "test.h"

void help() {
  test_inline(25);
}

$ clang -c test.h test1.c

$ nm test1.o
0000000000000018 T _help
0000000000000030 t _test_inline
0000000000000000 T _text_regular
0000000000000000 t ltmp0
0000000000000048 s ltmp1

What I'm saying is that

  1. The issue needs to be changed to clarify that we shouldn't "generate bindings for inline functions" - header or not, I think, is not an issue
  2. in a more general sense, I don't understand how the function can be part of public binary API if it may or may not be inlined depending on compiler settings..

@keynmol
Copy link
Contributor

keynmol commented Sep 19, 2022

Now, bindgen could generate a glue C function for an inlined one, thus ensuring that the compiler will add the inline function to list of symbols in the resulting object artifacts

@armanbilge
Copy link
Author

The issue needs to be changed to clarify that we shouldn't "generate bindings for inline functions" - header or not, I think, is not an issue

Hmm I see. Possibly true.

in a more general sense, I don't understand how the function can be part of public binary API if it may or may not be inlined depending on compiler settings..

It's because when I include the header that function definition will be in my sources (post-preprocessor). So if it gets inlined, no problem, and if it doesn't, then it will still be defined in my object files. No?

@keynmol
Copy link
Contributor

keynmol commented Sep 19, 2022

Yes, if you accompany a binary liburing artifact with your own sources that exercise these inlined functions, then they should either be preserved, or fully inlined - and this means that the C functions you write shouldn't notice either way.

That's my understanding at least - which means that bindgen can generate Scala functions that invoke wrappers that invoke inline functions...

@armanbilge
Copy link
Author

Yes, if you accompany a binary liburing artifact with your own sources that exercise these inlined functions, then they should either be preserved, or fully inlined - and this means that the C functions you write shouldn't notice either way.

Right, I wasn't aware there was another way to use these functions 😛

@keynmol
Copy link
Contributor

keynmol commented Sep 19, 2022

I've created #121 to track this particular improvement.
You can do as you wish with this issue :P

@armanbilge
Copy link
Author

@armanbilge armanbilge closed this as not planned Won't fix, can't repro, duplicate, stale Sep 19, 2022
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

2 participants