Skip to content

Allow type aliases to be marked as Trivial. #325

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

Merged
merged 4 commits into from
Oct 4, 2020
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
29 changes: 22 additions & 7 deletions gen/src/write.rs
Original file line number Diff line number Diff line change
Expand Up @@ -80,6 +80,11 @@ pub(super) fn gen(
write_struct_with_methods(out, ety, methods);
}
}
Api::TypeAlias(ety) => {
if types.required_trivial_aliases.contains(&ety.ident) {
check_trivial_extern_type(out, &ety.ident)
}
}
_ => {}
}
}
Expand Down Expand Up @@ -124,13 +129,18 @@ pub(super) fn gen(
fn write_includes(out: &mut OutFile, types: &Types) {
for ty in types {
match ty {
Type::Ident(ident) => match Atom::from(ident) {
Some(U8) | Some(U16) | Some(U32) | Some(U64) | Some(I8) | Some(I16) | Some(I32)
| Some(I64) => out.include.cstdint = true,
Some(Usize) => out.include.cstddef = true,
Some(CxxString) => out.include.string = true,
Some(Bool) | Some(Isize) | Some(F32) | Some(F64) | Some(RustString) | None => {}
},
Type::Ident(ident) => {
match Atom::from(ident) {
Some(U8) | Some(U16) | Some(U32) | Some(U64) | Some(I8) | Some(I16)
| Some(I32) | Some(I64) => out.include.cstdint = true,
Some(Usize) => out.include.cstddef = true,
Some(CxxString) => out.include.string = true,
Some(Bool) | Some(Isize) | Some(F32) | Some(F64) | Some(RustString) | None => {}
};
if types.required_trivial_aliases.contains(&ident) {
out.include.type_traits = true;
};
}
Type::RustBox(_) => out.include.type_traits = true,
Type::UniquePtr(_) => out.include.memory = true,
Type::CxxVector(_) => out.include.vector = true,
Expand Down Expand Up @@ -401,6 +411,11 @@ fn check_enum(out: &mut OutFile, enm: &Enum) {
}
}

fn check_trivial_extern_type(out: &mut OutFile, id: &Ident) {
writeln!(out, "static_assert(std::is_trivially_move_constructible<{}>::value,\"type {} marked as Trivial in Rust is not trivially move constructible in C++\");", id, id);
writeln!(out, "static_assert(std::is_trivially_destructible<{}>::value,\"type {} marked as Trivial in Rust is not trivially destructible in C++\");", id, id);
}

fn write_exception_glue(out: &mut OutFile, apis: &[Api]) {
let mut has_cxx_throws = false;
for api in apis {
Expand Down
17 changes: 17 additions & 0 deletions macro/src/expand.rs
Original file line number Diff line number Diff line change
Expand Up @@ -57,6 +57,10 @@ fn expand(ffi: Module, apis: &[Api], types: &Types) -> TokenStream {
Api::TypeAlias(alias) => {
expanded.extend(expand_type_alias(alias));
hidden.extend(expand_type_alias_verify(namespace, alias));
let ident = &alias.ident;
if types.required_trivial_aliases.contains(ident) {
hidden.extend(expand_type_alias_kind_trivial_verify(alias));
}
}
}
}
Expand Down Expand Up @@ -179,6 +183,7 @@ fn expand_cxx_type(namespace: &Namespace, ety: &ExternType) -> TokenStream {

unsafe impl ::cxx::ExternType for #ident {
type Id = #type_id;
type Kind = ::cxx::Opaque;
}
}
}
Expand Down Expand Up @@ -677,6 +682,18 @@ fn expand_type_alias_verify(namespace: &Namespace, alias: &TypeAlias) -> TokenSt
}
}

fn expand_type_alias_kind_trivial_verify(type_alias: &TypeAlias) -> TokenStream {
let ident = &type_alias.ident;
let begin_span = type_alias.type_token.span;
let end_span = type_alias.semi_token.span;
let begin = quote_spanned!(begin_span=> ::cxx::private::verify_extern_kind::<);
let end = quote_spanned!(end_span=> >);

quote! {
const _: fn() = #begin #ident, ::cxx::Trivial #end;
}
}

