From 11fd6d0f1bd377ab6f3a774a6cb4a564b0a35f88 Mon Sep 17 00:00:00 2001 From: David Tolnay Date: Sat, 3 Oct 2020 19:54:56 -0700 Subject: [PATCH 1/3] An unordered set suffices for required_trivial_aliases --- syntax/types.rs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/syntax/types.rs b/syntax/types.rs index 8ecc417aa..7eb22d4f9 100644 --- a/syntax/types.rs +++ b/syntax/types.rs @@ -14,7 +14,7 @@ pub struct Types<'a> { pub rust: Set<&'a Ident>, pub aliases: Map<&'a Ident, &'a TypeAlias>, pub untrusted: Map<&'a Ident, &'a ExternType>, - pub required_trivial_aliases: Set<&'a Ident>, + pub required_trivial_aliases: UnorderedSet<&'a Ident>, } impl<'a> Types<'a> { @@ -140,7 +140,7 @@ impl<'a> Types<'a> { // we check that this is permissible. We do this _after_ scanning all // the APIs above, in case some function or struct references a type // which is declared subsequently. - let mut required_trivial_aliases = Set::new(); + let mut required_trivial_aliases = UnorderedSet::new(); let mut insist_alias_types_are_trivial = |ty: &'a Type| { if let Type::Ident(ident) = ty { if aliases.contains_key(ident) { From b5aca7b8a7ee7b88534d39657beefabd8bed9a01 Mon Sep 17 00:00:00 2001 From: David Tolnay Date: Sat, 3 Oct 2020 19:43:18 -0700 Subject: [PATCH 2/3] Collect reason that each alias is required trivial --- gen/src/write.rs | 4 ++-- macro/src/expand.rs | 2 +- syntax/check.rs | 4 ++-- syntax/types.rs | 26 ++++++++++++++++++-------- 4 files changed, 23 insertions(+), 13 deletions(-) diff --git a/gen/src/write.rs b/gen/src/write.rs index ddde8a913..1f4b7bc12 100644 --- a/gen/src/write.rs +++ b/gen/src/write.rs @@ -81,7 +81,7 @@ pub(super) fn gen( } } Api::TypeAlias(ety) => { - if types.required_trivial_aliases.contains(&ety.ident) { + if types.required_trivial_aliases.contains_key(&ety.ident) { check_trivial_extern_type(out, &ety.ident) } } @@ -136,7 +136,7 @@ fn write_includes(out: &mut OutFile, types: &Types) { Some(CxxString) => out.include.string = true, Some(Bool) | Some(Isize) | Some(F32) | Some(F64) | Some(RustString) => {} None => { - if types.required_trivial_aliases.contains(&ident) { + if types.required_trivial_aliases.contains_key(&ident) { out.include.type_traits = true; } } diff --git a/macro/src/expand.rs b/macro/src/expand.rs index 02817a4c9..739529fe0 100644 --- a/macro/src/expand.rs +++ b/macro/src/expand.rs @@ -683,7 +683,7 @@ fn expand_type_alias_verify( const _: fn() = #begin #ident, #type_id #end; }; - if types.required_trivial_aliases.contains(&alias.ident) { + if types.required_trivial_aliases.contains_key(&alias.ident) { let begin = quote_spanned!(begin_span=> ::cxx::private::verify_extern_kind::<); verify.extend(quote! { const _: fn() = #begin #ident, ::cxx::kind::Trivial #end; diff --git a/syntax/check.rs b/syntax/check.rs index cfac23647..15439180e 100644 --- a/syntax/check.rs +++ b/syntax/check.rs @@ -338,7 +338,7 @@ fn is_unsized(cx: &mut Check, ty: &Type) -> bool { || cx.types.cxx.contains(ident) && !cx.types.structs.contains_key(ident) && !cx.types.enums.contains_key(ident) - && !cx.types.required_trivial_aliases.contains(ident) + && !cx.types.required_trivial_aliases.contains_key(ident) || cx.types.rust.contains(ident) } @@ -377,7 +377,7 @@ fn describe(cx: &mut Check, ty: &Type) -> String { } else if cx.types.enums.contains_key(ident) { "enum".to_owned() } else if cx.types.cxx.contains(ident) { - if cx.types.required_trivial_aliases.contains(ident) { + if cx.types.required_trivial_aliases.contains_key(ident) { "trivial C++ type".to_owned() } else { "non-trivial C++ type".to_owned() diff --git a/syntax/types.rs b/syntax/types.rs index 7eb22d4f9..efccb2b9d 100644 --- a/syntax/types.rs +++ b/syntax/types.rs @@ -1,7 +1,7 @@ use crate::syntax::atom::Atom::{self, *}; use crate::syntax::report::Errors; use crate::syntax::set::OrderedSet as Set; -use crate::syntax::{Api, Derive, Enum, ExternType, Struct, Type, TypeAlias}; +use crate::syntax::{Api, Derive, Enum, ExternFn, ExternType, Struct, Type, TypeAlias}; use proc_macro2::Ident; use quote::ToTokens; use std::collections::{BTreeMap as Map, HashSet as UnorderedSet}; @@ -14,7 +14,7 @@ pub struct Types<'a> { pub rust: Set<&'a Ident>, pub aliases: Map<&'a Ident, &'a TypeAlias>, pub untrusted: Map<&'a Ident, &'a ExternType>, - pub required_trivial_aliases: UnorderedSet<&'a Ident>, + pub required_trivial_aliases: Map<&'a Ident, TrivialReason<'a>>, } impl<'a> Types<'a> { @@ -140,27 +140,30 @@ impl<'a> Types<'a> { // we check that this is permissible. We do this _after_ scanning all // the APIs above, in case some function or struct references a type // which is declared subsequently. - let mut required_trivial_aliases = UnorderedSet::new(); - let mut insist_alias_types_are_trivial = |ty: &'a Type| { + let mut required_trivial_aliases = Map::new(); + let mut insist_alias_types_are_trivial = |ty: &'a Type, reason| { if let Type::Ident(ident) = ty { if aliases.contains_key(ident) { - required_trivial_aliases.insert(ident); + required_trivial_aliases.entry(ident).or_insert(reason); } } }; for api in apis { match api { Api::Struct(strct) => { + let reason = TrivialReason::StructField(strct); for field in &strct.fields { - insist_alias_types_are_trivial(&field.ty); + insist_alias_types_are_trivial(&field.ty, reason); } } Api::CxxFunction(efn) | Api::RustFunction(efn) => { + let reason = TrivialReason::FunctionArgument(efn); for arg in &efn.args { - insist_alias_types_are_trivial(&arg.ty); + insist_alias_types_are_trivial(&arg.ty, reason); } if let Some(ret) = &efn.ret { - insist_alias_types_are_trivial(&ret); + let reason = TrivialReason::FunctionReturn(efn); + insist_alias_types_are_trivial(&ret, reason); } } _ => {} @@ -211,6 +214,13 @@ impl<'t, 'a> IntoIterator for &'t Types<'a> { } } +#[derive(Copy, Clone)] +pub enum TrivialReason<'a> { + StructField(&'a Struct), + FunctionArgument(&'a ExternFn), + FunctionReturn(&'a ExternFn), +} + fn duplicate_name(cx: &mut Errors, sp: impl ToTokens, ident: &Ident) { let msg = format!("the name `{}` is defined multiple times", ident); cx.error(sp, msg); From 3208fd7a582bee72046b821dae191bac2026bcf0 Mon Sep 17 00:00:00 2001 From: David Tolnay Date: Sat, 3 Oct 2020 20:19:40 -0700 Subject: [PATCH 3/3] Provide more helpful error when opaque C++ type used by value --- gen/src/write.rs | 6 ++++-- macro/src/expand.rs | 2 +- syntax/check.rs | 25 +++++++++++++++++++------ syntax/types.rs | 10 +++++----- tests/ui/by_value_not_supported.stderr | 12 +++++++++--- 5 files changed, 38 insertions(+), 17 deletions(-) diff --git a/gen/src/write.rs b/gen/src/write.rs index 1f4b7bc12..3c81dc889 100644 --- a/gen/src/write.rs +++ b/gen/src/write.rs @@ -81,7 +81,7 @@ pub(super) fn gen( } } Api::TypeAlias(ety) => { - if types.required_trivial_aliases.contains_key(&ety.ident) { + if types.required_trivial.contains_key(&ety.ident) { check_trivial_extern_type(out, &ety.ident) } } @@ -136,7 +136,9 @@ fn write_includes(out: &mut OutFile, types: &Types) { Some(CxxString) => out.include.string = true, Some(Bool) | Some(Isize) | Some(F32) | Some(F64) | Some(RustString) => {} None => { - if types.required_trivial_aliases.contains_key(&ident) { + if types.aliases.contains_key(ident) + && types.required_trivial.contains_key(ident) + { out.include.type_traits = true; } } diff --git a/macro/src/expand.rs b/macro/src/expand.rs index 739529fe0..e7b28b984 100644 --- a/macro/src/expand.rs +++ b/macro/src/expand.rs @@ -683,7 +683,7 @@ fn expand_type_alias_verify( const _: fn() = #begin #ident, #type_id #end; }; - if types.required_trivial_aliases.contains_key(&alias.ident) { + if types.required_trivial.contains_key(&alias.ident) { let begin = quote_spanned!(begin_span=> ::cxx::private::verify_extern_kind::<); verify.extend(quote! { const _: fn() = #begin #ident, ::cxx::kind::Trivial #end; diff --git a/syntax/check.rs b/syntax/check.rs index 15439180e..4960111c1 100644 --- a/syntax/check.rs +++ b/syntax/check.rs @@ -1,6 +1,7 @@ use crate::syntax::atom::Atom::{self, *}; 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, @@ -208,6 +209,19 @@ fn check_api_enum(cx: &mut Check, enm: &Enum) { fn check_api_type(cx: &mut Check, ty: &ExternType) { check_reserved_name(cx, &ty.ident); + + if let Some(reason) = cx.types.required_trivial.get(&ty.ident) { + let what = match reason { + TrivialReason::StructField(strct) => format!("a field of `{}`", strct.ident), + TrivialReason::FunctionArgument(efn) => format!("an argument of `{}`", efn.ident), + TrivialReason::FunctionReturn(efn) => format!("a return value of `{}`", efn.ident), + }; + let msg = format!( + "needs a cxx::ExternType impl in order to be used as {}", + what, + ); + cx.error(ty, msg); + } } fn check_api_fn(cx: &mut Check, efn: &ExternFn) { @@ -338,7 +352,8 @@ fn is_unsized(cx: &mut Check, ty: &Type) -> bool { || cx.types.cxx.contains(ident) && !cx.types.structs.contains_key(ident) && !cx.types.enums.contains_key(ident) - && !cx.types.required_trivial_aliases.contains_key(ident) + && !(cx.types.aliases.contains_key(ident) + && cx.types.required_trivial.contains_key(ident)) || cx.types.rust.contains(ident) } @@ -376,12 +391,10 @@ fn describe(cx: &mut Check, ty: &Type) -> String { "struct".to_owned() } else if cx.types.enums.contains_key(ident) { "enum".to_owned() + } else if cx.types.aliases.contains_key(ident) { + "C++ type".to_owned() } else if cx.types.cxx.contains(ident) { - if cx.types.required_trivial_aliases.contains_key(ident) { - "trivial C++ type".to_owned() - } else { - "non-trivial C++ type".to_owned() - } + "opaque C++ type".to_owned() } else if cx.types.rust.contains(ident) { "opaque Rust type".to_owned() } else if Atom::from(ident) == Some(CxxString) { diff --git a/syntax/types.rs b/syntax/types.rs index efccb2b9d..6924a780b 100644 --- a/syntax/types.rs +++ b/syntax/types.rs @@ -14,7 +14,7 @@ pub struct Types<'a> { pub rust: Set<&'a Ident>, pub aliases: Map<&'a Ident, &'a TypeAlias>, pub untrusted: Map<&'a Ident, &'a ExternType>, - pub required_trivial_aliases: Map<&'a Ident, TrivialReason<'a>>, + pub required_trivial: Map<&'a Ident, TrivialReason<'a>>, } impl<'a> Types<'a> { @@ -140,11 +140,11 @@ impl<'a> Types<'a> { // we check that this is permissible. We do this _after_ scanning all // the APIs above, in case some function or struct references a type // which is declared subsequently. - let mut required_trivial_aliases = Map::new(); + let mut required_trivial = Map::new(); let mut insist_alias_types_are_trivial = |ty: &'a Type, reason| { if let Type::Ident(ident) = ty { - if aliases.contains_key(ident) { - required_trivial_aliases.entry(ident).or_insert(reason); + if cxx.contains(ident) { + required_trivial.entry(ident).or_insert(reason); } } }; @@ -178,7 +178,7 @@ impl<'a> Types<'a> { rust, aliases, untrusted, - required_trivial_aliases, + required_trivial, } } diff --git a/tests/ui/by_value_not_supported.stderr b/tests/ui/by_value_not_supported.stderr index e61ce8427..0a56dd48f 100644 --- a/tests/ui/by_value_not_supported.stderr +++ b/tests/ui/by_value_not_supported.stderr @@ -1,4 +1,4 @@ -error: using non-trivial C++ type by value is not supported +error: using opaque C++ type by value is not supported --> $DIR/by_value_not_supported.rs:4:9 | 4 | c: C, @@ -16,13 +16,19 @@ error: using C++ string by value is not supported 6 | s: CxxString, | ^^^^^^^^^^^^ -error: passing non-trivial C++ type by value is not supported +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; + | ^^^^^^ + +error: passing opaque C++ type by value is not supported --> $DIR/by_value_not_supported.rs:16:14 | 16 | fn f(c: C) -> C; | ^^^^ -error: returning non-trivial C++ type by value is not supported +error: returning opaque C++ type by value is not supported --> $DIR/by_value_not_supported.rs:16:23 | 16 | fn f(c: C) -> C;