diff --git a/gen/src/namespace_organizer.rs b/gen/src/namespace_organizer.rs index c54e784ef..4aacc8f84 100644 --- a/gen/src/namespace_organizer.rs +++ b/gen/src/namespace_organizer.rs @@ -2,39 +2,127 @@ use crate::syntax::Api; use proc_macro2::Ident; use std::collections::BTreeMap; -pub(crate) struct NamespaceEntries<'a> { - pub(crate) entries: Vec<&'a Api>, - pub(crate) children: BTreeMap<&'a Ident, NamespaceEntries<'a>>, +pub struct NamespaceEntries<'a> { + entries: Vec<&'a Api>, + children: BTreeMap<&'a Ident, NamespaceEntries<'a>>, } -pub(crate) fn sort_by_namespace(apis: &[Api]) -> NamespaceEntries { - let api_refs = apis.iter().collect::>(); - sort_by_inner_namespace(api_refs, 0) -} +impl<'a> NamespaceEntries<'a> { + pub fn new(apis: &'a [Api]) -> Self { + let api_refs = apis.iter().collect::>(); + Self::sort_by_inner_namespace(api_refs, 0) + } + + pub fn entries(&self) -> &[&'a Api] { + &self.entries + } -fn sort_by_inner_namespace(apis: Vec<&Api>, depth: usize) -> NamespaceEntries { - let mut root = NamespaceEntries { - entries: Vec::new(), - children: BTreeMap::new(), - }; - - let mut kids_by_child_ns = BTreeMap::new(); - for api in apis { - if let Some(ns) = api.get_namespace() { - let first_ns_elem = ns.iter().nth(depth); - if let Some(first_ns_elem) = first_ns_elem { - let list = kids_by_child_ns.entry(first_ns_elem).or_insert(Vec::new()); - list.push(api); - continue; + pub fn children(&self) -> impl Iterator { + self.children.iter() + } + + fn sort_by_inner_namespace(apis: Vec<&'a Api>, depth: usize) -> Self { + let mut root = NamespaceEntries { + entries: Vec::new(), + children: BTreeMap::new(), + }; + + let mut kids_by_child_ns = BTreeMap::new(); + for api in apis { + if let Some(ns) = api.get_namespace() { + let first_ns_elem = ns.iter().nth(depth); + if let Some(first_ns_elem) = first_ns_elem { + let list = kids_by_child_ns.entry(first_ns_elem).or_insert(Vec::new()); + list.push(api); + continue; + } } + root.entries.push(api); + } + + for (k, v) in kids_by_child_ns.into_iter() { + root.children + .insert(k, Self::sort_by_inner_namespace(v, depth + 1)); } - root.entries.push(api); + + root + } +} + +#[cfg(test)] +mod tests { + use super::NamespaceEntries; + use crate::syntax::namespace::Namespace; + use crate::syntax::{Api, Doc, ExternType, Pair}; + use proc_macro2::{Ident, Span}; + use syn::Token; + + #[test] + fn test_ns_entries_sort() { + let entries = vec![ + make_api(None, "C"), + make_api(None, "A"), + make_api(Some("G"), "E"), + make_api(Some("D"), "F"), + make_api(Some("G"), "H"), + make_api(Some("D::K"), "L"), + make_api(Some("D::K"), "M"), + make_api(None, "B"), + make_api(Some("D"), "I"), + make_api(Some("D"), "J"), + ]; + let ns = NamespaceEntries::new(&entries); + let root_entries = ns.entries(); + assert_eq!(root_entries.len(), 3); + assert_ident(root_entries[0], "C"); + assert_ident(root_entries[1], "A"); + assert_ident(root_entries[2], "B"); + let mut kids = ns.children(); + let (d_id, d_nse) = kids.next().unwrap(); + assert_eq!(d_id.to_string(), "D"); + let (g_id, g_nse) = kids.next().unwrap(); + assert_eq!(g_id.to_string(), "G"); + assert!(kids.next().is_none()); + let d_nse_entries = d_nse.entries(); + assert_eq!(d_nse_entries.len(), 3); + assert_ident(d_nse_entries[0], "F"); + assert_ident(d_nse_entries[1], "I"); + assert_ident(d_nse_entries[2], "J"); + let g_nse_entries = g_nse.entries(); + assert_eq!(g_nse_entries.len(), 2); + assert_ident(g_nse_entries[0], "E"); + assert_ident(g_nse_entries[1], "H"); + let mut g_kids = g_nse.children(); + assert!(g_kids.next().is_none()); + let mut d_kids = d_nse.children(); + let (k_id, k_nse) = d_kids.next().unwrap(); + assert_eq!(k_id.to_string(), "K"); + let k_nse_entries = k_nse.entries(); + assert_eq!(k_nse_entries.len(), 2); + assert_ident(k_nse_entries[0], "L"); + assert_ident(k_nse_entries[1], "M"); } - for (k, v) in kids_by_child_ns.into_iter() { - root.children - .insert(k, sort_by_inner_namespace(v, depth + 1)); + fn assert_ident(api: &Api, expected: &str) { + if let Api::CxxType(cxx_type) = api { + assert_eq!(cxx_type.ident.cxx.ident.to_string(), expected); + } else { + unreachable!() + } } - root + fn make_api(ns: Option<&str>, ident: &str) -> Api { + let ns = match ns { + Some(st) => Namespace::from_str(st), + None => Namespace::none(), + }; + let ident = Pair::new(ns, Ident::new(ident, Span::call_site())); + Api::CxxType(ExternType { + doc: Doc::new(), + type_token: Token![type](Span::call_site()), + ident, + semi_token: Token![;](Span::call_site()), + trusted: true, + }) + } } diff --git a/gen/src/write.rs b/gen/src/write.rs index 967d4ff53..b2d52bb60 100644 --- a/gen/src/write.rs +++ b/gen/src/write.rs @@ -1,4 +1,4 @@ -use crate::gen::namespace_organizer::{sort_by_namespace, NamespaceEntries}; +use crate::gen::namespace_organizer::NamespaceEntries; use crate::gen::out::OutFile; use crate::gen::{include, Opt}; use crate::syntax::atom::Atom::{self, *}; @@ -30,7 +30,7 @@ pub(super) fn gen(apis: &[Api], types: &Types, opt: &Opt, header: bool) -> OutFi out.next_section(); - let apis_by_namespace = sort_by_namespace(apis); + let apis_by_namespace = NamespaceEntries::new(apis); gen_namespace_contents(&apis_by_namespace, types, opt, header, out); @@ -51,10 +51,10 @@ fn gen_namespace_contents( header: bool, out: &mut OutFile, ) { - let apis = &ns_entries.entries; + let apis = ns_entries.entries(); out.next_section(); - for api in apis { + for api in apis.iter() { match api { Api::Struct(strct) => write_struct_decl(out, &strct.ident.cxx.ident), Api::CxxType(ety) => write_struct_using(out, &ety.ident.cxx), @@ -64,7 +64,7 @@ fn gen_namespace_contents( } let mut methods_for_type = HashMap::new(); - for api in apis { + for api in apis.iter() { if let Api::RustFunction(efn) = api { if let Some(receiver) = &efn.sig.receiver { methods_for_type @@ -134,7 +134,7 @@ fn gen_namespace_contents( out.next_section(); - for (child_ns, child_ns_entries) in &ns_entries.children { + for (child_ns, child_ns_entries) in ns_entries.children() { writeln!(out, "namespace {} {{", child_ns); gen_namespace_contents(&child_ns_entries, types, opt, header, out); writeln!(out, "}} // namespace {}", child_ns); diff --git a/syntax/check.rs b/syntax/check.rs index 08e0dfaba..ba488e1ad 100644 --- a/syntax/check.rs +++ b/syntax/check.rs @@ -211,9 +211,7 @@ fn check_api_type(cx: &mut Check, ety: &ExternType) { if let Some(reason) = cx.types.required_trivial.get(&ety.ident.rust) { let what = match reason { - TrivialReason::StructField(strct) => { - format!("a field of `{}`", strct.ident.cxx.to_fully_qualified()) - } + TrivialReason::StructField(strct) => format!("a field of `{}`", strct.ident.rust), TrivialReason::FunctionArgument(efn) => format!("an argument of `{}`", efn.ident.rust), TrivialReason::FunctionReturn(efn) => format!("a return value of `{}`", efn.ident.rust), }; diff --git a/syntax/impls.rs b/syntax/impls.rs index 424ad9d29..f4c5b05e4 100644 --- a/syntax/impls.rs +++ b/syntax/impls.rs @@ -300,14 +300,22 @@ impl Borrow for &Impl { impl Pair { /// Use this constructor when the item can't have a different - /// name in Rust and C++. For cases where #[rust_name] and similar - /// attributes can be used, construct the object by hand. + /// name in Rust and C++. pub fn new(ns: Namespace, ident: Ident) -> Self { Self { rust: ident.clone(), cxx: CppName::new(ns, ident), } } + + /// Use this constructor when attributes such as #[rust_name] + /// can be used to potentially give a different name in Rust vs C++. + pub fn new_from_differing_names(ns: Namespace, cxx_ident: Ident, rust_ident: Ident) -> Self { + Self { + rust: rust_ident, + cxx: CppName::new(ns, cxx_ident), + } + } } impl ResolvableName { @@ -315,10 +323,6 @@ impl ResolvableName { Self { rust: ident } } - pub fn from_pair(pair: Pair) -> Self { - Self { rust: pair.rust } - } - pub fn make_self(span: Span) -> Self { Self { rust: Token![Self](span).into(), @@ -357,9 +361,7 @@ impl CppName { Self { ns, ident } } - fn iter_all_segments( - &self, - ) -> std::iter::Chain, std::iter::Once<&Ident>> { + fn iter_all_segments(&self) -> impl Iterator { self.ns.iter().chain(std::iter::once(&self.ident)) } diff --git a/syntax/namespace.rs b/syntax/namespace.rs index b4c6716d5..5eb203ab6 100644 --- a/syntax/namespace.rs +++ b/syntax/namespace.rs @@ -1,4 +1,6 @@ use crate::syntax::qualified::QualifiedName; +#[cfg(test)] +use proc_macro2::Span; use quote::IdentFragment; use std::fmt::{self, Display}; use std::slice::Iter; @@ -36,6 +38,16 @@ impl Namespace { input.parse::>()?; Ok(ns) } + + #[cfg(test)] + pub fn from_str(ns: &str) -> Self { + Namespace { + segments: ns + .split("::") + .map(|x| Ident::new(x, Span::call_site())) + .collect(), + } + } } impl Parse for Namespace { diff --git a/syntax/parse.rs b/syntax/parse.rs index 7a56ab81e..19275547b 100644 --- a/syntax/parse.rs +++ b/syntax/parse.rs @@ -3,7 +3,7 @@ use crate::syntax::file::{Item, ItemForeignMod}; use crate::syntax::report::Errors; use crate::syntax::Atom::*; use crate::syntax::{ - attrs, error, Api, CppName, Doc, Enum, ExternFn, ExternType, Impl, Include, IncludeKind, Lang, + attrs, error, Api, Doc, Enum, ExternFn, ExternType, Impl, Include, IncludeKind, Lang, Namespace, Pair, Receiver, Ref, ResolvableName, Signature, Slice, Struct, Ty1, Type, TypeAlias, Var, Variant, }; @@ -245,7 +245,7 @@ fn parse_foreign_mod( if let Api::CxxFunction(efn) | Api::RustFunction(efn) = item { if let Some(receiver) = &mut efn.receiver { if receiver.ty.is_self() { - receiver.ty = ResolvableName::from_pair(single_type.clone()); + receiver.ty = ResolvableName::new(single_type.rust.clone()); } } } @@ -398,10 +398,11 @@ fn parse_extern_fn( let throws = throws_tokens.is_some(); let unsafety = foreign_fn.sig.unsafety; let fn_token = foreign_fn.sig.fn_token; - let ident = Pair { - cxx: CppName::new(ns, cxx_name.unwrap_or(foreign_fn.sig.ident.clone())), - rust: rust_name.unwrap_or(foreign_fn.sig.ident.clone()), - }; + let ident = Pair::new_from_differing_names( + ns, + cxx_name.unwrap_or(foreign_fn.sig.ident.clone()), + rust_name.unwrap_or(foreign_fn.sig.ident.clone()), + ); let paren_token = foreign_fn.sig.paren_token; let semi_token = foreign_fn.semi_token; let api_function = match lang { diff --git a/tests/BUCK b/tests/BUCK index f51be097a..5bbe50099 100644 --- a/tests/BUCK +++ b/tests/BUCK @@ -15,7 +15,6 @@ rust_library( "ffi/extra.rs", "ffi/lib.rs", "ffi/module.rs", - "ffi/class_in_ns.rs", ], crate = "cxx_test_suite", deps = [ @@ -31,7 +30,6 @@ cxx_library( ":bridge/source", ":extra/source", ":module/source", - ":class_in_ns/source", ], headers = { "ffi/lib.rs.h": ":bridge/header", @@ -54,8 +52,3 @@ rust_cxx_bridge( name = "module", src = "ffi/module.rs", ) - -rust_cxx_bridge( - name = "class_in_ns", - src = "ffi/class_in_ns.rs", -) diff --git a/tests/BUILD b/tests/BUILD index a400f4768..57ffab996 100644 --- a/tests/BUILD +++ b/tests/BUILD @@ -18,7 +18,6 @@ rust_library( "ffi/extra.rs", "ffi/lib.rs", "ffi/module.rs", - "ffi/class_in_ns.rs", ], deps = [ ":impl", @@ -33,7 +32,6 @@ cc_library( ":bridge/source", ":extra/source", ":module/source", - ":class_in_ns/source", ], hdrs = ["ffi/tests.h"], deps = [ @@ -59,9 +57,3 @@ rust_cxx_bridge( src = "ffi/module.rs", deps = [":impl"], ) - -rust_cxx_bridge( - name = "class_in_ns", - src = "ffi/class_in_ns.rs", - deps = [":impl"], -) \ No newline at end of file diff --git a/tests/ffi/build.rs b/tests/ffi/build.rs index 9bdb711e5..4b2cbdfe9 100644 --- a/tests/ffi/build.rs +++ b/tests/ffi/build.rs @@ -6,7 +6,7 @@ fn main() { } CFG.include_prefix = "tests/ffi"; - let sources = vec!["lib.rs", "extra.rs", "module.rs", "class_in_ns.rs"]; + let sources = vec!["lib.rs", "extra.rs", "module.rs"]; cxx_build::bridges(sources) .file("tests.cc") .flag_if_supported(cxxbridge_flags::STD) diff --git a/tests/ffi/class_in_ns.rs b/tests/ffi/class_in_ns.rs deleted file mode 100644 index a03da78ce..000000000 --- a/tests/ffi/class_in_ns.rs +++ /dev/null @@ -1,21 +0,0 @@ -// To test receivers on a type in a namespace outide -// the default. cxx::bridge blocks can only have a single -// receiver type, and there can only be one such block per, -// which is why this is outside. - -#[rustfmt::skip] -#[cxx::bridge(namespace = tests)] -pub mod ffi3 { - - extern "C" { - include!("tests/ffi/tests.h"); - - #[namespace = "I"] - type I; - - fn get(self: &I) -> u32; - - #[namespace = "I"] - fn ns_c_return_unique_ptr_ns() -> UniquePtr; - } -} diff --git a/tests/ffi/extra.rs b/tests/ffi/extra.rs index 58700a2f4..a11970df2 100644 --- a/tests/ffi/extra.rs +++ b/tests/ffi/extra.rs @@ -51,5 +51,13 @@ pub mod ffi2 { fn ns_c_take_trivial(d: D); #[namespace = "other"] fn ns_c_return_trivial() -> D; + + #[namespace = "I"] + type I; + + fn get(self: &I) -> u32; + + #[namespace = "I"] + fn ns_c_return_unique_ptr_ns() -> UniquePtr; } } diff --git a/tests/ffi/lib.rs b/tests/ffi/lib.rs index 17146ce7f..fee5bcb11 100644 --- a/tests/ffi/lib.rs +++ b/tests/ffi/lib.rs @@ -4,7 +4,6 @@ clippy::trivially_copy_pass_by_ref )] -pub mod class_in_ns; pub mod extra; pub mod module; diff --git a/tests/test.rs b/tests/test.rs index 20b23acef..b651feb3f 100644 --- a/tests/test.rs +++ b/tests/test.rs @@ -1,4 +1,3 @@ -use cxx_test_suite::class_in_ns::ffi3; use cxx_test_suite::extra::ffi2; use cxx_test_suite::ffi; use std::cell::Cell; @@ -196,7 +195,7 @@ fn test_c_method_calls() { #[test] fn test_c_ns_method_calls() { - let unique_ptr = ffi3::ns_c_return_unique_ptr_ns(); + let unique_ptr = ffi2::ns_c_return_unique_ptr_ns(); let old_value = unique_ptr.get(); assert_eq!(1000, old_value); diff --git a/tests/ui/by_value_not_supported.stderr b/tests/ui/by_value_not_supported.stderr index a860f3ddb..0a56dd48f 100644 --- a/tests/ui/by_value_not_supported.stderr +++ b/tests/ui/by_value_not_supported.stderr @@ -16,7 +16,7 @@ error: using C++ string by value is not supported 6 | s: CxxString, | ^^^^^^^^^^^^ -error: needs a cxx::ExternType impl in order to be used as a field of `::S` +error: needs a cxx::ExternType impl in order to be used as a field of `S` --> $DIR/by_value_not_supported.rs:10:9 | 10 | type C;