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 CxxChar to bind to C++ char32_t type #725

Open
wants to merge 3 commits into
base: master
Choose a base branch
from

Conversation

edsrzf
Copy link
Contributor

@edsrzf edsrzf commented Feb 20, 2021

This makes it possible to implement functions in Rust that accept a char32_t. The CxxChar type is necessary because C++ does not guarantee that char32_t contains a valid Unicode scalar value.

There is not yet a way for C++ code to create a Rust char.

I'll update the docs after getting feedback on the design and implementation.

Partly addresses #592.

This makes it possible to implement functions in Rust that accept
a char32_t. The CxxChar type is necessary because C++ does not guarantee
that char32_t contains a valid Unicode scalar value.

There is not yet a way for C++ code to create a Rust char.
/// To avoid confusion with Rust's built-in char type you probably
/// shouldn't import this type with `use`. Instead, write `cxx::Char`, or
/// import and use `CxxChar`.
pub type Char = CxxChar;
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I wasn't entirely sure of the reason for these aliases, so just following the existing pattern. 🤷‍♂️

Copy link
Owner

@dtolnay dtolnay left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for the PR! This implementation looks good. However I wonder if it's necessary to have a repr(transparent) newtype wrapper, or if we could use just u32 as the type on the Rust side? Do you have a use case where providing a wrapper type is important?

@edsrzf
Copy link
Contributor Author

edsrzf commented Mar 23, 2021

The problem with using straight u32 for the Rust type is that if we do this:

fn rust_fn(ch: u32) {}

#[cxx::bridge]
pub mod ffi {
    extern "Rust" {
        fn rust_fn(ch: u32);
    }
}

...then the generated C++ declaration will look something like this:

void rust_fn(::std::uint32_t ch) noexcept;

...and uint32_t and char32_t are distinct C++ types, mangled differently. (Reference link for the IA64 ABI followed by gcc and clang.)

It's not specifically important to me to provide a wrapper type, but it is important to me to be able to generate C++ function signatures compatible with char32_t. If there's a better way to accomplish this, then I'm all ears.

Copy link
Owner

@dtolnay dtolnay left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Okay in that case I would ask that there not be a newtype wrapper. char32_t in the bridge should use u32 in Rust and ::std::char32_t in C++. You can also provide a type alias for u32 as something like cxx::num::char32_t for downstream code to use if they choose.

@edsrzf
Copy link
Contributor Author

edsrzf commented Mar 24, 2021

So just to be clear, would you expect to be able to write:

fn rust_fn(ch: u32) {}

#[cxx::bridge]
pub mod ffi {
    extern "Rust" {
        fn rust_fn(ch: char32_t);
    }
}

...and then generate a C++ declaration using char32_t?

@dtolnay
Copy link
Owner

dtolnay commented Mar 24, 2021

Right. That is how the standard library and libc do C integer types.

https://doc.rust-lang.org/1.50.0/std/os/raw/index.html
https://docs.rs/libc/0.2.91/libc/#types

Canonically the rust_fn implementation would be written using cxx::num::char32_t though.

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.

2 participants