Skip to content

Handle pointers #102

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

Closed
adetaylor opened this issue Nov 14, 2020 · 5 comments
Closed

Handle pointers #102

adetaylor opened this issue Nov 14, 2020 · 5 comments
Labels
cpp-feature C++ feature not yet supported

Comments

@adetaylor
Copy link
Collaborator

At the moment, we're not handling pointers which may be null. We're just converting any pointers that we discover into references.

We probably need to convert Foo* into Option<&Foo> rather than Foo. This will probably require cxx support first, and/or wrapper functions.

It's also not clear whether the information output from bindgen even distinguishes references from pointers enough for us to make this distinction. TBD.

@adetaylor
Copy link
Collaborator Author

Here's my plan here.

  • bindgen needs to indicate the difference between reference and pointer in the generated bindings. Currently it does not.
    • bindgen currently generates (almost) identical bindings for uint32_t take_bob(const Bob& a) as uint32_t take_bob(const Bob* a). At some point, we'll need to upstream a patch to bindgen to indicate the difference, if they're amenable, and re-fork bindgen meanwhile to add extra metadata.
    • Meanwhile there is one exception: the #[link_name] attribute added to such functions. As a world-class dreadful hack, we can demangle that name to work out whether a given parameter is a reference or a pointer. Perhaps. This might be a hack too far even for me, but I'm still at the stage of figuring out whether autocxx is ergonomic.
    • Another option is to allow this to be specified in a directive within include_cpp!.
    • Mangled names don't include the return type, so we can't know if a function returns a pointer or a reference. Returning a reference is pretty unusual so I think we'll assume a pointer unless a directive says it's a reference. Probably.
    • A final option is to move away from bindgen to libclang directly, which has been proposed by a couple of people, but I tend to assume that autocxx currently relies on a large mountain of subtleties within bindgen which would be incredibly complex to replicate.
  • We will assume that C++ references can't be null.
  • We will assume that C++ pointers can be null.
  • For pointers, we will use UniquePtr in Rust bindings, just as we do for non-POD value parameters. This means we'll need to generate wrapper functions for a higher percentage of bindings, which will package and unpackage the std::unique_ptr into a raw pointer and vice-versa.
  • When calling functions that take a raw pointer parameter, I believe it is safe to call them using a UniquePtr from Rust.
  • When calling functions which return a raw pointer, we have no guarantees about the uniqueness of the return value. As such I think such functions need to be tagged as unsafe in order to indicate that the caller needs to take special care to ensure the uniqueness of the pointer.

@adetaylor adetaylor mentioned this issue Nov 24, 2020
8 tasks
@adetaylor
Copy link
Collaborator Author

See also dtolnay/cxx#115

@dtolnay
Copy link
Contributor

dtolnay commented Dec 4, 2020

Possibly more relevant upstream issue: dtolnay/cxx#164.

@adetaylor
Copy link
Collaborator Author

In progress at dtolnay/cxx#689

@adetaylor
Copy link
Collaborator Author

This landed!

@adetaylor adetaylor added the cpp-feature C++ feature not yet supported label Mar 25, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
cpp-feature C++ feature not yet supported
Projects
None yet
Development

No branches or pull requests

2 participants