From 7e69f89714717d7f6ca8ea3d2056035d860fd6cb Mon Sep 17 00:00:00 2001 From: David Tolnay Date: Sat, 3 Oct 2020 22:20:22 -0700 Subject: [PATCH 1/5] Explicitly requesting an instantiation --- gen/src/write.rs | 8 ++++++-- macro/src/expand.rs | 10 +++++++--- syntax/check.rs | 17 +++++++++++++++-- syntax/file.rs | 6 ++++-- syntax/ident.rs | 2 +- syntax/mod.rs | 7 +++++++ syntax/parse.rs | 45 ++++++++++++++++++++++++++++++++++++++++----- syntax/tokens.rs | 12 ++++++++++-- syntax/types.rs | 7 +++++++ 9 files changed, 97 insertions(+), 17 deletions(-) diff --git a/gen/src/write.rs b/gen/src/write.rs index 4276e9b09..b83611a3e 100644 --- a/gen/src/write.rs +++ b/gen/src/write.rs @@ -1056,14 +1056,18 @@ fn write_generic_instantiations(out: &mut OutFile, types: &Types) { } } else if let Type::UniquePtr(ptr) = ty { if let Type::Ident(inner) = &ptr.inner { - if Atom::from(inner).is_none() && !types.aliases.contains_key(inner) { + if Atom::from(inner).is_none() + && (!types.aliases.contains_key(inner) || types.explicit_impls.contains(ty)) + { out.next_section(); write_unique_ptr(out, inner, types); } } } else if let Type::CxxVector(ptr) = ty { if let Type::Ident(inner) = &ptr.inner { - if Atom::from(inner).is_none() && !types.aliases.contains_key(inner) { + if Atom::from(inner).is_none() + && (!types.aliases.contains_key(inner) || types.explicit_impls.contains(ty)) + { out.next_section(); write_cxx_vector(out, ty, inner, types); } diff --git a/macro/src/expand.rs b/macro/src/expand.rs index e7b28b984..4778b14cf 100644 --- a/macro/src/expand.rs +++ b/macro/src/expand.rs @@ -39,7 +39,7 @@ fn expand(ffi: Module, apis: &[Api], types: &Types) -> TokenStream { for api in apis { match api { - Api::Include(_) | Api::RustType(_) => {} + Api::Include(_) | Api::RustType(_) | Api::Impl(_) => {} Api::Struct(strct) => expanded.extend(expand_struct(strct)), Api::Enum(enm) => expanded.extend(expand_enum(enm)), Api::CxxType(ety) => { @@ -76,13 +76,17 @@ fn expand(ffi: Module, apis: &[Api], types: &Types) -> TokenStream { } } else if let Type::UniquePtr(ptr) = ty { if let Type::Ident(ident) = &ptr.inner { - if Atom::from(ident).is_none() && !types.aliases.contains_key(ident) { + if Atom::from(ident).is_none() + && (!types.aliases.contains_key(ident) || types.explicit_impls.contains(ty)) + { expanded.extend(expand_unique_ptr(namespace, ident, types)); } } } else if let Type::CxxVector(ptr) = ty { if let Type::Ident(ident) = &ptr.inner { - if Atom::from(ident).is_none() && !types.aliases.contains_key(ident) { + if Atom::from(ident).is_none() + && (!types.aliases.contains_key(ident) || types.explicit_impls.contains(ty)) + { // Generate impl for CxxVector if T is a struct or opaque // C++ type. Impl for primitives is already provided by cxx // crate. diff --git a/syntax/check.rs b/syntax/check.rs index f25694838..ff23ec9be 100644 --- a/syntax/check.rs +++ b/syntax/check.rs @@ -3,8 +3,8 @@ use crate::syntax::namespace::Namespace; use crate::syntax::report::Errors; use crate::syntax::types::TrivialReason; use crate::syntax::{ - error, ident, Api, Enum, ExternFn, ExternType, Lang, Receiver, Ref, Slice, Struct, Ty1, Type, - Types, + error, ident, Api, Enum, ExternFn, ExternType, Impl, Lang, Receiver, Ref, Slice, Struct, Ty1, + Type, Types, }; use proc_macro2::{Delimiter, Group, Ident, TokenStream}; use quote::{quote, ToTokens}; @@ -48,6 +48,7 @@ fn do_typecheck(cx: &mut Check) { Api::Enum(enm) => check_api_enum(cx, enm), Api::CxxType(ety) | Api::RustType(ety) => check_api_type(cx, ety), Api::CxxFunction(efn) | Api::RustFunction(efn) => check_api_fn(cx, efn), + Api::Impl(imp) => check_api_impl(cx, imp), _ => {} } } @@ -286,6 +287,18 @@ fn check_api_fn(cx: &mut Check, efn: &ExternFn) { check_multiple_arg_lifetimes(cx, efn); } +fn check_api_impl(cx: &mut Check, imp: &Impl) { + if let Type::UniquePtr(ty) | Type::CxxVector(ty) = &imp.ty { + if let Type::Ident(inner) = &ty.inner { + if Atom::from(inner).is_none() { + return; + } + } + } + + cx.error(imp, "unsupported Self type of explicit impl"); +} + fn check_mut_return_restriction(cx: &mut Check, efn: &ExternFn) { match &efn.ret { Some(Type::Ref(ty)) if ty.mutability.is_some() => {} diff --git a/syntax/file.rs b/syntax/file.rs index 8b86adc73..931ce6e8d 100644 --- a/syntax/file.rs +++ b/syntax/file.rs @@ -2,8 +2,8 @@ use crate::syntax::namespace::Namespace; use quote::quote; use syn::parse::{Error, Parse, ParseStream, Result}; use syn::{ - braced, token, Abi, Attribute, ForeignItem, Ident, Item as RustItem, ItemEnum, ItemStruct, - ItemUse, LitStr, Token, Visibility, + braced, token, Abi, Attribute, ForeignItem, Ident, Item as RustItem, ItemEnum, ItemImpl, + ItemStruct, ItemUse, LitStr, Token, Visibility, }; pub struct Module { @@ -22,6 +22,7 @@ pub enum Item { Enum(ItemEnum), ForeignMod(ItemForeignMod), Use(ItemUse), + Impl(ItemImpl), Other(RustItem), } @@ -99,6 +100,7 @@ impl Parse for Item { brace_token: item.brace_token, items: item.items, })), + RustItem::Impl(item) => Ok(Item::Impl(ItemImpl { attrs, ..item })), RustItem::Use(item) => Ok(Item::Use(ItemUse { attrs, ..item })), other => Ok(Item::Other(other)), } diff --git a/syntax/ident.rs b/syntax/ident.rs index 7545e92ce..74e779903 100644 --- a/syntax/ident.rs +++ b/syntax/ident.rs @@ -20,7 +20,7 @@ pub(crate) fn check_all(cx: &mut Check, namespace: &Namespace, apis: &[Api]) { for api in apis { match api { - Api::Include(_) => {} + Api::Include(_) | Api::Impl(_) => {} Api::Struct(strct) => { check(cx, &strct.ident); for field in &strct.fields { diff --git a/syntax/mod.rs b/syntax/mod.rs index 8ef3c8b9c..934f6c67a 100644 --- a/syntax/mod.rs +++ b/syntax/mod.rs @@ -42,6 +42,7 @@ pub enum Api { RustType(ExternType), RustFunction(ExternFn), TypeAlias(TypeAlias), + Impl(Impl), } pub struct ExternType { @@ -87,6 +88,12 @@ pub struct TypeAlias { pub semi_token: Token![;], } +pub struct Impl { + pub impl_token: Token![impl], + pub ty: Type, + pub brace_token: Brace, +} + pub struct Signature { pub unsafety: Option, pub fn_token: Token![fn], diff --git a/syntax/parse.rs b/syntax/parse.rs index cec818d37..49dc26b5b 100644 --- a/syntax/parse.rs +++ b/syntax/parse.rs @@ -3,17 +3,17 @@ use crate::syntax::file::{Item, ItemForeignMod}; use crate::syntax::report::Errors; use crate::syntax::Atom::*; use crate::syntax::{ - attrs, error, Api, Doc, Enum, ExternFn, ExternType, Lang, Receiver, Ref, Signature, Slice, - Struct, Ty1, Type, TypeAlias, Var, Variant, + attrs, error, Api, Doc, Enum, ExternFn, ExternType, Impl, Lang, Receiver, Ref, Signature, + Slice, Struct, Ty1, Type, TypeAlias, Var, Variant, }; -use proc_macro2::{TokenStream, TokenTree}; +use proc_macro2::{Delimiter, Group, TokenStream, TokenTree}; use quote::{format_ident, quote, quote_spanned}; use syn::parse::{ParseStream, Parser}; use syn::punctuated::Punctuated; use syn::{ Abi, Attribute, Error, Fields, FnArg, ForeignItem, ForeignItemFn, ForeignItemType, - GenericArgument, Ident, ItemEnum, ItemStruct, LitStr, Pat, PathArguments, Result, ReturnType, - Token, Type as RustType, TypeBareFn, TypePath, TypeReference, TypeSlice, + GenericArgument, Ident, ItemEnum, ItemImpl, ItemStruct, LitStr, Pat, PathArguments, Result, + ReturnType, Token, Type as RustType, TypeBareFn, TypePath, TypeReference, TypeSlice, }; pub mod kw { @@ -33,6 +33,10 @@ pub fn parse_items(cx: &mut Errors, items: Vec, trusted: bool) -> Vec Err(err) => cx.push(err), }, Item::ForeignMod(foreign_mod) => parse_foreign_mod(cx, foreign_mod, &mut apis, trusted), + Item::Impl(item) => match parse_impl(item) { + Ok(imp) => apis.push(imp), + Err(err) => cx.push(err), + }, Item::Use(item) => cx.error(item, error::USE_NOT_ALLOWED), Item::Other(item) => cx.error(item, "unsupported item"), } @@ -420,6 +424,37 @@ fn parse_extern_verbatim(cx: &mut Errors, tokens: &TokenStream, lang: Lang) -> R } } +fn parse_impl(imp: ItemImpl) -> Result { + if !imp.items.is_empty() { + let mut span = Group::new(Delimiter::Brace, TokenStream::new()); + span.set_span(imp.brace_token.span); + return Err(Error::new_spanned(span, "expected an empty impl block")); + } + + let self_ty = &imp.self_ty; + if let Some((bang, path, for_token)) = &imp.trait_ { + let span = quote!(#bang #path #for_token #self_ty); + return Err(Error::new_spanned( + span, + "unexpected impl, expected something like `impl UniquePtr {}`", + )); + } + + let generics = &imp.generics; + if !generics.params.is_empty() || generics.where_clause.is_some() { + return Err(Error::new_spanned( + imp, + "generic parameters on an impl is not supported", + )); + } + + Ok(Api::Impl(Impl { + impl_token: imp.impl_token, + ty: parse_type(&self_ty)?, + brace_token: imp.brace_token, + })) +} + fn parse_include(input: ParseStream) -> Result { if input.peek(LitStr) { return Ok(input.parse::()?.value()); diff --git a/syntax/tokens.rs b/syntax/tokens.rs index 4ed264a78..7618e99da 100644 --- a/syntax/tokens.rs +++ b/syntax/tokens.rs @@ -1,7 +1,7 @@ use crate::syntax::atom::Atom::*; use crate::syntax::{ - Atom, Derive, Enum, ExternFn, ExternType, Receiver, Ref, Signature, Slice, Struct, Ty1, Type, - TypeAlias, Var, + Atom, Derive, Enum, ExternFn, ExternType, Impl, Receiver, Ref, Signature, Slice, Struct, Ty1, + Type, TypeAlias, Var, }; use proc_macro2::{Ident, Span, TokenStream}; use quote::{quote_spanned, ToTokens}; @@ -121,6 +121,14 @@ impl ToTokens for ExternFn { } } +impl ToTokens for Impl { + fn to_tokens(&self, tokens: &mut TokenStream) { + self.impl_token.to_tokens(tokens); + self.ty.to_tokens(tokens); + self.brace_token.surround(tokens, |_tokens| {}); + } +} + impl ToTokens for Signature { fn to_tokens(&self, tokens: &mut TokenStream) { self.fn_token.to_tokens(tokens); diff --git a/syntax/types.rs b/syntax/types.rs index 6924a780b..2afce256c 100644 --- a/syntax/types.rs +++ b/syntax/types.rs @@ -15,6 +15,7 @@ pub struct Types<'a> { pub aliases: Map<&'a Ident, &'a TypeAlias>, pub untrusted: Map<&'a Ident, &'a ExternType>, pub required_trivial: Map<&'a Ident, TrivialReason<'a>>, + pub explicit_impls: Set<&'a Type>, } impl<'a> Types<'a> { @@ -26,6 +27,7 @@ impl<'a> Types<'a> { let mut rust = Set::new(); let mut aliases = Map::new(); let mut untrusted = Map::new(); + let mut explicit_impls = Set::new(); fn visit<'a>(all: &mut Set<&'a Type>, ty: &'a Type) { all.insert(ty); @@ -133,6 +135,10 @@ impl<'a> Types<'a> { cxx.insert(ident); aliases.insert(ident, alias); } + Api::Impl(imp) => { + visit(&mut all, &imp.ty); + explicit_impls.insert(&imp.ty); + } } } @@ -179,6 +185,7 @@ impl<'a> Types<'a> { aliases, untrusted, required_trivial, + explicit_impls, } } From ac8394bd1897a1f1880238a4fecc5cf5f45d21c0 Mon Sep 17 00:00:00 2001 From: David Tolnay Date: Sat, 3 Oct 2020 23:32:43 -0700 Subject: [PATCH 2/5] Add test of UniquePtrTarget impl conflict --- tests/ui/unique_ptr_twice.rs | 19 +++++++++++++++++++ tests/ui/unique_ptr_twice.stderr | 10 ++++++++++ 2 files changed, 29 insertions(+) create mode 100644 tests/ui/unique_ptr_twice.rs create mode 100644 tests/ui/unique_ptr_twice.stderr diff --git a/tests/ui/unique_ptr_twice.rs b/tests/ui/unique_ptr_twice.rs new file mode 100644 index 000000000..b6cb4d42b --- /dev/null +++ b/tests/ui/unique_ptr_twice.rs @@ -0,0 +1,19 @@ +#[cxx::bridge] +mod here { + extern "C" { + type C; + } + + impl UniquePtr {} +} + +#[cxx::bridge] +mod there { + extern "C" { + type C = crate::here::C; + } + + impl UniquePtr {} +} + +fn main() {} diff --git a/tests/ui/unique_ptr_twice.stderr b/tests/ui/unique_ptr_twice.stderr new file mode 100644 index 000000000..d77d3ff27 --- /dev/null +++ b/tests/ui/unique_ptr_twice.stderr @@ -0,0 +1,10 @@ +error[E0119]: conflicting implementations of trait `cxx::private::UniquePtrTarget` for type `here::C`: + --> $DIR/unique_ptr_twice.rs:10:1 + | +1 | #[cxx::bridge] + | -------------- first implementation here +... +10 | #[cxx::bridge] + | ^^^^^^^^^^^^^^ conflicting implementation for `here::C` + | + = note: this error originates in an attribute macro (in Nightly builds, run with -Z macro-backtrace for more info) From 50b7563de4e41dace6477d707d33fa5bd4141d85 Mon Sep 17 00:00:00 2001 From: David Tolnay Date: Sun, 4 Oct 2020 00:16:09 -0700 Subject: [PATCH 3/5] Add ui test of explicit impl with unsupported Self --- tests/ui/bad_explicit_impl.rs | 10 ++++++++++ tests/ui/bad_explicit_impl.stderr | 5 +++++ 2 files changed, 15 insertions(+) create mode 100644 tests/ui/bad_explicit_impl.rs create mode 100644 tests/ui/bad_explicit_impl.stderr diff --git a/tests/ui/bad_explicit_impl.rs b/tests/ui/bad_explicit_impl.rs new file mode 100644 index 000000000..210644661 --- /dev/null +++ b/tests/ui/bad_explicit_impl.rs @@ -0,0 +1,10 @@ +#[cxx::bridge] +mod ffi { + struct S { + x: u8, + } + + impl fn() -> &S {} +} + +fn main() {} diff --git a/tests/ui/bad_explicit_impl.stderr b/tests/ui/bad_explicit_impl.stderr new file mode 100644 index 000000000..cd0a31738 --- /dev/null +++ b/tests/ui/bad_explicit_impl.stderr @@ -0,0 +1,5 @@ +error: unsupported Self type of explicit impl + --> $DIR/bad_explicit_impl.rs:7:5 + | +7 | impl fn() -> &S {} + | ^^^^^^^^^^^^^^^^^^ From 2b3c2b2fa4ac49ddd382d3dded3fdb9008c4162c Mon Sep 17 00:00:00 2001 From: David Tolnay Date: Sun, 4 Oct 2020 00:18:10 -0700 Subject: [PATCH 4/5] Add ui test of invalid nonempty impl block --- tests/ui/nonempty_impl_block.rs | 12 ++++++++++++ tests/ui/nonempty_impl_block.stderr | 8 ++++++++ 2 files changed, 20 insertions(+) create mode 100644 tests/ui/nonempty_impl_block.rs create mode 100644 tests/ui/nonempty_impl_block.stderr diff --git a/tests/ui/nonempty_impl_block.rs b/tests/ui/nonempty_impl_block.rs new file mode 100644 index 000000000..239d1ece6 --- /dev/null +++ b/tests/ui/nonempty_impl_block.rs @@ -0,0 +1,12 @@ +#[cxx::bridge] +mod ffi { + struct S { + x: u8, + } + + impl UniquePtr { + fn new() -> Self; + } +} + +fn main() {} diff --git a/tests/ui/nonempty_impl_block.stderr b/tests/ui/nonempty_impl_block.stderr new file mode 100644 index 000000000..e7881bb05 --- /dev/null +++ b/tests/ui/nonempty_impl_block.stderr @@ -0,0 +1,8 @@ +error: expected an empty impl block + --> $DIR/nonempty_impl_block.rs:7:23 + | +7 | impl UniquePtr { + | _______________________^ +8 | | fn new() -> Self; +9 | | } + | |_____^ From 64cab486888bc24fc7e4abbaef6d12f74960f0cb Mon Sep 17 00:00:00 2001 From: David Tolnay Date: Sun, 4 Oct 2020 00:19:09 -0700 Subject: [PATCH 5/5] Add ui test of explicit impl trait for type --- tests/ui/impl_trait_for_type.rs | 10 ++++++++++ tests/ui/impl_trait_for_type.stderr | 5 +++++ 2 files changed, 15 insertions(+) create mode 100644 tests/ui/impl_trait_for_type.rs create mode 100644 tests/ui/impl_trait_for_type.stderr diff --git a/tests/ui/impl_trait_for_type.rs b/tests/ui/impl_trait_for_type.rs new file mode 100644 index 000000000..9284f738b --- /dev/null +++ b/tests/ui/impl_trait_for_type.rs @@ -0,0 +1,10 @@ +#[cxx::bridge] +mod ffi { + struct S { + x: u8, + } + + impl UniquePtrTarget for S {} +} + +fn main() {} diff --git a/tests/ui/impl_trait_for_type.stderr b/tests/ui/impl_trait_for_type.stderr new file mode 100644 index 000000000..e05a461e0 --- /dev/null +++ b/tests/ui/impl_trait_for_type.stderr @@ -0,0 +1,5 @@ +error: unexpected impl, expected something like `impl UniquePtr {}` + --> $DIR/impl_trait_for_type.rs:7:10 + | +7 | impl UniquePtrTarget for S {} + | ^^^^^^^^^^^^^^^^^^^^^