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

Only generate non-included structs as Opaque if only referenced by pointer #395

Closed
dcharkes opened this issue Mar 5, 2021 · 5 comments · Fixed by dart-archive/ffigen#181

Comments

@dcharkes
Copy link
Collaborator

dcharkes commented Mar 5, 2021

https://github.com/dart-lang/ffigen/blob/89084d968ce0d587b19290366762ab7af7ffbe61/README.md#L474-L477

If structs are only referenced by Pointer<struct> we should generate them as Opaque if they are not included.

For example here.

If someone wants the full struct, they can include it.

@mannprerak2
Copy link
Contributor

mannprerak2 commented Mar 5, 2021

It's not always practical to include structs manually in config (since there could be a lot of them).

So I think we should probably keep this behind a flag (Something like opaque-excluded-structs?, default: false).
With this, ffigen will generate complete structs by default, but if the option is enabled, structs that were not included would be Opaque if referenced only with Pointer.

@dcharkes
Copy link
Collaborator Author

dcharkes commented Mar 5, 2021

It's not always practical to include structs manually in config (since there could be a lot of them).

So far, in using BoringSSL it's been only a dozen.

I think we should probably keep this behind a flag

Agreed.

I'm not entirely sure about what the default should be. When generating Opaque you cannot allocate them nor access their fields, but if the API only uses pointers to the struct anyway it's likely that you'll always interact through the pointer. (This is at least true for BoringSSL and SQLite.) So, I think the default should be generate them as Opaque. However, there might be C APIs out there which do only expose pointers to structs in their API but still expect you to either access the structs fields or to allocate them yourself. Do you know of any?

(Something like opaque-excluded-structs?, default: false)

structs:
  include:
    - MyStruct
  dependencies: full | opaque

The structs that are pulled into the generated code because they are dependencies are either generated fully or opaque. wdyt?

@mannprerak2
Copy link
Contributor

dependencies: full | opaque

This seems cool too. But I think the default should be full, since it doesn't really affect the usage of the library if they were completely generated instead of being Opaque (but not the other way around). I'd like the bindings to be complete if the user hasn't specified explicitly.

@dcharkes
Copy link
Collaborator Author

dcharkes commented Mar 5, 2021

I'd like the bindings to be complete if the user hasn't specified explicitly.

That's fair.

I was approaching it the other way around, I'd like to generate as small a set of bindings for my use case as possible.

@mannprerak2
Copy link
Contributor

On second thought, I think we should have a top level key called struct-dependencies with value full or opaque.
Nesting it under structs makes it seems like this only targets dependencies for structs and not dependencies of functions.

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

Successfully merging a pull request may close this issue.

3 participants