fn type_id(namespace: &Namespace, ident: &Ident) -> TokenStream {
let mut path = String::new();
for name in namespace {
Expand Down
50 changes: 48 additions & 2 deletions src/extern_type.rs
Original file line number Diff line number Diff line change
Expand Up @@ -54,7 +54,7 @@
/// ## Integrating with bindgen-generated types
///
/// Handwritten `ExternType` impls make it possible to plug in a data structure
/// emitted by bindgen as the definition of an opaque C++ type emitted by CXX.
/// emitted by bindgen as the definition of a C++ type emitted by CXX.
///
/// By writing the unsafe `ExternType` impl, the programmer asserts that the C++
/// namespace and type name given in the type id refers to a C++ type that is
Expand All @@ -69,10 +69,11 @@
/// # pub struct StringPiece([usize; 2]);
/// # }
///
/// use cxx::{type_id, ExternType};
/// use cxx::{type_id, ExternType, Opaque};
///
/// unsafe impl ExternType for folly_sys::StringPiece {
/// type Id = type_id!("folly::StringPiece");
/// type Kind = Opaque;
/// }
///
/// #[cxx::bridge(namespace = folly)]
Expand All @@ -92,6 +93,29 @@
/// #
/// # fn main() {}
/// ```
///
/// ## Opaque and Trivial types
///
/// Some C++ types are safe to hold and pass around in Rust, by value.
/// Those C++ types must have a trivial move constructor, and must
/// have no destructor.
///
/// If you believe your C++ type is indeed trivial, you can specify
/// ```
/// # struct TypeName;
/// # unsafe impl cxx::ExternType for TypeName {
/// type Id = cxx::type_id!("name::space::of::TypeName");
/// type Kind = cxx::Trivial;
/// # }
/// ```
/// which will enable you to pass it into C++ functions by value,
/// return it by value from such functions, and include it in
/// `struct`s that you have declared to `cxx::bridge`. Your promises
/// about the triviality of the C++ type will be checked using
/// `static_assert`s in the generated C++.
///
/// Opaque types can't be passed by value, but can still be held
/// in `UniquePtr`.
pub unsafe trait ExternType {
/// A type-level representation of the type's C++ namespace and type name.
///
Expand All @@ -101,10 +125,32 @@ pub unsafe trait ExternType {
/// # struct TypeName;
/// # unsafe impl cxx::ExternType for TypeName {
/// type Id = cxx::type_id!("name::space::of::TypeName");
/// type Kind = cxx::Opaque;
/// # }
/// ```
type Id;

/// Either `cxx::Opaque` or `cxx::Trivial`. If in doubt, use
/// `cxx::Opaque`.
type Kind;
}

pub(crate) mod kind {

/// An opaque type which can't be passed or held by value within Rust.
/// For example, a C++ type with a destructor, or a non-trivial move
/// constructor. Rust's strict move semantics mean that we can't own
/// these by value in Rust, but they can still be owned by a
/// `UniquePtr`...
pub struct Opaque;

/// A type with trivial move constructors and no destructor, which
/// can therefore be owned and moved around in Rust code directly.
pub struct Trivial;
}

#[doc(hidden)]
pub fn verify_extern_type<T: ExternType<Id = Id>, Id>() {}

#[doc(hidden)]
pub fn verify_extern_kind<T: ExternType<Kind = Kind>, Kind>() {}
3 changes: 3 additions & 0 deletions src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -395,6 +395,8 @@ mod unwind;
pub use crate::cxx_string::CxxString;
pub use crate::cxx_vector::CxxVector;
pub use crate::exception::Exception;
pub use crate::extern_type::kind::Opaque;
pub use crate::extern_type::kind::Trivial;
pub use crate::extern_type::ExternType;
pub use crate::unique_ptr::UniquePtr;
pub use cxxbridge_macro::bridge;
Expand Down Expand Up @@ -422,6 +424,7 @@ pub type Vector<T> = CxxVector<T>;
#[doc(hidden)]
pub mod private {
pub use crate::cxx_vector::VectorElement;
pub use crate::extern_type::verify_extern_kind;
pub use crate::extern_type::verify_extern_type;
pub use crate::function::FatFunction;
pub use crate::opaque::Opaque;
Expand Down
7 changes: 6 additions & 1 deletion syntax/check.rs
Original file line number Diff line number Diff line change
Expand Up @@ -338,6 +338,7 @@ fn is_unsized(cx: &mut Check, ty: &Type) -> bool {
|| cx.types.cxx.contains(ident)
&& !cx.types.structs.contains_key(ident)
&& !cx.types.enums.contains_key(ident)
&& !cx.types.required_trivial_aliases.contains(ident)
|| cx.types.rust.contains(ident)
}

Expand Down Expand Up @@ -376,7 +377,11 @@ fn describe(cx: &mut Check, ty: &Type) -> String {
} else if cx.types.enums.contains_key(ident) {
"enum".to_owned()
} else if cx.types.cxx.contains(ident) {
"C++ type".to_owned()
if cx.types.required_trivial_aliases.contains(ident) {
"trivial C++ type".to_owned()
} else {
"non-trivial C++ type".to_owned()
}
} else if cx.types.rust.contains(ident) {
"opaque Rust type".to_owned()
} else if Atom::from(ident) == Some(CxxString) {
Expand Down
51 changes: 51 additions & 0 deletions syntax/types.rs
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@ pub struct Types<'a> {
pub rust: Set<&'a Ident>,
pub aliases: Map<&'a Ident, &'a TypeAlias>,
pub untrusted: Map<&'a Ident, &'a ExternType>,
pub required_trivial_aliases: Set<&'a Ident>,
}

impl<'a> Types<'a> {
Expand Down Expand Up @@ -135,6 +136,55 @@ impl<'a> Types<'a> {
}
}

// All these APIs may contain types passed by value. We need to ensure
// we check that this is permissible. We do this _after_ scanning all
// the APIs above, in case some function or struct references a type
// which is declared subsequently.
let mut required_trivial_aliases = Set::new();

fn insist_alias_types_are_trivial<'c>(
required_trivial_aliases: &mut Set<&'c Ident>,
aliases: &Map<&'c Ident, &'c TypeAlias>,
ty: &'c Type,
) {
if let Type::Ident(ident) = ty {
if aliases.contains_key(ident) {
required_trivial_aliases.insert(ident);
}
}
}

for api in apis {
match api {
Api::Struct(strct) => {
for field in &strct.fields {
insist_alias_types_are_trivial(
&mut required_trivial_aliases,
&aliases,
&field.ty,
);
}
}
Api::CxxFunction(efn) | Api::RustFunction(efn) => {
for arg in &efn.args {
insist_alias_types_are_trivial(
&mut required_trivial_aliases,
&aliases,
&arg.ty,
);
}
if let Some(ret) = &efn.ret {
insist_alias_types_are_trivial(
&mut required_trivial_aliases,
&aliases,
&ret,
);
}
}
_ => {}
}
}

Types {
all,
structs,
Expand All @@ -143,6 +193,7 @@ impl<'a> Types<'a> {
rust,
aliases,
untrusted,
required_trivial_aliases,
}
}

Expand Down
6 changes: 3 additions & 3 deletions tests/ui/by_value_not_supported.stderr
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
error: using C++ type by value is not supported
error: using non-trivial C++ type by value is not supported
--> $DIR/by_value_not_supported.rs:4:9
|
4 | c: C,
Expand All @@ -16,13 +16,13 @@ error: using C++ string by value is not supported
6 | s: CxxString,
| ^^^^^^^^^^^^

error: passing C++ type by value is not supported
error: passing non-trivial C++ type by value is not supported
--> $DIR/by_value_not_supported.rs:16:14
|
16 | fn f(c: C) -> C;
| ^^^^

error: returning C++ type by value is not supported
error: returning non-trivial C++ type by value is not supported
--> $DIR/by_value_not_supported.rs:16:23
|
16 | fn f(c: C) -> C;
Expand Down