From b1a00558f2fb44d0846735e5d1b166d5f9ee7a68 Mon Sep 17 00:00:00 2001 From: Bryan Henry Date: Sun, 13 Sep 2020 10:34:31 -0700 Subject: [PATCH 01/11] Preserve docs on aliases --- macro/src/expand.rs | 2 ++ syntax/mod.rs | 1 + syntax/parse.rs | 3 ++- 3 files changed, 5 insertions(+), 1 deletion(-) diff --git a/macro/src/expand.rs b/macro/src/expand.rs index 879431e03..f38208838 100644 --- a/macro/src/expand.rs +++ b/macro/src/expand.rs @@ -657,9 +657,11 @@ fn expand_rust_function_shim_impl( } fn expand_type_alias(alias: &TypeAlias) -> TokenStream { + let doc = &alias.doc; let ident = &alias.ident; let ty = &alias.ty; quote! { + #doc pub type #ident = #ty; } } diff --git a/syntax/mod.rs b/syntax/mod.rs index f52a582c1..8ef3c8b9c 100644 --- a/syntax/mod.rs +++ b/syntax/mod.rs @@ -79,6 +79,7 @@ pub struct ExternFn { } pub struct TypeAlias { + pub doc: Doc, pub type_token: Token![type], pub ident: Ident, pub eq_token: Token![=], diff --git a/syntax/parse.rs b/syntax/parse.rs index 3ac530471..c1c565fd4 100644 --- a/syntax/parse.rs +++ b/syntax/parse.rs @@ -396,9 +396,10 @@ fn parse_extern_verbatim(cx: &mut Errors, tokens: &TokenStream, lang: Lang) -> R let eq_token: Token![=] = input.parse()?; let ty: RustType = input.parse()?; let semi_token: Token![;] = input.parse()?; - attrs::parse_doc(cx, &attrs); + let doc = attrs::parse_doc(cx, &attrs); Ok(TypeAlias { + doc, type_token, ident, eq_token, From 8446a2ac4c97b2eb899d846f1369001ac2c7516d Mon Sep 17 00:00:00 2001 From: Bryan Henry Date: Sun, 13 Sep 2020 12:43:18 -0700 Subject: [PATCH 02/11] Rename tests/ffi/module.rs to alias.rs to clarify purpose --- tests/BUCK | 10 +++++----- tests/BUILD | 10 +++++----- tests/ffi/{module.rs => alias.rs} | 0 tests/ffi/build.rs | 2 +- tests/ffi/lib.rs | 2 +- tests/test.rs | 4 ++-- 6 files changed, 14 insertions(+), 14 deletions(-) rename tests/ffi/{module.rs => alias.rs} (100%) diff --git a/tests/BUCK b/tests/BUCK index a96bfb8d0..9ccf941c4 100644 --- a/tests/BUCK +++ b/tests/BUCK @@ -7,8 +7,8 @@ rust_test( rust_library( name = "ffi", srcs = [ + "ffi/alias.rs", "ffi/lib.rs", - "ffi/module.rs", ], crate = "cxx_test_suite", deps = [ @@ -21,8 +21,8 @@ cxx_library( name = "impl", srcs = [ "ffi/tests.cc", + ":gen-alias-source", ":gen-lib-source", - ":gen-module-source", ], header_namespace = "cxx-test-suite", headers = { @@ -47,8 +47,8 @@ genrule( ) genrule( - name = "gen-module-source", - srcs = ["ffi/module.rs"], - out = "module.rs.cc", + name = "gen-alias-source", + srcs = ["ffi/alias.rs"], + out = "alias.rs.cc", cmd = "$(exe //:codegen) ${SRCS} > ${OUT}", ) diff --git a/tests/BUILD b/tests/BUILD index 29cc79ce5..e42985c76 100644 --- a/tests/BUILD +++ b/tests/BUILD @@ -9,8 +9,8 @@ rust_test( rust_library( name = "cxx_test_suite", srcs = [ + "ffi/alias.rs", "ffi/lib.rs", - "ffi/module.rs", ], deps = [ ":impl", @@ -22,8 +22,8 @@ cc_library( name = "impl", srcs = [ "ffi/tests.cc", + ":gen-alias-source", ":gen-lib-source", - ":gen-module-source", ], hdrs = ["ffi/tests.h"], include_prefix = "cxx-test-suite", @@ -57,9 +57,9 @@ cc_library( ) genrule( - name = "gen-module-source", - srcs = ["ffi/module.rs"], - outs = ["module.rs.cc"], + name = "gen-alias-source", + srcs = ["ffi/alias.rs"], + outs = ["alias.rs.cc"], cmd = "$(location //:codegen) $< > $@", tools = ["//:codegen"], ) diff --git a/tests/ffi/module.rs b/tests/ffi/alias.rs similarity index 100% rename from tests/ffi/module.rs rename to tests/ffi/alias.rs diff --git a/tests/ffi/build.rs b/tests/ffi/build.rs index 8042129fa..10078531f 100644 --- a/tests/ffi/build.rs +++ b/tests/ffi/build.rs @@ -3,7 +3,7 @@ fn main() { return; } - let sources = vec!["lib.rs", "module.rs"]; + let sources = vec!["alias.rs", "lib.rs"]; cxx_build::bridges(sources) .file("tests.cc") .flag_if_supported(cxxbridge_flags::STD) diff --git a/tests/ffi/lib.rs b/tests/ffi/lib.rs index d6724f569..e9fde7151 100644 --- a/tests/ffi/lib.rs +++ b/tests/ffi/lib.rs @@ -4,7 +4,7 @@ clippy::trivially_copy_pass_by_ref )] -pub mod module; +pub mod alias; use cxx::{CxxString, UniquePtr}; use std::fmt::{self, Display}; diff --git a/tests/test.rs b/tests/test.rs index 9c71bd5aa..f866c1e22 100644 --- a/tests/test.rs +++ b/tests/test.rs @@ -1,4 +1,4 @@ -use cxx_test_suite::ffi; +use cxx_test_suite::{alias, ffi}; use std::cell::Cell; use std::ffi::CStr; @@ -89,7 +89,7 @@ fn test_c_take() { check!(ffi::c_take_shared(ffi::Shared { z: 2020 })); check!(ffi::c_take_box(Box::new(2020))); check!(ffi::c_take_ref_c(&unique_ptr)); - check!(cxx_test_suite::module::ffi::c_take_unique_ptr(unique_ptr)); + check!(alias::ffi::c_take_unique_ptr(unique_ptr)); check!(ffi::c_take_str("2020")); check!(ffi::c_take_sliceu8(b"2020")); check!(ffi::c_take_rust_string("2020".to_owned())); From de3702b9cef237d98a444456a012ff7335836c7f Mon Sep 17 00:00:00 2001 From: Bryan Henry Date: Sun, 13 Sep 2020 13:36:36 -0700 Subject: [PATCH 03/11] Support type aliases in different namespaces --- gen/src/write.rs | 12 +++++++++++- macro/src/expand.rs | 9 ++------- syntax/attrs.rs | 20 +++++++++++++++++++- syntax/mod.rs | 6 ++++-- syntax/namespace.rs | 22 +++++++++++++++++----- syntax/parse.rs | 16 ++++++++++++++-- tests/ffi/alias.rs | 6 +++++- tests/ffi/lib.rs | 1 + tests/ffi/tests.h | 9 +++++++++ tests/test.rs | 8 +++++++- 10 files changed, 89 insertions(+), 20 deletions(-) diff --git a/gen/src/write.rs b/gen/src/write.rs index 8cc1b1b55..4ea94819a 100644 --- a/gen/src/write.rs +++ b/gen/src/write.rs @@ -3,7 +3,9 @@ use crate::gen::{include, Opt}; use crate::syntax::atom::Atom::{self, *}; use crate::syntax::namespace::Namespace; use crate::syntax::symbol::Symbol; -use crate::syntax::{mangle, Api, Enum, ExternFn, ExternType, Signature, Struct, Type, Types, Var}; +use crate::syntax::{ + mangle, Api, Enum, ExternFn, ExternType, Signature, Struct, Type, TypeAlias, Types, Var, +}; use proc_macro2::Ident; use std::collections::HashMap; @@ -42,6 +44,7 @@ pub(super) fn gen( Api::Struct(strct) => write_struct_decl(out, &strct.ident), Api::CxxType(ety) => write_struct_using(out, &ety.ident), Api::RustType(ety) => write_struct_decl(out, &ety.ident), + Api::TypeAlias(alias) => write_alias(out, alias), _ => {} } } @@ -931,6 +934,13 @@ fn write_type(out: &mut OutFile, ty: &Type) { } } +fn write_alias(out: &mut OutFile, alias: &TypeAlias) { + if let Some(namespace) = &alias.namespace { + let path = namespace.path_for_type(&alias.ident); + writeln!(out, "using {} = {};", alias.ident, path) + } +} + fn write_atom(out: &mut OutFile, atom: Atom) { match atom { Bool => write!(out, "bool"), diff --git a/macro/src/expand.rs b/macro/src/expand.rs index f38208838..42c0c2c72 100644 --- a/macro/src/expand.rs +++ b/macro/src/expand.rs @@ -667,6 +667,7 @@ fn expand_type_alias(alias: &TypeAlias) -> TokenStream { } fn expand_type_alias_verify(namespace: &Namespace, alias: &TypeAlias) -> TokenStream { + let namespace = alias.namespace.as_ref().unwrap_or(namespace); let ident = &alias.ident; let type_id = type_id(namespace, ident); let begin_span = alias.type_token.span; @@ -680,13 +681,7 @@ fn expand_type_alias_verify(namespace: &Namespace, alias: &TypeAlias) -> TokenSt } fn type_id(namespace: &Namespace, ident: &Ident) -> TokenStream { - let mut path = String::new(); - for name in namespace { - path += &name.to_string(); - path += "::"; - } - path += &ident.to_string(); - + let path = namespace.path_for_type(ident); quote! { ::cxx::type_id!(#path) } diff --git a/syntax/attrs.rs b/syntax/attrs.rs index 4c76641cc..ccbbc7cc5 100644 --- a/syntax/attrs.rs +++ b/syntax/attrs.rs @@ -1,6 +1,7 @@ +use crate::syntax::qualified::QualifiedName; use crate::syntax::report::Errors; use crate::syntax::Atom::{self, *}; -use crate::syntax::{Derive, Doc}; +use crate::syntax::{Derive, Doc, Namespace}; use proc_macro2::Ident; use syn::parse::{ParseStream, Parser as _}; use syn::{Attribute, Error, LitStr, Path, Result, Token}; @@ -10,6 +11,7 @@ pub struct Parser<'a> { pub doc: Option<&'a mut Doc>, pub derives: Option<&'a mut Vec>, pub repr: Option<&'a mut Option>, + pub namespace: Option<&'a mut Option>, } pub(super) fn parse_doc(cx: &mut Errors, attrs: &[Attribute]) -> Doc { @@ -57,6 +59,16 @@ pub(super) fn parse(cx: &mut Errors, attrs: &[Attribute], mut parser: Parser) { } Err(err) => return cx.push(err), } + } else if attr.path.is_ident("namespace") { + match parse_namespace_attribute.parse2(attr.tokens.clone()) { + Ok(namespace) => { + if let Some(ns) = &mut parser.namespace { + **ns = Some(Namespace::from(namespace)); + continue; + } + } + Err(err) => return cx.push(err), + } } return cx.error(attr, "unsupported attribute"); } @@ -99,3 +111,9 @@ fn parse_repr_attribute(input: ParseStream) -> Result { "unrecognized repr", )) } + +fn parse_namespace_attribute(input: ParseStream) -> Result { + input.parse::()?; + let name = input.call(QualifiedName::parse_quoted_or_unquoted)?; + Ok(Namespace::from(name)) +} diff --git a/syntax/mod.rs b/syntax/mod.rs index 8ef3c8b9c..86c49be54 100644 --- a/syntax/mod.rs +++ b/syntax/mod.rs @@ -25,11 +25,12 @@ use self::parse::kw; use proc_macro2::{Ident, Span}; use syn::punctuated::Punctuated; use syn::token::{Brace, Bracket, Paren}; -use syn::{Expr, Lifetime, Token, Type as RustType}; +use syn::{Expr, Lifetime, Token, TypePath}; pub use self::atom::Atom; pub use self::derive::Derive; pub use self::doc::Doc; +pub use self::namespace::Namespace; pub use self::parse::parse_items; pub use self::types::Types; @@ -80,10 +81,11 @@ pub struct ExternFn { pub struct TypeAlias { pub doc: Doc, + pub namespace: Option, pub type_token: Token![type], pub ident: Ident, pub eq_token: Token![=], - pub ty: RustType, + pub ty: TypePath, pub semi_token: Token![;], } diff --git a/syntax/namespace.rs b/syntax/namespace.rs index 49b31d14f..2d8fe9106 100644 --- a/syntax/namespace.rs +++ b/syntax/namespace.rs @@ -24,20 +24,32 @@ impl Namespace { pub fn iter(&self) -> Iter { self.segments.iter() } + + pub fn path_for_type(&self, ident: &Ident) -> String { + let mut segments = self.iter().map(ToString::to_string).collect::>(); + segments.push(ident.to_string()); + segments.join("::") + } +} + +impl From for Namespace { + fn from(value: QualifiedName) -> Namespace { + Namespace { + segments: value.segments, + } + } } impl Parse for Namespace { fn parse(input: ParseStream) -> Result { - let mut segments = Vec::new(); if !input.is_empty() { input.parse::()?; input.parse::()?; - segments = input - .call(QualifiedName::parse_quoted_or_unquoted)? - .segments; + let name = input.call(QualifiedName::parse_quoted_or_unquoted)?; input.parse::>()?; + return Ok(Namespace::from(name)); } - Ok(Namespace { segments }) + Ok(Namespace::none()) } } diff --git a/syntax/parse.rs b/syntax/parse.rs index c1c565fd4..e38e92f2e 100644 --- a/syntax/parse.rs +++ b/syntax/parse.rs @@ -394,12 +394,24 @@ fn parse_extern_verbatim(cx: &mut Errors, tokens: &TokenStream, lang: Lang) -> R }; let ident: Ident = input.parse()?; let eq_token: Token![=] = input.parse()?; - let ty: RustType = input.parse()?; + let ty: TypePath = input.parse()?; let semi_token: Token![;] = input.parse()?; - let doc = attrs::parse_doc(cx, &attrs); + + let mut doc = Doc::new(); + let mut namespace = None; + attrs::parse( + cx, + &attrs, + attrs::Parser { + doc: Some(&mut doc), + namespace: Some(&mut namespace), + ..Default::default() + }, + ); Ok(TypeAlias { doc, + namespace, type_token, ident, eq_token, diff --git a/tests/ffi/alias.rs b/tests/ffi/alias.rs index 8862dc161..19132e5b8 100644 --- a/tests/ffi/alias.rs +++ b/tests/ffi/alias.rs @@ -1,13 +1,17 @@ // Rustfmt mangles the extern type alias. // https://github.com/rust-lang/rustfmt/issues/4159 #[rustfmt::skip] -#[cxx::bridge(namespace = tests)] +#[cxx::bridge(namespace = alias_tests)] pub mod ffi { extern "C" { include!("cxx-test-suite/tests.h"); + // Review TODO: Unquoted namespace here doesn't work, is that expected or a bug + // in my parsing? + #[namespace = "tests"] type C = crate::ffi::C; + fn c_return_unique_ptr() -> UniquePtr; fn c_take_unique_ptr(c: UniquePtr); } } diff --git a/tests/ffi/lib.rs b/tests/ffi/lib.rs index e9fde7151..03ca4f34e 100644 --- a/tests/ffi/lib.rs +++ b/tests/ffi/lib.rs @@ -60,6 +60,7 @@ pub mod ffi { fn c_take_str(s: &str); fn c_take_sliceu8(s: &[u8]); fn c_take_rust_string(s: String); + fn c_take_unique_ptr(c: UniquePtr); fn c_take_unique_ptr_string(s: UniquePtr); fn c_take_unique_ptr_vector_u8(v: UniquePtr>); fn c_take_unique_ptr_vector_f64(v: UniquePtr>); diff --git a/tests/ffi/tests.h b/tests/ffi/tests.h index f3bc2dd5f..2af2b0fc1 100644 --- a/tests/ffi/tests.h +++ b/tests/ffi/tests.h @@ -102,3 +102,12 @@ rust::Vec c_try_return_rust_vec_string(); const rust::Vec &c_try_return_ref_rust_vec(const C &c); } // namespace tests + +namespace alias_tests { + +// These aliases on the C++ side aren't under test, there's just no reason to +// duplicate these functions +using tests::c_return_unique_ptr; +using tests::c_take_unique_ptr; + +} // namespace alias_tests diff --git a/tests/test.rs b/tests/test.rs index f866c1e22..b7ce86e93 100644 --- a/tests/test.rs +++ b/tests/test.rs @@ -89,7 +89,7 @@ fn test_c_take() { check!(ffi::c_take_shared(ffi::Shared { z: 2020 })); check!(ffi::c_take_box(Box::new(2020))); check!(ffi::c_take_ref_c(&unique_ptr)); - check!(alias::ffi::c_take_unique_ptr(unique_ptr)); + check!(ffi::c_take_unique_ptr(unique_ptr)); check!(ffi::c_take_str("2020")); check!(ffi::c_take_sliceu8(b"2020")); check!(ffi::c_take_rust_string("2020".to_owned())); @@ -172,6 +172,12 @@ fn test_enum_representations() { assert_eq!(2021, ffi::Enum::CVal.repr); } +#[test] +fn test_alias() { + let unique_ptr = alias::ffi::c_return_unique_ptr(); + check!(alias::ffi::c_take_unique_ptr(unique_ptr)); +} + #[no_mangle] extern "C" fn cxx_test_suite_get_box() -> *mut cxx_test_suite::R { Box::into_raw(Box::new(2020usize)) From 7a8c2581149d4eb960593e74b75e70f9bd6ecf97 Mon Sep 17 00:00:00 2001 From: Bryan Henry Date: Sun, 13 Sep 2020 13:38:57 -0700 Subject: [PATCH 04/11] Support local aliased type name differing from remote name --- gen/src/write.rs | 5 ++++- macro/src/expand.rs | 5 ++++- tests/ffi/alias.rs | 5 ++++- tests/ui/wrong_type_id.rs | 7 +++++-- tests/ui/wrong_type_id.stderr | 12 ++++++------ 5 files changed, 23 insertions(+), 11 deletions(-) diff --git a/gen/src/write.rs b/gen/src/write.rs index 4ea94819a..064729c6b 100644 --- a/gen/src/write.rs +++ b/gen/src/write.rs @@ -936,7 +936,10 @@ fn write_type(out: &mut OutFile, ty: &Type) { fn write_alias(out: &mut OutFile, alias: &TypeAlias) { if let Some(namespace) = &alias.namespace { - let path = namespace.path_for_type(&alias.ident); + // Review TODO: Is this unwrap fine? i.e. is it ok to assume that, if + // the TypePath parsed, that it has at least one segment? + let remote_type = &alias.ty.path.segments.last().unwrap().ident; + let path = namespace.path_for_type(remote_type); writeln!(out, "using {} = {};", alias.ident, path) } } diff --git a/macro/src/expand.rs b/macro/src/expand.rs index 42c0c2c72..4593777b0 100644 --- a/macro/src/expand.rs +++ b/macro/src/expand.rs @@ -668,8 +668,11 @@ fn expand_type_alias(alias: &TypeAlias) -> TokenStream { fn expand_type_alias_verify(namespace: &Namespace, alias: &TypeAlias) -> TokenStream { let namespace = alias.namespace.as_ref().unwrap_or(namespace); + // Review TODO: Is this unwrap fine? i.e. is it ok to assume that, if the + // TypePath parsed, that it has at least one segment? + let remote_type = &alias.ty.path.segments.last().unwrap().ident; + let type_id = type_id(namespace, remote_type); let ident = &alias.ident; - let type_id = type_id(namespace, ident); let begin_span = alias.type_token.span; let end_span = alias.semi_token.span; let begin = quote_spanned!(begin_span=> ::cxx::private::verify_extern_type::<); diff --git a/tests/ffi/alias.rs b/tests/ffi/alias.rs index 19132e5b8..241f1fe41 100644 --- a/tests/ffi/alias.rs +++ b/tests/ffi/alias.rs @@ -11,7 +11,10 @@ pub mod ffi { #[namespace = "tests"] type C = crate::ffi::C; + #[namespace = "tests"] + type SameC = crate::ffi::C; + fn c_return_unique_ptr() -> UniquePtr; - fn c_take_unique_ptr(c: UniquePtr); + fn c_take_unique_ptr(c: UniquePtr); } } diff --git a/tests/ui/wrong_type_id.rs b/tests/ui/wrong_type_id.rs index 81a9b3f87..f87b7c3fb 100644 --- a/tests/ui/wrong_type_id.rs +++ b/tests/ui/wrong_type_id.rs @@ -1,14 +1,17 @@ -#[cxx::bridge(namespace = folly)] +#[cxx::bridge(namespace = correct)] mod here { extern "C" { type StringPiece; } } +// Rustfmt mangles the extern type alias. +// https://github.com/rust-lang/rustfmt/issues/4159 +#[rustfmt::skip] #[cxx::bridge(namespace = folly)] mod there { extern "C" { - type ByteRange = crate::here::StringPiece; + type OtherName = crate::here::StringPiece; } } diff --git a/tests/ui/wrong_type_id.stderr b/tests/ui/wrong_type_id.stderr index 2d8e50aac..0d98bbc82 100644 --- a/tests/ui/wrong_type_id.stderr +++ b/tests/ui/wrong_type_id.stderr @@ -1,13 +1,13 @@ -error[E0271]: type mismatch resolving `::Id == (f, o, l, l, y, (), B, y, t, e, R, a, n, g, e)` - --> $DIR/wrong_type_id.rs:11:9 +error[E0271]: type mismatch resolving `::Id == (f, o, l, l, y, (), S, t, r, i, n, g, P, i, e, c, e)` + --> $DIR/wrong_type_id.rs:11:1 | -11 | type ByteRange = crate::here::StringPiece; - | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ expected a tuple with 15 elements, found one with 17 elements +11 | #[cxx::bridge(namespace = folly)] + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ expected a tuple with 17 elements, found one with 19 elements | ::: $WORKSPACE/src/extern_type.rs | | pub fn verify_extern_type, Id>() {} | ------- required by this bound in `verify_extern_type` | - = note: expected tuple `(f, o, l, l, y, (), B, y, t, e, R, a, n, g, e)` - found tuple `(f, o, l, l, y, (), S, t, r, i, n, g, P, i, e, c, e)` + = note: expected tuple `(f, o, l, l, y, (), S, t, r, i, n, g, P, i, e, c, e)` + found tuple `(c, o, r, r, e, c, t, (), S, t, r, i, n, g, P, i, e, c, e)` From 7b089df883b86ce8bdc91fe6af42c3327e7e0204 Mon Sep 17 00:00:00 2001 From: Bryan Henry Date: Sun, 13 Sep 2020 22:30:21 -0700 Subject: [PATCH 05/11] Cleanup Namespace parsing --- gen/src/file.rs | 2 +- macro/src/lib.rs | 7 +++++-- syntax/attrs.rs | 4 +--- syntax/namespace.rs | 24 +++++++++++------------- 4 files changed, 18 insertions(+), 19 deletions(-) diff --git a/gen/src/file.rs b/gen/src/file.rs index c69640711..1b324cb68 100644 --- a/gen/src/file.rs +++ b/gen/src/file.rs @@ -67,6 +67,6 @@ fn parse_args(attr: &Attribute) -> Result { if attr.tokens.is_empty() { Ok(Namespace::none()) } else { - attr.parse_args() + attr.parse_args_with(Namespace::parse_bridge_attr_namespace) } } diff --git a/macro/src/lib.rs b/macro/src/lib.rs index fae874a6c..a32833000 100644 --- a/macro/src/lib.rs +++ b/macro/src/lib.rs @@ -17,7 +17,7 @@ use crate::syntax::file::Module; use crate::syntax::namespace::Namespace; use crate::syntax::qualified::QualifiedName; use proc_macro::TokenStream; -use syn::parse::{Parse, ParseStream, Result}; +use syn::parse::{Parse, ParseStream, Parser, Result}; use syn::parse_macro_input; /// `#[cxx::bridge] mod ffi { ... }` @@ -41,7 +41,10 @@ use syn::parse_macro_input; pub fn bridge(args: TokenStream, input: TokenStream) -> TokenStream { let _ = syntax::error::ERRORS; - let namespace = parse_macro_input!(args as Namespace); + let namespace = match Namespace::parse_bridge_attr_namespace.parse(args) { + Ok(ns) => ns, + Err(err) => return err.to_compile_error().into(), + }; let mut ffi = parse_macro_input!(input as Module); ffi.namespace = namespace; diff --git a/syntax/attrs.rs b/syntax/attrs.rs index ccbbc7cc5..0e9733ea0 100644 --- a/syntax/attrs.rs +++ b/syntax/attrs.rs @@ -1,4 +1,3 @@ -use crate::syntax::qualified::QualifiedName; use crate::syntax::report::Errors; use crate::syntax::Atom::{self, *}; use crate::syntax::{Derive, Doc, Namespace}; @@ -114,6 +113,5 @@ fn parse_repr_attribute(input: ParseStream) -> Result { fn parse_namespace_attribute(input: ParseStream) -> Result { input.parse::()?; - let name = input.call(QualifiedName::parse_quoted_or_unquoted)?; - Ok(Namespace::from(name)) + input.parse::() } diff --git a/syntax/namespace.rs b/syntax/namespace.rs index 2d8fe9106..9a4705891 100644 --- a/syntax/namespace.rs +++ b/syntax/namespace.rs @@ -30,26 +30,24 @@ impl Namespace { segments.push(ident.to_string()); segments.join("::") } -} -impl From for Namespace { - fn from(value: QualifiedName) -> Namespace { - Namespace { - segments: value.segments, + pub fn parse_bridge_attr_namespace(input: ParseStream) -> Result { + if input.is_empty() { + return Ok(Namespace::none()); } + + input.parse::()?; + input.parse::()?; + let ns = input.parse::()?; + input.parse::>()?; + Ok(ns) } } impl Parse for Namespace { fn parse(input: ParseStream) -> Result { - if !input.is_empty() { - input.parse::()?; - input.parse::()?; - let name = input.call(QualifiedName::parse_quoted_or_unquoted)?; - input.parse::>()?; - return Ok(Namespace::from(name)); - } - Ok(Namespace::none()) + let segments = QualifiedName::parse_quoted_or_unquoted(input)?.segments; + Ok(Namespace { segments }) } } From 30244a705c145f1479e4188b3815ce091b9aff9e Mon Sep 17 00:00:00 2001 From: Bryan Henry Date: Mon, 14 Sep 2020 00:18:22 -0700 Subject: [PATCH 06/11] Move Module attribute parsing to syntax --- gen/src/file.rs | 5 ++--- macro/src/expand.rs | 7 ++----- syntax/attrs.rs | 5 ++++- syntax/doc.rs | 4 ++++ syntax/file.rs | 30 ++++++++++++++++++++++++++---- 5 files changed, 38 insertions(+), 13 deletions(-) diff --git a/gen/src/file.rs b/gen/src/file.rs index 1b324cb68..8f3db120e 100644 --- a/gen/src/file.rs +++ b/gen/src/file.rs @@ -21,7 +21,7 @@ fn parse(input: ParseStream, modules: &mut Vec) -> Result<()> { while !input.is_empty() { let mut cxx_bridge = false; let mut namespace = Namespace::none(); - let mut attrs = input.call(Attribute::parse_outer)?; + let attrs = input.call(Attribute::parse_outer)?; for attr in &attrs { let path = &attr.path.segments; if path.len() == 2 && path[0].ident == "cxx" && path[1].ident == "bridge" { @@ -45,8 +45,7 @@ fn parse(input: ParseStream, modules: &mut Vec) -> Result<()> { if cxx_bridge { let mut module: Module = input.parse()?; module.namespace = namespace; - attrs.extend(module.attrs); - module.attrs = attrs; + module.add_attributes(&attrs)?; modules.push(module); } else { input.advance_to(&ahead); diff --git a/macro/src/expand.rs b/macro/src/expand.rs index 4593777b0..97b83c197 100644 --- a/macro/src/expand.rs +++ b/macro/src/expand.rs @@ -102,15 +102,12 @@ fn expand(ffi: Module, apis: &[Api], types: &Types) -> TokenStream { }); } - let attrs = ffi - .attrs - .into_iter() - .filter(|attr| attr.path.is_ident("doc")); + let doc = &ffi.doc; let vis = &ffi.vis; let ident = &ffi.ident; quote! { - #(#attrs)* + #doc #[deny(improper_ctypes)] #[allow(non_snake_case)] #vis mod #ident { diff --git a/syntax/attrs.rs b/syntax/attrs.rs index 0e9733ea0..2adef93f3 100644 --- a/syntax/attrs.rs +++ b/syntax/attrs.rs @@ -11,6 +11,7 @@ pub struct Parser<'a> { pub derives: Option<&'a mut Vec>, pub repr: Option<&'a mut Option>, pub namespace: Option<&'a mut Option>, + pub ignore_unsupported: bool, } pub(super) fn parse_doc(cx: &mut Errors, attrs: &[Attribute]) -> Doc { @@ -69,7 +70,9 @@ pub(super) fn parse(cx: &mut Errors, attrs: &[Attribute], mut parser: Parser) { Err(err) => return cx.push(err), } } - return cx.error(attr, "unsupported attribute"); + if !parser.ignore_unsupported { + return cx.error(attr, "unsupported attribute"); + } } } diff --git a/syntax/doc.rs b/syntax/doc.rs index 60bb4da1b..906e8915d 100644 --- a/syntax/doc.rs +++ b/syntax/doc.rs @@ -17,6 +17,10 @@ impl Doc { self.fragments.push(lit); } + pub fn extend(&mut self, doc: Doc) { + self.fragments.extend(doc.fragments) + } + pub fn to_string(&self) -> String { let mut doc = String::new(); for lit in &self.fragments { diff --git a/syntax/file.rs b/syntax/file.rs index 8b86adc73..49bb68db2 100644 --- a/syntax/file.rs +++ b/syntax/file.rs @@ -1,4 +1,6 @@ use crate::syntax::namespace::Namespace; +use crate::syntax::report::Errors; +use crate::syntax::{attrs, Doc}; use quote::quote; use syn::parse::{Error, Parse, ParseStream, Result}; use syn::{ @@ -8,7 +10,7 @@ use syn::{ pub struct Module { pub namespace: Namespace, - pub attrs: Vec, + pub doc: Doc, pub vis: Visibility, pub unsafety: Option, pub mod_token: Token![mod], @@ -33,6 +35,24 @@ pub struct ItemForeignMod { pub items: Vec, } +impl Module { + pub fn add_attributes(&mut self, attrs: &[Attribute]) -> Result<()> { + let ref mut errors = Errors::new(); + let mut doc = Doc::new(); + attrs::parse( + errors, + attrs, + attrs::Parser { + doc: Some(&mut doc), + ignore_unsupported: true, + ..attrs::Parser::default() + }, + ); + self.doc.extend(doc); + errors.propagate() + } +} + impl Parse for Module { fn parse(input: ParseStream) -> Result { let namespace = Namespace::none(); @@ -60,16 +80,18 @@ impl Parse for Module { items.push(content.parse()?); } - Ok(Module { + let mut module = Module { namespace, - attrs, + doc: Doc::new(), vis, unsafety, mod_token, ident, brace_token, content: items, - }) + }; + module.add_attributes(&attrs)?; + Ok(module) } } From 9cfc5669ce7eea024baf42d7a9391f685eccf774 Mon Sep 17 00:00:00 2001 From: Bryan Henry Date: Mon, 14 Sep 2020 01:53:50 -0700 Subject: [PATCH 07/11] Improved cxx::alias_namespace attribute --- gen/src/mod.rs | 10 ++++++---- gen/src/write.rs | 16 ++++++++-------- macro/src/expand.rs | 11 ++++------- syntax/attrs.rs | 34 ++++++++++++++++++++++++++-------- syntax/file.rs | 17 +++++++++++++---- syntax/mod.rs | 4 +++- syntax/namespace.rs | 2 +- syntax/parse.rs | 27 ++++++++++++++------------- syntax/qualified.rs | 27 ++++++++++++++++++++++++++- tests/ffi/alias.rs | 6 +----- 10 files changed, 102 insertions(+), 52 deletions(-) diff --git a/gen/src/mod.rs b/gen/src/mod.rs index 9578fe867..b4252a542 100644 --- a/gen/src/mod.rs +++ b/gen/src/mod.rs @@ -16,6 +16,7 @@ use self::error::{format_err, Result}; use self::file::File; use crate::syntax::report::Errors; use crate::syntax::{self, check, Types}; +use std::mem; use std::path::Path; /// Options for C++ code generation. @@ -104,14 +105,15 @@ fn generate_from_string(source: &str, opt: &Opt) -> Result { pub(super) fn generate(syntax: File, opt: &Opt) -> Result { proc_macro2::fallback::force(); let ref mut errors = Errors::new(); - let bridge = syntax + let mut bridge = syntax .modules .into_iter() .next() .ok_or(Error::NoBridgeMod)?; let ref namespace = bridge.namespace; + let content = mem::take(&mut bridge.content); let trusted = bridge.unsafety.is_some(); - let ref apis = syntax::parse_items(errors, bridge.content, trusted); + let ref apis = syntax::parse_items(errors, content, trusted); let ref types = Types::collect(errors, apis); errors.propagate()?; check::typecheck(errors, namespace, apis, types); @@ -121,12 +123,12 @@ pub(super) fn generate(syntax: File, opt: &Opt) -> Result { // only need to generate one or the other. Ok(GeneratedCode { header: if opt.gen_header { - write::gen(namespace, apis, types, opt, true).content() + write::gen(&bridge, apis, types, opt, true).content() } else { Vec::new() }, implementation: if opt.gen_implementation { - write::gen(namespace, apis, types, opt, false).content() + write::gen(&bridge, apis, types, opt, false).content() } else { Vec::new() }, diff --git a/gen/src/write.rs b/gen/src/write.rs index 064729c6b..7d5543d9d 100644 --- a/gen/src/write.rs +++ b/gen/src/write.rs @@ -1,6 +1,7 @@ use crate::gen::out::OutFile; use crate::gen::{include, Opt}; use crate::syntax::atom::Atom::{self, *}; +use crate::syntax::file::Module; use crate::syntax::namespace::Namespace; use crate::syntax::symbol::Symbol; use crate::syntax::{ @@ -10,12 +11,13 @@ use proc_macro2::Ident; use std::collections::HashMap; pub(super) fn gen( - namespace: &Namespace, + bridge: &Module, apis: &[Api], types: &Types, opt: &Opt, header: bool, ) -> OutFile { + let namespace = &bridge.namespace; let mut out_file = OutFile::new(namespace.clone(), header); let out = &mut out_file; @@ -44,7 +46,7 @@ pub(super) fn gen( Api::Struct(strct) => write_struct_decl(out, &strct.ident), Api::CxxType(ety) => write_struct_using(out, &ety.ident), Api::RustType(ety) => write_struct_decl(out, &ety.ident), - Api::TypeAlias(alias) => write_alias(out, alias), + Api::TypeAlias(alias) => write_alias(out, bridge, alias), _ => {} } } @@ -934,12 +936,10 @@ fn write_type(out: &mut OutFile, ty: &Type) { } } -fn write_alias(out: &mut OutFile, alias: &TypeAlias) { - if let Some(namespace) = &alias.namespace { - // Review TODO: Is this unwrap fine? i.e. is it ok to assume that, if - // the TypePath parsed, that it has at least one segment? - let remote_type = &alias.ty.path.segments.last().unwrap().ident; - let path = namespace.path_for_type(remote_type); +fn write_alias(out: &mut OutFile, bridge: &Module, alias: &TypeAlias) { + let namespace = bridge.namespace_for_alias(alias); + if namespace != &bridge.namespace { + let path = namespace.path_for_type(&alias.ty_ident); writeln!(out, "using {} = {};", alias.ident, path) } } diff --git a/macro/src/expand.rs b/macro/src/expand.rs index 97b83c197..17d58c3e6 100644 --- a/macro/src/expand.rs +++ b/macro/src/expand.rs @@ -56,7 +56,7 @@ 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)); + hidden.extend(expand_type_alias_verify(&ffi, alias)); } } } @@ -663,12 +663,9 @@ fn expand_type_alias(alias: &TypeAlias) -> TokenStream { } } -fn expand_type_alias_verify(namespace: &Namespace, alias: &TypeAlias) -> TokenStream { - let namespace = alias.namespace.as_ref().unwrap_or(namespace); - // Review TODO: Is this unwrap fine? i.e. is it ok to assume that, if the - // TypePath parsed, that it has at least one segment? - let remote_type = &alias.ty.path.segments.last().unwrap().ident; - let type_id = type_id(namespace, remote_type); +fn expand_type_alias_verify(ffi: &Module, alias: &TypeAlias) -> TokenStream { + let namespace = ffi.namespace_for_alias(alias); + let type_id = type_id(namespace, &alias.ty_ident); let ident = &alias.ident; let begin_span = alias.type_token.span; let end_span = alias.semi_token.span; diff --git a/syntax/attrs.rs b/syntax/attrs.rs index 2adef93f3..5a3339e55 100644 --- a/syntax/attrs.rs +++ b/syntax/attrs.rs @@ -1,7 +1,9 @@ +use crate::syntax::qualified::QualifiedName; use crate::syntax::report::Errors; use crate::syntax::Atom::{self, *}; use crate::syntax::{Derive, Doc, Namespace}; use proc_macro2::Ident; +use std::collections::HashMap; use syn::parse::{ParseStream, Parser as _}; use syn::{Attribute, Error, LitStr, Path, Result, Token}; @@ -10,7 +12,7 @@ pub struct Parser<'a> { pub doc: Option<&'a mut Doc>, pub derives: Option<&'a mut Vec>, pub repr: Option<&'a mut Option>, - pub namespace: Option<&'a mut Option>, + pub alias_namespaces: Option<&'a mut HashMap>, pub ignore_unsupported: bool, } @@ -59,11 +61,20 @@ pub(super) fn parse(cx: &mut Errors, attrs: &[Attribute], mut parser: Parser) { } Err(err) => return cx.push(err), } - } else if attr.path.is_ident("namespace") { - match parse_namespace_attribute.parse2(attr.tokens.clone()) { - Ok(namespace) => { - if let Some(ns) = &mut parser.namespace { - **ns = Some(Namespace::from(namespace)); + } else if is_cxx_alias_namespace_attr(attr) { + match attr.parse_args_with(parse_namespace_attribute) { + Ok((name, namespace)) => { + if let Some(map) = &mut parser.alias_namespaces { + if let Some(existing) = map.get(&name) { + return cx.error( + attr, + format!( + "conflicting cxx::alias_namespace attributes for {}: {}, {}", + name, existing, namespace + ), + ); + } + map.insert(name, namespace); continue; } } @@ -76,6 +87,11 @@ pub(super) fn parse(cx: &mut Errors, attrs: &[Attribute], mut parser: Parser) { } } +fn is_cxx_alias_namespace_attr(attr: &Attribute) -> bool { + let path = &attr.path.segments; + path.len() == 2 && path[0].ident == "cxx" && path[1].ident == "alias_namespace" +} + fn parse_doc_attribute(input: ParseStream) -> Result { input.parse::()?; let lit: LitStr = input.parse()?; @@ -114,7 +130,9 @@ fn parse_repr_attribute(input: ParseStream) -> Result { )) } -fn parse_namespace_attribute(input: ParseStream) -> Result { +fn parse_namespace_attribute(input: ParseStream) -> Result<(QualifiedName, Namespace)> { + let name = QualifiedName::parse_quoted_or_unquoted(input)?; input.parse::()?; - input.parse::() + let namespace = input.parse::()?; + Ok((name, namespace)) } diff --git a/syntax/file.rs b/syntax/file.rs index 49bb68db2..a642cc595 100644 --- a/syntax/file.rs +++ b/syntax/file.rs @@ -1,7 +1,9 @@ use crate::syntax::namespace::Namespace; +use crate::syntax::qualified::QualifiedName; use crate::syntax::report::Errors; -use crate::syntax::{attrs, Doc}; +use crate::syntax::{attrs, Doc, TypeAlias}; use quote::quote; +use std::collections::HashMap; use syn::parse::{Error, Parse, ParseStream, Result}; use syn::{ braced, token, Abi, Attribute, ForeignItem, Ident, Item as RustItem, ItemEnum, ItemStruct, @@ -10,6 +12,7 @@ use syn::{ pub struct Module { pub namespace: Namespace, + pub alias_namespaces: HashMap, pub doc: Doc, pub vis: Visibility, pub unsafety: Option, @@ -38,19 +41,24 @@ pub struct ItemForeignMod { impl Module { pub fn add_attributes(&mut self, attrs: &[Attribute]) -> Result<()> { let ref mut errors = Errors::new(); - let mut doc = Doc::new(); attrs::parse( errors, attrs, attrs::Parser { - doc: Some(&mut doc), + doc: Some(&mut self.doc), + alias_namespaces: Some(&mut self.alias_namespaces), ignore_unsupported: true, ..attrs::Parser::default() }, ); - self.doc.extend(doc); errors.propagate() } + + pub fn namespace_for_alias(&self, alias: &TypeAlias) -> &Namespace { + self.alias_namespaces + .get(&alias.ty_path) + .unwrap_or(&self.namespace) + } } impl Parse for Module { @@ -82,6 +90,7 @@ impl Parse for Module { let mut module = Module { namespace, + alias_namespaces: HashMap::new(), doc: Doc::new(), vis, unsafety, diff --git a/syntax/mod.rs b/syntax/mod.rs index 86c49be54..a3c87a8e6 100644 --- a/syntax/mod.rs +++ b/syntax/mod.rs @@ -32,6 +32,7 @@ pub use self::derive::Derive; pub use self::doc::Doc; pub use self::namespace::Namespace; pub use self::parse::parse_items; +pub use self::qualified::QualifiedName; pub use self::types::Types; pub enum Api { @@ -81,11 +82,12 @@ pub struct ExternFn { pub struct TypeAlias { pub doc: Doc, - pub namespace: Option, pub type_token: Token![type], pub ident: Ident, pub eq_token: Token![=], pub ty: TypePath, + pub ty_path: QualifiedName, + pub ty_ident: Ident, pub semi_token: Token![;], } diff --git a/syntax/namespace.rs b/syntax/namespace.rs index 9a4705891..8dfe97ac6 100644 --- a/syntax/namespace.rs +++ b/syntax/namespace.rs @@ -9,7 +9,7 @@ mod kw { syn::custom_keyword!(namespace); } -#[derive(Clone)] +#[derive(Clone, PartialEq, Eq)] pub struct Namespace { segments: Vec, } diff --git a/syntax/parse.rs b/syntax/parse.rs index e38e92f2e..13519559d 100644 --- a/syntax/parse.rs +++ b/syntax/parse.rs @@ -1,5 +1,6 @@ use crate::syntax::discriminant::DiscriminantSet; use crate::syntax::file::{Item, ItemForeignMod}; +use crate::syntax::qualified::QualifiedName; use crate::syntax::report::Errors; use crate::syntax::Atom::*; use crate::syntax::{ @@ -10,6 +11,7 @@ use proc_macro2::{TokenStream, TokenTree}; use quote::{format_ident, quote, quote_spanned}; use syn::parse::{ParseStream, Parser}; use syn::punctuated::Punctuated; +use syn::spanned::Spanned; use syn::{ Abi, Attribute, Error, Fields, FnArg, ForeignItem, ForeignItemFn, ForeignItemType, GenericArgument, Ident, ItemEnum, ItemStruct, LitStr, Pat, PathArguments, Result, ReturnType, @@ -385,6 +387,7 @@ fn parse_extern_verbatim(cx: &mut Errors, tokens: &TokenStream, lang: Lang) -> R // type Alias = crate::path::to::Type; let parse = |input: ParseStream| -> Result { let attrs = input.call(Attribute::parse_outer)?; + let doc = attrs::parse_doc(cx, &attrs); let type_token: Token![type] = match input.parse()? { Some(type_token) => type_token, None => { @@ -395,27 +398,25 @@ fn parse_extern_verbatim(cx: &mut Errors, tokens: &TokenStream, lang: Lang) -> R let ident: Ident = input.parse()?; let eq_token: Token![=] = input.parse()?; let ty: TypePath = input.parse()?; - let semi_token: Token![;] = input.parse()?; - let mut doc = Doc::new(); - let mut namespace = None; - attrs::parse( - cx, - &attrs, - attrs::Parser { - doc: Some(&mut doc), - namespace: Some(&mut namespace), - ..Default::default() - }, - ); + let segments = &ty.path.segments; + if segments.len() <= 1 { + return Err(Error::new(ty.span(), "invalid type alias")); + } + let iter = segments.iter().take(segments.len() - 1).cloned(); + let ty_path = QualifiedName::from_path_segments(iter, ty.span())?; + let ty_ident = segments.last().unwrap().ident.clone(); + + let semi_token: Token![;] = input.parse()?; Ok(TypeAlias { doc, - namespace, type_token, ident, eq_token, ty, + ty_path, + ty_ident, semi_token, }) }; diff --git a/syntax/qualified.rs b/syntax/qualified.rs index be9bceb43..6c75593b4 100644 --- a/syntax/qualified.rs +++ b/syntax/qualified.rs @@ -1,7 +1,10 @@ +use proc_macro2::Span; +use std::fmt::{self, Display}; use syn::ext::IdentExt; use syn::parse::{ParseStream, Result}; -use syn::{Ident, LitStr, Token}; +use syn::{Error, Ident, LitStr, PathArguments, PathSegment, Token}; +#[derive(Hash, PartialEq, Eq)] pub struct QualifiedName { pub segments: Vec, } @@ -32,4 +35,26 @@ impl QualifiedName { Self::parse_unquoted(input) } } + + pub fn from_path_segments>( + iter: T, + span: Span, + ) -> Result { + let mut segments = Vec::new(); + for segment in iter { + match segment.arguments { + PathArguments::None => {} + _ => return Err(Error::new(span, "unexpected path arguments")), + }; + segments.push(segment.ident); + } + Ok(QualifiedName { segments }) + } +} + +impl Display for QualifiedName { + fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result { + let segments: Vec = self.segments.iter().map(ToString::to_string).collect(); + write!(f, "{}", segments.join("::")) + } } diff --git a/tests/ffi/alias.rs b/tests/ffi/alias.rs index 241f1fe41..db886aafd 100644 --- a/tests/ffi/alias.rs +++ b/tests/ffi/alias.rs @@ -2,16 +2,12 @@ // https://github.com/rust-lang/rustfmt/issues/4159 #[rustfmt::skip] #[cxx::bridge(namespace = alias_tests)] +#[cxx::alias_namespace(crate::ffi = tests)] pub mod ffi { extern "C" { include!("cxx-test-suite/tests.h"); - // Review TODO: Unquoted namespace here doesn't work, is that expected or a bug - // in my parsing? - #[namespace = "tests"] type C = crate::ffi::C; - - #[namespace = "tests"] type SameC = crate::ffi::C; fn c_return_unique_ptr() -> UniquePtr; From 3396b3c4c5e82f84809ca43470cbc2436568c082 Mon Sep 17 00:00:00 2001 From: Bryan Henry Date: Mon, 14 Sep 2020 02:24:55 -0700 Subject: [PATCH 08/11] Add ui test for conflicting cxx::alias_namespace attributes --- gen/src/write.rs | 2 +- macro/src/expand.rs | 2 +- syntax/attrs.rs | 4 +++- syntax/namespace.rs | 6 ++++-- tests/ui/conflicting_alias_namespace.rs | 20 ++++++++++++++++++++ tests/ui/conflicting_alias_namespace.stderr | 1 + 6 files changed, 30 insertions(+), 5 deletions(-) create mode 100644 tests/ui/conflicting_alias_namespace.rs create mode 100644 tests/ui/conflicting_alias_namespace.stderr diff --git a/gen/src/write.rs b/gen/src/write.rs index 7d5543d9d..cc4886cac 100644 --- a/gen/src/write.rs +++ b/gen/src/write.rs @@ -939,7 +939,7 @@ fn write_type(out: &mut OutFile, ty: &Type) { fn write_alias(out: &mut OutFile, bridge: &Module, alias: &TypeAlias) { let namespace = bridge.namespace_for_alias(alias); if namespace != &bridge.namespace { - let path = namespace.path_for_type(&alias.ty_ident); + let path = namespace.path_for_type(Some(&alias.ty_ident)); writeln!(out, "using {} = {};", alias.ident, path) } } diff --git a/macro/src/expand.rs b/macro/src/expand.rs index 17d58c3e6..00b2b328a 100644 --- a/macro/src/expand.rs +++ b/macro/src/expand.rs @@ -678,7 +678,7 @@ fn expand_type_alias_verify(ffi: &Module, alias: &TypeAlias) -> TokenStream { } fn type_id(namespace: &Namespace, ident: &Ident) -> TokenStream { - let path = namespace.path_for_type(ident); + let path = namespace.path_for_type(Some(ident)); quote! { ::cxx::type_id!(#path) } diff --git a/syntax/attrs.rs b/syntax/attrs.rs index 5a3339e55..f554357b4 100644 --- a/syntax/attrs.rs +++ b/syntax/attrs.rs @@ -70,7 +70,9 @@ pub(super) fn parse(cx: &mut Errors, attrs: &[Attribute], mut parser: Parser) { attr, format!( "conflicting cxx::alias_namespace attributes for {}: {}, {}", - name, existing, namespace + name, + existing.path_for_type(None), + namespace.path_for_type(None) ), ); } diff --git a/syntax/namespace.rs b/syntax/namespace.rs index 8dfe97ac6..5ff1e28e1 100644 --- a/syntax/namespace.rs +++ b/syntax/namespace.rs @@ -25,9 +25,11 @@ impl Namespace { self.segments.iter() } - pub fn path_for_type(&self, ident: &Ident) -> String { + pub fn path_for_type(&self, ident: Option<&Ident>) -> String { let mut segments = self.iter().map(ToString::to_string).collect::>(); - segments.push(ident.to_string()); + if let Some(ident) = ident { + segments.push(ident.to_string()); + } segments.join("::") } diff --git a/tests/ui/conflicting_alias_namespace.rs b/tests/ui/conflicting_alias_namespace.rs new file mode 100644 index 000000000..886ce702b --- /dev/null +++ b/tests/ui/conflicting_alias_namespace.rs @@ -0,0 +1,20 @@ +#[cxx::bridge(namespace = correct)] +mod here { + extern "C" { + type StringPiece; + } +} + +// Rustfmt mangles the extern type alias. +// https://github.com/rust-lang/rustfmt/issues/4159 +#[rustfmt::skip] +#[cxx::bridge(namespace = other)] +#[cxx::alias_namespace(crate::here = correct)] +#[cxx::alias_namespace(crate::here = folly)] +mod there { + extern "C" { + type StringPiece = crate::here::StringPiece; + } +} + +fn main() {} diff --git a/tests/ui/conflicting_alias_namespace.stderr b/tests/ui/conflicting_alias_namespace.stderr new file mode 100644 index 000000000..d2e6a8e87 --- /dev/null +++ b/tests/ui/conflicting_alias_namespace.stderr @@ -0,0 +1 @@ +error: conflicting cxx::alias_namespace attributes for crate::here: correct, folly From c20deeaed0d7106b3f309beeb62e122087631c83 Mon Sep 17 00:00:00 2001 From: Bryan Henry Date: Mon, 14 Sep 2020 19:51:26 -0700 Subject: [PATCH 09/11] More tests, including UI tests of alias usage that doesn't currently work --- tests/BUCK | 9 +++++++ tests/BUILD | 10 ++++++++ tests/ffi/alias.rs | 6 +++++ tests/ffi/alias2.rs | 20 +++++++++++++++ tests/ffi/build.rs | 2 +- tests/ffi/lib.rs | 1 + tests/ffi/tests.cc | 9 +++++++ tests/ffi/tests.h | 14 ++++++++++ tests/test.rs | 10 +++++++- tests/ui/alias_of_alias.rs | 41 ++++++++++++++++++++++++++++++ tests/ui/alias_of_alias.stderr | 13 ++++++++++ tests/ui/alias_unique_usage.rs | 26 +++++++++++++++++++ tests/ui/alias_unique_usage.stderr | 10 ++++++++ 13 files changed, 169 insertions(+), 2 deletions(-) create mode 100644 tests/ffi/alias2.rs create mode 100644 tests/ui/alias_of_alias.rs create mode 100644 tests/ui/alias_of_alias.stderr create mode 100644 tests/ui/alias_unique_usage.rs create mode 100644 tests/ui/alias_unique_usage.stderr diff --git a/tests/BUCK b/tests/BUCK index 9ccf941c4..05361f7b5 100644 --- a/tests/BUCK +++ b/tests/BUCK @@ -8,6 +8,7 @@ rust_library( name = "ffi", srcs = [ "ffi/alias.rs", + "ffi/alias2.rs", "ffi/lib.rs", ], crate = "cxx_test_suite", @@ -22,6 +23,7 @@ cxx_library( srcs = [ "ffi/tests.cc", ":gen-alias-source", + ":gen-alias2-source", ":gen-lib-source", ], header_namespace = "cxx-test-suite", @@ -52,3 +54,10 @@ genrule( out = "alias.rs.cc", cmd = "$(exe //:codegen) ${SRCS} > ${OUT}", ) + +genrule( + name = "gen-alias2-source", + srcs = ["ffi/alias2.rs"], + out = "alias2.rs.cc", + cmd = "$(exe //:codegen) ${SRCS} > ${OUT}", +) diff --git a/tests/BUILD b/tests/BUILD index e42985c76..0f300e006 100644 --- a/tests/BUILD +++ b/tests/BUILD @@ -10,6 +10,7 @@ rust_library( name = "cxx_test_suite", srcs = [ "ffi/alias.rs", + "ffi/alias2.rs", "ffi/lib.rs", ], deps = [ @@ -23,6 +24,7 @@ cc_library( srcs = [ "ffi/tests.cc", ":gen-alias-source", + ":gen-alias2-source", ":gen-lib-source", ], hdrs = ["ffi/tests.h"], @@ -63,3 +65,11 @@ genrule( cmd = "$(location //:codegen) $< > $@", tools = ["//:codegen"], ) + +genrule( + name = "gen-alias2-source", + srcs = ["ffi/alias2.rs"], + outs = ["alias2.rs.cc"], + cmd = "$(location //:codegen) $< > $@", + tools = ["//:codegen"], +) diff --git a/tests/ffi/alias.rs b/tests/ffi/alias.rs index db886aafd..23cee6977 100644 --- a/tests/ffi/alias.rs +++ b/tests/ffi/alias.rs @@ -10,7 +10,13 @@ pub mod ffi { type C = crate::ffi::C; type SameC = crate::ffi::C; + // Used to test multiple cxx::alias_namespace attributes in alias2 + type DifferentC; + fn c_return_unique_ptr() -> UniquePtr; fn c_take_unique_ptr(c: UniquePtr); + + // TODO: Workaround for github.com/dtolnay/cxx/issues/XXX + fn create_different_c() -> UniquePtr; } } diff --git a/tests/ffi/alias2.rs b/tests/ffi/alias2.rs new file mode 100644 index 000000000..9f8895b00 --- /dev/null +++ b/tests/ffi/alias2.rs @@ -0,0 +1,20 @@ +// Rustfmt mangles the extern type alias. +// https://github.com/rust-lang/rustfmt/issues/4159 +#[rustfmt::skip] +#[cxx::bridge(namespace = alias2_tests)] +#[cxx::alias_namespace(crate::ffi = tests)] +#[cxx::alias_namespace(crate::alias::ffi = alias_tests)] +pub mod ffi { + extern "C" { + include!("cxx-test-suite/tests.h"); + include!("cxx-test-suite/alias.rs.h"); + + type C = crate::ffi::C; + type DifferentC = crate::alias::ffi::DifferentC; + + fn c_return_unique_ptr() -> UniquePtr; + fn c_take_unique_ptr(c: UniquePtr); + + fn create_different_c() -> UniquePtr; + } +} diff --git a/tests/ffi/build.rs b/tests/ffi/build.rs index 10078531f..6e435ace9 100644 --- a/tests/ffi/build.rs +++ b/tests/ffi/build.rs @@ -3,7 +3,7 @@ fn main() { return; } - let sources = vec!["alias.rs", "lib.rs"]; + let sources = vec!["alias.rs", "alias2.rs", "lib.rs"]; cxx_build::bridges(sources) .file("tests.cc") .flag_if_supported(cxxbridge_flags::STD) diff --git a/tests/ffi/lib.rs b/tests/ffi/lib.rs index 03ca4f34e..0defe3042 100644 --- a/tests/ffi/lib.rs +++ b/tests/ffi/lib.rs @@ -5,6 +5,7 @@ )] pub mod alias; +pub mod alias2; use cxx::{CxxString, UniquePtr}; use std::fmt::{self, Display}; diff --git a/tests/ffi/tests.cc b/tests/ffi/tests.cc index 8dd16e8d9..8e45a444e 100644 --- a/tests/ffi/tests.cc +++ b/tests/ffi/tests.cc @@ -421,3 +421,12 @@ extern "C" const char *cxx_run_test() noexcept { } } // namespace tests + +namespace alias_tests { + +std::unique_ptr create_different_c() { + return std::unique_ptr( + new alias_tests::DifferentC{2000}); +} + +} // namespace alias_tests diff --git a/tests/ffi/tests.h b/tests/ffi/tests.h index 2af2b0fc1..6325212d9 100644 --- a/tests/ffi/tests.h +++ b/tests/ffi/tests.h @@ -110,4 +110,18 @@ namespace alias_tests { using tests::c_return_unique_ptr; using tests::c_take_unique_ptr; +struct DifferentC { + size_t n; +}; + +std::unique_ptr create_different_c(); + } // namespace alias_tests + +namespace alias2_tests { + +using alias_tests::create_different_c; +using tests::c_return_unique_ptr; +using tests::c_take_unique_ptr; + +} // namespace alias2_tests diff --git a/tests/test.rs b/tests/test.rs index b7ce86e93..785b4e982 100644 --- a/tests/test.rs +++ b/tests/test.rs @@ -1,4 +1,4 @@ -use cxx_test_suite::{alias, ffi}; +use cxx_test_suite::{alias, alias2, ffi}; use std::cell::Cell; use std::ffi::CStr; @@ -178,6 +178,14 @@ fn test_alias() { check!(alias::ffi::c_take_unique_ptr(unique_ptr)); } +#[test] +fn test_alias2() { + let unique_ptr = alias2::ffi::c_return_unique_ptr(); + check!(alias2::ffi::c_take_unique_ptr(unique_ptr)); + + alias2::ffi::create_different_c(); +} + #[no_mangle] extern "C" fn cxx_test_suite_get_box() -> *mut cxx_test_suite::R { Box::into_raw(Box::new(2020usize)) diff --git a/tests/ui/alias_of_alias.rs b/tests/ui/alias_of_alias.rs new file mode 100644 index 000000000..b2b7db116 --- /dev/null +++ b/tests/ui/alias_of_alias.rs @@ -0,0 +1,41 @@ +// Aliases of aliases don't work if the namespace of any intermediate alias +// differs from the original bridge's namespace. This is because the ExternType +// trait being checked against will use the original namespace, which the final +// alias does not know. However, if not for the ExternType check, this would +// work because of the C++ "using" aliases generated from each bridge. +// +// This can be made to work if needed - either through an attribute on the alias +// as a "manual" fix, or potentially through some creativity with ExternType +// (like defining a related type that implements ExternType and checking against +// that instead of the actual type) - but seems like a rare case. + +#[cxx::bridge(namespace = here)] +mod here { + extern "C" { + type C; + } +} + +// Rustfmt mangles the extern type alias. +// https://github.com/rust-lang/rustfmt/issues/4159 +#[rustfmt::skip] +#[cxx::bridge(namespace = there)] +#[cxx::alias_namespace(crate::here = here)] +mod there { + extern "C" { + type C = crate::here::C; + } +} + +// Rustfmt mangles the extern type alias. +// https://github.com/rust-lang/rustfmt/issues/4159 +#[rustfmt::skip] +#[cxx::bridge(namespace = anywhere)] +#[cxx::alias_namespace(crate::there = there)] +mod anywhere { + extern "C" { + type C = crate::there::C; + } +} + +fn main() {} diff --git a/tests/ui/alias_of_alias.stderr b/tests/ui/alias_of_alias.stderr new file mode 100644 index 000000000..f79e449b4 --- /dev/null +++ b/tests/ui/alias_of_alias.stderr @@ -0,0 +1,13 @@ +error[E0271]: type mismatch resolving `::Id == (t, h, e, r, e, (), cxx::C)` + --> $DIR/alias_of_alias.rs:33:1 + | +33 | #[cxx::bridge(namespace = anywhere)] + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ expected a tuple with 7 elements, found one with 6 elements + | + ::: $WORKSPACE/src/extern_type.rs + | + | pub fn verify_extern_type, Id>() {} + | ------- required by this bound in `verify_extern_type` + | + = note: expected tuple `(t, h, e, r, e, (), cxx::C)` + found tuple `(h, e, r, e, (), cxx::C)` diff --git a/tests/ui/alias_unique_usage.rs b/tests/ui/alias_unique_usage.rs new file mode 100644 index 000000000..4472e23fb --- /dev/null +++ b/tests/ui/alias_unique_usage.rs @@ -0,0 +1,26 @@ +// This fails because the original bridge does not see any usage of +// UniquePtr and so does not generate the approprite trait. The +// original bridge currently needs to include some usage of UniquePtr. +// TODO: File a Github issue to fix this. + +#[cxx::bridge(namespace = here)] +mod here { + extern "C" { + type C; + } +} + +// Rustfmt mangles the extern type alias. +// https://github.com/rust-lang/rustfmt/issues/4159 +#[rustfmt::skip] +#[cxx::bridge(namespace = there)] +#[cxx::alias_namespace(crate::here = here)] +mod there { + extern "C" { + type C = crate::here::C; + + fn c_return_unique_ptr() -> UniquePtr; + } +} + +fn main() {} diff --git a/tests/ui/alias_unique_usage.stderr b/tests/ui/alias_unique_usage.stderr new file mode 100644 index 000000000..78c5106e8 --- /dev/null +++ b/tests/ui/alias_unique_usage.stderr @@ -0,0 +1,10 @@ +error[E0277]: the trait bound `here::C: UniquePtrTarget` is not satisfied + --> $DIR/alias_unique_usage.rs:16:1 + | +16 | #[cxx::bridge(namespace = there)] + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ the trait `UniquePtrTarget` is not implemented for `here::C` + | + ::: $WORKSPACE/src/unique_ptr.rs + | + | T: UniquePtrTarget, + | --------------- required by this bound in `UniquePtr` From 0cb1ba5308795276bd507624187b75233f6ce753 Mon Sep 17 00:00:00 2001 From: Bryan Henry Date: Fri, 18 Sep 2020 18:19:22 -0700 Subject: [PATCH 10/11] Add macros for Bazel and Buck to generate a cc_library from Rust bridge sources --- tests/BUCK | 45 +++++------------------ tests/BUILD | 58 +++++------------------------ tools/bazel/cxx_cc_library.bzl | 67 ++++++++++++++++++++++++++++++++++ tools/buck/cxx_cxx_library.bzl | 58 +++++++++++++++++++++++++++++ 4 files changed, 143 insertions(+), 85 deletions(-) create mode 100644 tools/bazel/cxx_cc_library.bzl create mode 100644 tools/buck/cxx_cxx_library.bzl diff --git a/tests/BUCK b/tests/BUCK index 05361f7b5..d63b1960e 100644 --- a/tests/BUCK +++ b/tests/BUCK @@ -1,3 +1,5 @@ +load("//tools/buck:cxx_cxx_library.bzl", "cxx_cxx_library") + rust_test( name = "test", srcs = ["test.rs"], @@ -18,46 +20,17 @@ rust_library( ], ) -cxx_library( +cxx_cxx_library( name = "impl", - srcs = [ - "ffi/tests.cc", - ":gen-alias-source", - ":gen-alias2-source", - ":gen-lib-source", + crate_name = "cxx-test-suite", + bridge_srcs = [ + "ffi/alias.rs", + "ffi/alias2.rs", + "ffi/lib.rs", ], - header_namespace = "cxx-test-suite", + srcs = ["ffi/tests.cc"], headers = { - "lib.rs.h": ":gen-lib-header", "tests.h": "ffi/tests.h", }, deps = ["//:core"], ) - -genrule( - name = "gen-lib-header", - srcs = ["ffi/lib.rs"], - out = "lib.rs.h", - cmd = "$(exe //:codegen) --header ${SRCS} > ${OUT}", -) - -genrule( - name = "gen-lib-source", - srcs = ["ffi/lib.rs"], - out = "lib.rs.cc", - cmd = "$(exe //:codegen) ${SRCS} > ${OUT}", -) - -genrule( - name = "gen-alias-source", - srcs = ["ffi/alias.rs"], - out = "alias.rs.cc", - cmd = "$(exe //:codegen) ${SRCS} > ${OUT}", -) - -genrule( - name = "gen-alias2-source", - srcs = ["ffi/alias2.rs"], - out = "alias2.rs.cc", - cmd = "$(exe //:codegen) ${SRCS} > ${OUT}", -) diff --git a/tests/BUILD b/tests/BUILD index 0f300e006..7d35d924c 100644 --- a/tests/BUILD +++ b/tests/BUILD @@ -1,3 +1,4 @@ +load("//tools/bazel:cxx_cc_library.bzl", "cxx_cc_library") load("//tools/bazel:rust.bzl", "rust_library", "rust_test") rust_test( @@ -19,57 +20,16 @@ rust_library( ], ) -cc_library( +cxx_cc_library( name = "impl", - srcs = [ - "ffi/tests.cc", - ":gen-alias-source", - ":gen-alias2-source", - ":gen-lib-source", + crate_name = "cxx-test-suite", + bridge_srcs = [ + "ffi/alias.rs", + "ffi/alias2.rs", + "ffi/lib.rs", ], + srcs = ["ffi/tests.cc"], hdrs = ["ffi/tests.h"], - include_prefix = "cxx-test-suite", strip_include_prefix = "ffi", - deps = [ - ":lib-include", - "//:core", - ], -) - -genrule( - name = "gen-lib-header", - srcs = ["ffi/lib.rs"], - outs = ["lib.rs.h"], - cmd = "$(location //:codegen) --header $< > $@", - tools = ["//:codegen"], -) - -genrule( - name = "gen-lib-source", - srcs = ["ffi/lib.rs"], - outs = ["lib.rs.cc"], - cmd = "$(location //:codegen) $< > $@", - tools = ["//:codegen"], -) - -cc_library( - name = "lib-include", - hdrs = [":gen-lib-header"], - include_prefix = "cxx-test-suite", -) - -genrule( - name = "gen-alias-source", - srcs = ["ffi/alias.rs"], - outs = ["alias.rs.cc"], - cmd = "$(location //:codegen) $< > $@", - tools = ["//:codegen"], -) - -genrule( - name = "gen-alias2-source", - srcs = ["ffi/alias2.rs"], - outs = ["alias2.rs.cc"], - cmd = "$(location //:codegen) $< > $@", - tools = ["//:codegen"], + deps = ["//:core"], ) diff --git a/tools/bazel/cxx_cc_library.bzl b/tools/bazel/cxx_cc_library.bzl new file mode 100644 index 000000000..362c04bda --- /dev/null +++ b/tools/bazel/cxx_cc_library.bzl @@ -0,0 +1,67 @@ +"""cxx_cc_library macro""" + +load("@rules_cc//cc:defs.bzl", "cc_library") + +def cxx_cc_library( + name, + crate_name, + bridge_srcs, + srcs = None, + hdrs = None, + deps = None, + strip_include_prefix = None, + visibility = None): + """Generate C++ source for one or more Rust cxx::bridges and bundle into a cc_library. + + Args: + name: A unique name for the cc_library rule. + crate_name: The name of the corresponding Rust crate, used as the include_prefix. + bridge_srcs: One or more Rust source files containing cxx::bridge modules. + srcs: Forwarded to cc_library unmodified. + hdrs: Forwarded to cc_library unmodified. + deps: Forwarded to cc_library unmodified + strip_include_prefix: Forwarded to cc_library unmodified + visibility: Visibility for all rules generated by this macro. + """ + + gen_srcs = [] + gen_hdrs = [] + for bridge_source in bridge_srcs: + source_basename = bridge_source.rsplit("/", 1)[-1] + header_target = source_basename + "-gen-header" + source_target = source_basename + "-gen-source" + + header_out = source_basename + ".h" + if strip_include_prefix: + header_out = strip_include_prefix + "/" + header_out + + native.genrule( + name = header_target, + srcs = [bridge_source], + outs = [header_out], + cmd = "$(location //:codegen) --header $< > $@", + tools = ["//:codegen"], + visibility = visibility, + ) + + native.genrule( + name = source_target, + srcs = [bridge_source], + outs = [source_basename + ".cc"], + cmd = "$(location //:codegen) $< > $@", + tools = ["//:codegen"], + visibility = visibility, + ) + + gen_hdrs.append(header_target) + gen_srcs.append(source_target) + + cc_library( + name = name, + srcs = srcs + gen_srcs, + hdrs = hdrs + gen_hdrs, + include_prefix = crate_name, + strip_include_prefix = strip_include_prefix, + deps = deps, + visibility = visibility, + ) diff --git a/tools/buck/cxx_cxx_library.bzl b/tools/buck/cxx_cxx_library.bzl new file mode 100644 index 000000000..58c60f5d8 --- /dev/null +++ b/tools/buck/cxx_cxx_library.bzl @@ -0,0 +1,58 @@ +"""cxx_cxx_library macro""" + +def cxx_cxx_library( + name, + crate_name, + bridge_srcs, + srcs = None, + headers = None, + deps = None, + visibility = None): + """Generate C++ source for one or more Rust cxx::bridges and bundle into a cc_library. + + Args: + name: A unique name for the cc_library rule. + crate_name: The name of the corresponding Rust crate, used as the include_prefix. + bridge_srcs: One or more Rust source files containing cxx::bridge modules. + srcs: Forwarded to cc_library unmodified. + headers: Forwarded to cc_library unmodified. + deps: Forwarded to cc_library unmodified + visibility: Visibility for all rules generated by this macro. + """ + + gen_srcs = [] + gen_hdrs = {} + for bridge_source in bridge_srcs: + source_basename = bridge_source.rsplit("/", 1)[-1] + header_target = source_basename + "-gen-header" + source_target = source_basename + "-gen-source" + + hdr_out = source_basename + ".h" + native.genrule( + name = header_target, + srcs = [bridge_source], + out = hdr_out, + cmd = "$(exe //:codegen) --header ${SRCS} > ${OUT}", + visibility = visibility, + ) + + native.genrule( + name = source_target, + srcs = [bridge_source], + out = source_basename + ".cc", + cmd = "$(exe //:codegen) ${SRCS} > ${OUT}", + visibility = visibility, + ) + + gen_hdrs[hdr_out] = ":" + header_target + gen_srcs.append(":" + source_target) + + cxx_library( + name = name, + srcs = srcs + gen_srcs, + headers = flatten_dicts(headers, gen_hdrs), + header_namespace = crate_name, + deps = deps, + visibility = visibility, + ) + From 242018c74d57e0b536cab4e4ed8ca63897031ad1 Mon Sep 17 00:00:00 2001 From: Bryan Henry Date: Sat, 19 Sep 2020 14:03:40 -0700 Subject: [PATCH 11/11] Better names for Bazel/Buck templates, bit of cleanup --- tests/BUCK | 22 +++++++++---------- tests/BUILD | 22 +++++++++---------- ...c_library.bzl => cxxbridge_cc_library.bzl} | 16 +++++++------- ..._library.bzl => cxxbridge_cxx_library.bzl} | 20 ++++++++--------- 4 files changed, 38 insertions(+), 42 deletions(-) rename tools/bazel/{cxx_cc_library.bzl => cxxbridge_cc_library.bzl} (82%) rename tools/buck/{cxx_cxx_library.bzl => cxxbridge_cxx_library.bzl} (75%) diff --git a/tests/BUCK b/tests/BUCK index d63b1960e..d6c23fd7e 100644 --- a/tests/BUCK +++ b/tests/BUCK @@ -1,4 +1,4 @@ -load("//tools/buck:cxx_cxx_library.bzl", "cxx_cxx_library") +load("//tools/buck:cxxbridge_cxx_library.bzl", "cxxbridge_cxx_library") rust_test( name = "test", @@ -6,13 +6,15 @@ rust_test( deps = [":ffi"], ) +bridge_srcs = [ + "ffi/alias.rs", + "ffi/alias2.rs", + "ffi/lib.rs", +] + rust_library( name = "ffi", - srcs = [ - "ffi/alias.rs", - "ffi/alias2.rs", - "ffi/lib.rs", - ], + srcs = bridge_srcs, crate = "cxx_test_suite", deps = [ ":impl", @@ -20,14 +22,10 @@ rust_library( ], ) -cxx_cxx_library( +cxxbridge_cxx_library( name = "impl", crate_name = "cxx-test-suite", - bridge_srcs = [ - "ffi/alias.rs", - "ffi/alias2.rs", - "ffi/lib.rs", - ], + bridge_srcs = bridge_srcs, srcs = ["ffi/tests.cc"], headers = { "tests.h": "ffi/tests.h", diff --git a/tests/BUILD b/tests/BUILD index 7d35d924c..c9c880852 100644 --- a/tests/BUILD +++ b/tests/BUILD @@ -1,4 +1,4 @@ -load("//tools/bazel:cxx_cc_library.bzl", "cxx_cc_library") +load("//tools/bazel:cxxbridge_cc_library.bzl", "cxxbridge_cc_library") load("//tools/bazel:rust.bzl", "rust_library", "rust_test") rust_test( @@ -7,27 +7,25 @@ rust_test( deps = [":cxx_test_suite"], ) +bridge_srcs = [ + "ffi/alias.rs", + "ffi/alias2.rs", + "ffi/lib.rs", +] + rust_library( name = "cxx_test_suite", - srcs = [ - "ffi/alias.rs", - "ffi/alias2.rs", - "ffi/lib.rs", - ], + srcs = bridge_srcs, deps = [ ":impl", "//:cxx", ], ) -cxx_cc_library( +cxxbridge_cc_library( name = "impl", crate_name = "cxx-test-suite", - bridge_srcs = [ - "ffi/alias.rs", - "ffi/alias2.rs", - "ffi/lib.rs", - ], + bridge_srcs = bridge_srcs, srcs = ["ffi/tests.cc"], hdrs = ["ffi/tests.h"], strip_include_prefix = "ffi", diff --git a/tools/bazel/cxx_cc_library.bzl b/tools/bazel/cxxbridge_cc_library.bzl similarity index 82% rename from tools/bazel/cxx_cc_library.bzl rename to tools/bazel/cxxbridge_cc_library.bzl index 362c04bda..f158c8644 100644 --- a/tools/bazel/cxx_cc_library.bzl +++ b/tools/bazel/cxxbridge_cc_library.bzl @@ -1,8 +1,8 @@ -"""cxx_cc_library macro""" +"""cxxbridge_cc_library macro""" load("@rules_cc//cc:defs.bzl", "cc_library") -def cxx_cc_library( +def cxxbridge_cc_library( name, crate_name, bridge_srcs, @@ -17,10 +17,10 @@ def cxx_cc_library( name: A unique name for the cc_library rule. crate_name: The name of the corresponding Rust crate, used as the include_prefix. bridge_srcs: One or more Rust source files containing cxx::bridge modules. - srcs: Forwarded to cc_library unmodified. - hdrs: Forwarded to cc_library unmodified. - deps: Forwarded to cc_library unmodified - strip_include_prefix: Forwarded to cc_library unmodified + srcs: Additional sources. Forwarded to cc_library unmodified. + hdrs: Additional headers. Forwarded to cc_library unmodified. + deps: Additional deps. Forwarded to cc_library unmodified. + strip_include_prefix: Forwarded to cc_library unmodified. visibility: Visibility for all rules generated by this macro. """ @@ -28,8 +28,8 @@ def cxx_cc_library( gen_hdrs = [] for bridge_source in bridge_srcs: source_basename = bridge_source.rsplit("/", 1)[-1] - header_target = source_basename + "-gen-header" - source_target = source_basename + "-gen-source" + header_target = name + "-" + source_basename + "-gen-header" + source_target = name + "-" + source_basename + "-gen-source" header_out = source_basename + ".h" if strip_include_prefix: diff --git a/tools/buck/cxx_cxx_library.bzl b/tools/buck/cxxbridge_cxx_library.bzl similarity index 75% rename from tools/buck/cxx_cxx_library.bzl rename to tools/buck/cxxbridge_cxx_library.bzl index 58c60f5d8..e747cfe36 100644 --- a/tools/buck/cxx_cxx_library.bzl +++ b/tools/buck/cxxbridge_cxx_library.bzl @@ -1,6 +1,6 @@ -"""cxx_cxx_library macro""" +"""cxxbridge_cxx_library macro""" -def cxx_cxx_library( +def cxxbridge_cxx_library( name, crate_name, bridge_srcs, @@ -8,15 +8,15 @@ def cxx_cxx_library( headers = None, deps = None, visibility = None): - """Generate C++ source for one or more Rust cxx::bridges and bundle into a cc_library. + """Generate C++ source for one or more Rust cxx::bridges and bundle into a cxx_library. Args: name: A unique name for the cc_library rule. crate_name: The name of the corresponding Rust crate, used as the include_prefix. bridge_srcs: One or more Rust source files containing cxx::bridge modules. - srcs: Forwarded to cc_library unmodified. - headers: Forwarded to cc_library unmodified. - deps: Forwarded to cc_library unmodified + srcs: Additional sources. Forwarded to cxx_library unmodified. + headers: Additional headers. Forwarded to cxx_library unmodified. + deps: Additional deps. Forwarded to cxx_library unmodified. visibility: Visibility for all rules generated by this macro. """ @@ -24,11 +24,11 @@ def cxx_cxx_library( gen_hdrs = {} for bridge_source in bridge_srcs: source_basename = bridge_source.rsplit("/", 1)[-1] - header_target = source_basename + "-gen-header" - source_target = source_basename + "-gen-source" + header_target = name + "-" + source_basename + "-gen-header" + source_target = name + "-" + source_basename + "-gen-source" hdr_out = source_basename + ".h" - native.genrule( + genrule( name = header_target, srcs = [bridge_source], out = hdr_out, @@ -36,7 +36,7 @@ def cxx_cxx_library( visibility = visibility, ) - native.genrule( + genrule( name = source_target, srcs = [bridge_source], out = source_basename + ".cc",