Merge pull request #335 from dtolnay/kind

Add dedicated error message referring to cxx::ExternType when opaque type is used by value

Author: dtolnay

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.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(&ident) {
+                    if types.aliases.contains_key(ident)
+                        && types.required_trivial.contains_key(ident)
+                    {
                         out.include.type_traits = true;
                     }
                 }

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.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;

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(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(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) {

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: Set<&'a Ident>,
+    pub required_trivial: 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 = Set::new();
-        let mut insist_alias_types_are_trivial = |ty: &'a Type| {
+        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.insert(ident);
+                if cxx.contains(ident) {
+                    required_trivial.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);
                     }
                 }
                 _ => {}
@@ -175,7 +178,7 @@ impl<'a> Types<'a> {
             rust,
             aliases,
             untrusted,
-            required_trivial_aliases,
+            required_trivial,
         }
     }
 
@@ -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);

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;