From 905eb2e1f5307f64850503ca1f46101239dabf17 Mon Sep 17 00:00:00 2001 From: Joel Galenson Date: Mon, 4 May 2020 14:58:14 -0700 Subject: [PATCH 1/3] Support enums defined in C++ code. This allows listing an enum in an extern "C" block as well as in the shared block. In this case, we do not emit the C++ declaration of the enum but instead emit static assertions that it has the values that we expect. --- gen/src/write.rs | 40 ++++++++++++++++++++++++++++++++++++++-- macro/src/expand.rs | 13 ++++++++++++- syntax/types.rs | 4 +++- 3 files changed, 53 insertions(+), 4 deletions(-) diff --git a/gen/src/write.rs b/gen/src/write.rs index 8ce38138..2ea6ca85 100644 --- a/gen/src/write.rs +++ b/gen/src/write.rs @@ -5,7 +5,7 @@ use crate::syntax::namespace::Namespace; use crate::syntax::symbol::Symbol; use crate::syntax::{mangle, Api, Enum, ExternFn, ExternType, Signature, Struct, Type, Types, Var}; use proc_macro2::Ident; -use std::collections::HashMap; +use std::collections::{HashMap, HashSet}; pub(super) fn gen( namespace: &Namespace, @@ -46,6 +46,7 @@ pub(super) fn gen( } } + let mut cxx_types = HashSet::new(); let mut methods_for_type = HashMap::new(); for api in apis { if let Api::RustFunction(efn) = api { @@ -56,6 +57,9 @@ pub(super) fn gen( .push(efn); } } + if let Api::CxxType(enm) = api { + cxx_types.insert(&enm.ident); + } } for api in apis { @@ -66,7 +70,11 @@ pub(super) fn gen( } Api::Enum(enm) => { out.next_section(); - write_enum(out, enm); + if cxx_types.contains(&enm.ident) { + check_enum(out, enm); + } else { + write_enum(out, enm); + } } Api::RustType(ety) => { if let Some(methods) = methods_for_type.get(&ety.ident) { @@ -373,6 +381,34 @@ fn write_enum(out: &mut OutFile, enm: &Enum) { writeln!(out, "}};"); } +fn check_enum(out: &mut OutFile, enm: &Enum) { + let discriminants = enm + .variants + .iter() + .scan(None, |prev_discriminant, variant| { + let discriminant = variant + .discriminant + .unwrap_or_else(|| prev_discriminant.map_or(0, |n| n + 1)); + *prev_discriminant = Some(discriminant); + Some(discriminant) + }); + writeln!( + out, + "static_assert(sizeof({}) == sizeof(uint32_t));", + enm.ident + ); + enm.variants + .iter() + .zip(discriminants) + .for_each(|(variant, discriminant)| { + writeln!( + out, + "static_assert({}::{} == {});", + enm.ident, variant.ident, discriminant + ); + }); +} + fn write_exception_glue(out: &mut OutFile, apis: &[Api]) { let mut has_cxx_throws = false; for api in apis { diff --git a/macro/src/expand.rs b/macro/src/expand.rs index e67902b3..1a23960c 100644 --- a/macro/src/expand.rs +++ b/macro/src/expand.rs @@ -7,6 +7,7 @@ use crate::syntax::{ }; use proc_macro2::{Ident, Span, TokenStream}; use quote::{format_ident, quote, quote_spanned, ToTokens}; +use std::collections::HashSet; use syn::{parse_quote, Error, ItemMod, Result, Token}; pub fn bridge(namespace: &Namespace, mut ffi: ItemMod) -> Result { @@ -39,12 +40,22 @@ fn expand(namespace: &Namespace, ffi: ItemMod, apis: &[Api], types: &Types) -> T } } + let mut enums = HashSet::new(); + for api in apis { + if let Api::Enum(enm) = api { + enums.insert(&enm.ident); + } + } for api in apis { match api { Api::Include(_) | Api::RustType(_) => {} Api::Struct(strct) => expanded.extend(expand_struct(strct)), Api::Enum(enm) => expanded.extend(expand_enum(enm)), - Api::CxxType(ety) => expanded.extend(expand_cxx_type(ety)), + Api::CxxType(ety) => { + if !enums.contains(&ety.ident) { + expanded.extend(expand_cxx_type(ety)); + } + } Api::CxxFunction(efn) => { expanded.extend(expand_cxx_function_shim(namespace, efn, types)); } diff --git a/syntax/types.rs b/syntax/types.rs index 02249e2e..bfe7bf0e 100644 --- a/syntax/types.rs +++ b/syntax/types.rs @@ -69,7 +69,9 @@ impl<'a> Types<'a> { } Api::CxxType(ety) => { let ident = &ety.ident; - if !type_names.insert(ident) { + // We allow declaring the same type as a shared enum and as a Cxxtype, as this + // means not to emit the C++ enum definition. + if !type_names.insert(ident) && !enums.contains_key(ident) { duplicate_name(cx, ety, ident); } cxx.insert(ident); From 0f654ffeb0037fba4aadaa098e163434406a3fd5 Mon Sep 17 00:00:00 2001 From: Joel Galenson Date: Mon, 4 May 2020 20:04:21 -0700 Subject: [PATCH 2/3] Fix previous commit. --- gen/src/write.rs | 41 +++++++++++++++-------------------------- macro/src/expand.rs | 9 +-------- syntax/types.rs | 7 ++++--- tests/ffi/lib.rs | 9 +++++++++ tests/ffi/tests.h | 5 +++++ 5 files changed, 34 insertions(+), 37 deletions(-) diff --git a/gen/src/write.rs b/gen/src/write.rs index 2ea6ca85..70620ce7 100644 --- a/gen/src/write.rs +++ b/gen/src/write.rs @@ -5,7 +5,7 @@ use crate::syntax::namespace::Namespace; use crate::syntax::symbol::Symbol; use crate::syntax::{mangle, Api, Enum, ExternFn, ExternType, Signature, Struct, Type, Types, Var}; use proc_macro2::Ident; -use std::collections::{HashMap, HashSet}; +use std::collections::HashMap; pub(super) fn gen( namespace: &Namespace, @@ -46,7 +46,6 @@ pub(super) fn gen( } } - let mut cxx_types = HashSet::new(); let mut methods_for_type = HashMap::new(); for api in apis { if let Api::RustFunction(efn) = api { @@ -57,9 +56,6 @@ pub(super) fn gen( .push(efn); } } - if let Api::CxxType(enm) = api { - cxx_types.insert(&enm.ident); - } } for api in apis { @@ -70,7 +66,7 @@ pub(super) fn gen( } Api::Enum(enm) => { out.next_section(); - if cxx_types.contains(&enm.ident) { + if types.cxx.contains(&enm.ident) { check_enum(out, enm); } else { write_enum(out, enm); @@ -382,31 +378,24 @@ fn write_enum(out: &mut OutFile, enm: &Enum) { } fn check_enum(out: &mut OutFile, enm: &Enum) { - let discriminants = enm - .variants - .iter() - .scan(None, |prev_discriminant, variant| { - let discriminant = variant - .discriminant - .unwrap_or_else(|| prev_discriminant.map_or(0, |n| n + 1)); - *prev_discriminant = Some(discriminant); - Some(discriminant) - }); writeln!( out, "static_assert(sizeof({}) == sizeof(uint32_t));", enm.ident ); - enm.variants - .iter() - .zip(discriminants) - .for_each(|(variant, discriminant)| { - writeln!( - out, - "static_assert({}::{} == {});", - enm.ident, variant.ident, discriminant - ); - }); + let mut prev_discriminant = None; + for variant in &enm.variants { + let discriminant = variant + .discriminant + .unwrap_or_else(|| prev_discriminant.map_or(0, |n| n + 1)); + writeln!( + out, + "static_assert(static_cast({}::{}) == {}, + \"disagrees with the value in #[cxx::bridge]\");", + enm.ident, variant.ident, discriminant, + ); + prev_discriminant = Some(discriminant); + } } fn write_exception_glue(out: &mut OutFile, apis: &[Api]) { diff --git a/macro/src/expand.rs b/macro/src/expand.rs index 1a23960c..d8810dcb 100644 --- a/macro/src/expand.rs +++ b/macro/src/expand.rs @@ -7,7 +7,6 @@ use crate::syntax::{ }; use proc_macro2::{Ident, Span, TokenStream}; use quote::{format_ident, quote, quote_spanned, ToTokens}; -use std::collections::HashSet; use syn::{parse_quote, Error, ItemMod, Result, Token}; pub fn bridge(namespace: &Namespace, mut ffi: ItemMod) -> Result { @@ -40,19 +39,13 @@ fn expand(namespace: &Namespace, ffi: ItemMod, apis: &[Api], types: &Types) -> T } } - let mut enums = HashSet::new(); - for api in apis { - if let Api::Enum(enm) = api { - enums.insert(&enm.ident); - } - } for api in apis { match api { Api::Include(_) | Api::RustType(_) => {} Api::Struct(strct) => expanded.extend(expand_struct(strct)), Api::Enum(enm) => expanded.extend(expand_enum(enm)), Api::CxxType(ety) => { - if !enums.contains(&ety.ident) { + if !types.enums.contains_key(&ety.ident) { expanded.extend(expand_cxx_type(ety)); } } diff --git a/syntax/types.rs b/syntax/types.rs index bfe7bf0e..58005139 100644 --- a/syntax/types.rs +++ b/syntax/types.rs @@ -61,11 +61,12 @@ impl<'a> Types<'a> { } Api::Enum(enm) => { let ident = &enm.ident; - if type_names.insert(ident) { - enums.insert(ident.clone(), enm); - } else { + // We allow declaring the same type as a shared enum and as a Cxxtype, as this + // means not to emit the C++ enum definition. + if !type_names.insert(ident) && !cxx.contains(ident) { duplicate_name(cx, enm, ident); } + enums.insert(ident.clone(), enm); } Api::CxxType(ety) => { let ident = &ety.ident; diff --git a/tests/ffi/lib.rs b/tests/ffi/lib.rs index 0162c9b8..16846fbd 100644 --- a/tests/ffi/lib.rs +++ b/tests/ffi/lib.rs @@ -84,6 +84,15 @@ pub mod ffi { fn set2(&mut self, n: usize) -> usize; } + extern "C" { + type COwnedEnum; + } + + enum COwnedEnum { + CVal1, + CVal2, + } + extern "Rust" { type R; type R2; diff --git a/tests/ffi/tests.h b/tests/ffi/tests.h index d3b7d38d..795b4a9f 100644 --- a/tests/ffi/tests.h +++ b/tests/ffi/tests.h @@ -23,6 +23,11 @@ private: std::vector v; }; +enum COwnedEnum { + CVal1, + CVal2, +}; + size_t c_return_primitive(); Shared c_return_shared(); rust::Box c_return_box(); From 9fe5ea3ad742ab7d465adb16e4cc52febc939c61 Mon Sep 17 00:00:00 2001 From: Joel Galenson Date: Mon, 4 May 2020 20:25:15 -0700 Subject: [PATCH 3/3] Add missing message. --- gen/src/write.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/gen/src/write.rs b/gen/src/write.rs index 70620ce7..3c059a71 100644 --- a/gen/src/write.rs +++ b/gen/src/write.rs @@ -380,7 +380,7 @@ fn write_enum(out: &mut OutFile, enm: &Enum) { fn check_enum(out: &mut OutFile, enm: &Enum) { writeln!( out, - "static_assert(sizeof({}) == sizeof(uint32_t));", + "static_assert(sizeof({}) == sizeof(uint32_t), \"incorrect size\");", enm.ident ); let mut prev_discriminant = None;