Join trivial reasons in one message
diff --git a/gen/src/write.rs b/gen/src/write.rs
index 434ca17..315fb35 100644
--- a/gen/src/write.rs
+++ b/gen/src/write.rs
@@ -4,10 +4,10 @@
use crate::gen::{builtin, include, Opt};
use crate::syntax::atom::Atom::{self, *};
use crate::syntax::symbol::Symbol;
-use crate::syntax::trivial::TrivialReason;
+use crate::syntax::trivial::{self, TrivialReason};
use crate::syntax::{
derive, mangle, Api, Enum, ExternFn, ExternType, Pair, RustName, Signature, Struct, Trait,
- Type, Types, Var,
+ Type, TypeAlias, Types, Var,
};
use proc_macro2::Ident;
use std::collections::{HashMap, HashSet};
@@ -119,7 +119,7 @@
for api in apis {
if let Api::TypeAlias(ety) = api {
if let Some(reasons) = out.types.required_trivial.get(&ety.name.rust) {
- check_trivial_extern_type(out, &ety.name, reasons)
+ check_trivial_extern_type(out, ety, reasons)
}
}
}
@@ -370,7 +370,7 @@
}
}
-fn check_trivial_extern_type(out: &mut OutFile, id: &Pair, reasons: &[TrivialReason]) {
+fn check_trivial_extern_type(out: &mut OutFile, alias: &TypeAlias, reasons: &[TrivialReason]) {
// NOTE: The following static assertion is just nice-to-have and not
// necessary for soundness. That's because triviality is always declared by
// the user in the form of an unsafe impl of cxx::ExternType:
@@ -404,18 +404,16 @@
// + struct rust::IsRelocatable<MyType> : std::true_type {};
//
- let id = id.to_fully_qualified();
+ let id = alias.name.to_fully_qualified();
out.builtin.relocatable = true;
- for reason in reasons {
- writeln!(out, "static_assert(");
- writeln!(out, " ::rust::IsRelocatable<{}>::value,", id);
- writeln!(
- out,
- " \"type {} is not move constructible and trivially destructible in C++ yet is used as a trivial type in Rust ({})\");",
- id.trim_start_matches("::"),
- reason,
- );
- }
+ writeln!(out, "static_assert(");
+ writeln!(out, " ::rust::IsRelocatable<{}>::value,", id);
+ writeln!(
+ out,
+ " \"type {} should be trivially move constructible and trivially destructible in C++ to be used as {} in Rust\");",
+ id.trim_start_matches("::"),
+ trivial::as_what(&alias.name, reasons),
+ );
}
fn write_struct_operator_decls<'a>(out: &mut OutFile<'a>, strct: &'a Struct) {
diff --git a/syntax/check.rs b/syntax/check.rs
index 32d147b..d0b0ba0 100644
--- a/syntax/check.rs
+++ b/syntax/check.rs
@@ -1,8 +1,8 @@
use crate::syntax::atom::Atom::{self, *};
use crate::syntax::report::Errors;
use crate::syntax::{
- error, ident, Api, Array, Enum, ExternFn, ExternType, Impl, Lang, Receiver, Ref, Signature,
- SliceRef, Struct, Trait, Ty1, Type, TypeAlias, Types,
+ error, ident, trivial, Api, Array, Enum, ExternFn, ExternType, Impl, Lang, Receiver, Ref,
+ Signature, SliceRef, Struct, Trait, Ty1, Type, TypeAlias, Types,
};
use proc_macro2::{Delimiter, Group, Ident, TokenStream};
use quote::{quote, ToTokens};
@@ -325,14 +325,11 @@
}
if let Some(reasons) = cx.types.required_trivial.get(&ety.name.rust) {
- for reason in reasons {
- let what = reason.describe_in_context(&ety);
- let msg = format!(
- "needs a cxx::ExternType impl in order to be used as {}",
- what,
- );
- cx.error(ety, msg);
- }
+ let msg = format!(
+ "needs a cxx::ExternType impl in order to be used as {}",
+ trivial::as_what(&ety.name, reasons),
+ );
+ cx.error(ety, msg);
}
}
diff --git a/syntax/set.rs b/syntax/set.rs
index 0618564..0df5f87 100644
--- a/syntax/set.rs
+++ b/syntax/set.rs
@@ -50,11 +50,21 @@
}
}
+ impl<'a, T> OrderedSet<&'a T> {
+ pub fn is_empty(&self) -> bool {
+ self.vec.is_empty()
+ }
+
+ pub fn iter(&self) -> Iter<'_, 'a, T> {
+ Iter(self.vec.iter())
+ }
+ }
+
impl<'s, 'a, T> IntoIterator for &'s OrderedSet<&'a T> {
type Item = &'a T;
type IntoIter = Iter<'s, 'a, T>;
fn into_iter(self) -> Self::IntoIter {
- Iter(self.vec.iter())
+ self.iter()
}
}
}
diff --git a/syntax/trivial.rs b/syntax/trivial.rs
index 7a77937..85184d8 100644
--- a/syntax/trivial.rs
+++ b/syntax/trivial.rs
@@ -1,8 +1,8 @@
use crate::syntax::set::OrderedSet as Set;
-use crate::syntax::{Api, Enum, ExternFn, ExternType, RustName, Struct, Type};
+use crate::syntax::{Api, Enum, ExternFn, Pair, RustName, Struct, Type};
use proc_macro2::Ident;
use std::collections::BTreeMap as Map;
-use std::fmt::Display;
+use std::fmt::{self, Display};
#[derive(Copy, Clone)]
pub enum TrivialReason<'a> {
@@ -101,31 +101,146 @@
required_trivial
}
-impl<'a> TrivialReason<'a> {
- pub fn describe_in_context(&self, ety: &ExternType) -> String {
- match self {
- TrivialReason::BoxTarget => format!("Box<{}>", ety.name.rust),
- TrivialReason::VecElement => format!("a vector element in Vec<{}>", ety.name.rust),
- _ => self.to_string(),
- }
+// Context:
+// "type {type} should be trivially move constructible and trivially destructible in C++ to be used as {what} in Rust"
+// "needs a cxx::ExternType impl in order to be used as {what}"
+pub fn as_what<'a>(name: &'a Pair, reasons: &'a [TrivialReason]) -> impl Display + 'a {
+ struct Description<'a> {
+ name: &'a Pair,
+ reasons: &'a [TrivialReason<'a>],
}
-}
-impl<'a> Display for TrivialReason<'a> {
- fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result {
- match self {
- TrivialReason::StructField(strct) => write!(f, "a field of `{}`", strct.name.rust),
- TrivialReason::FunctionArgument(efn) => write!(f, "an argument of `{}`", efn.name.rust),
- TrivialReason::FunctionReturn(efn) => {
- write!(f, "a return value of `{}`", efn.name.rust)
+ impl<'a> Display for Description<'a> {
+ fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result {
+ let mut field_of = Set::new();
+ let mut argument_of = Set::new();
+ let mut return_of = Set::new();
+ let mut box_target = false;
+ let mut vec_element = false;
+ let mut unpinned_mut = Set::new();
+
+ for reason in self.reasons {
+ match reason {
+ TrivialReason::StructField(strct) => {
+ field_of.insert(&strct.name.rust);
+ }
+ TrivialReason::FunctionArgument(efn) => {
+ argument_of.insert(&efn.name.rust);
+ }
+ TrivialReason::FunctionReturn(efn) => {
+ return_of.insert(&efn.name.rust);
+ }
+ TrivialReason::BoxTarget => box_target = true,
+ TrivialReason::VecElement => vec_element = true,
+ TrivialReason::UnpinnedMutArg(efn) => {
+ unpinned_mut.insert(&efn.name.rust);
+ }
+ }
}
- TrivialReason::BoxTarget => write!(f, "in a Box<...>"),
- TrivialReason::VecElement => write!(f, "a Vec<...> element"),
- TrivialReason::UnpinnedMutArg(efn) => write!(
- f,
- "a non-pinned mutable reference argument of {}",
- efn.name.rust
- ),
+
+ let mut clauses = Vec::new();
+ if !field_of.is_empty() {
+ clauses.push(Clause::Set {
+ article: "a",
+ desc: "field of",
+ set: &field_of,
+ });
+ }
+ if !argument_of.is_empty() {
+ clauses.push(Clause::Set {
+ article: "an",
+ desc: "argument of",
+ set: &argument_of,
+ });
+ }
+ if !return_of.is_empty() {
+ clauses.push(Clause::Set {
+ article: "a",
+ desc: "return value of",
+ set: &return_of,
+ });
+ }
+ if box_target {
+ clauses.push(Clause::Ty1 {
+ article: "type",
+ desc: "Box",
+ param: self.name,
+ });
+ }
+ if vec_element {
+ clauses.push(Clause::Ty1 {
+ article: "a",
+ desc: "vector element in Vec",
+ param: self.name,
+ });
+ }
+ if !unpinned_mut.is_empty() {
+ clauses.push(Clause::Set {
+ article: "a",
+ desc: "non-pinned mutable reference argument of",
+ set: &unpinned_mut,
+ });
+ }
+
+ for (i, clause) in clauses.iter().enumerate() {
+ if i == 0 {
+ write!(f, "{} ", clause.article())?;
+ } else if i + 1 < clauses.len() {
+ write!(f, ", ")?;
+ } else {
+ write!(f, " or ")?;
+ }
+ clause.fmt(f)?;
+ }
+
+ Ok(())
}
}
+
+ enum Clause<'a> {
+ Set {
+ article: &'a str,
+ desc: &'a str,
+ set: &'a Set<&'a Ident>,
+ },
+ Ty1 {
+ article: &'a str,
+ desc: &'a str,
+ param: &'a Pair,
+ },
+ }
+
+ impl<'a> Clause<'a> {
+ fn article(&self) -> &'a str {
+ match self {
+ Clause::Set { article, .. } | Clause::Ty1 { article, .. } => article,
+ }
+ }
+
+ fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result {
+ match self {
+ Clause::Set {
+ article: _,
+ desc,
+ set,
+ } => {
+ write!(f, "{} ", desc)?;
+ for (i, ident) in set.iter().take(3).enumerate() {
+ if i > 0 {
+ write!(f, ", ")?;
+ }
+ write!(f, "`{}`", ident)?;
+ }
+ Ok(())
+ }
+ Clause::Ty1 {
+ article: _,
+ desc,
+ param,
+ } => write!(f, "{}<{}>", desc, param.rust),
+ }
+ }
+ }
+
+ Description { name, reasons }
}
diff --git a/tests/ui/by_value_not_supported.stderr b/tests/ui/by_value_not_supported.stderr
index 4bcce1b..7288c93 100644
--- a/tests/ui/by_value_not_supported.stderr
+++ b/tests/ui/by_value_not_supported.stderr
@@ -16,19 +16,7 @@
6 | s: CxxString,
| ^^^^^^^^^^^^
-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: needs a cxx::ExternType impl in order to be used as an argument of `f`
- --> $DIR/by_value_not_supported.rs:10:9
- |
-10 | type C;
- | ^^^^^^
-
-error: needs a cxx::ExternType impl in order to be used as a return value of `f`
+error: needs a cxx::ExternType impl in order to be used as a field of `S`, argument of `f` or return value of `f`
--> $DIR/by_value_not_supported.rs:10:9
|
10 | type C;
diff --git a/tests/ui/pin_mut_opaque.stderr b/tests/ui/pin_mut_opaque.stderr
index 2de4264..53e651e 100644
--- a/tests/ui/pin_mut_opaque.stderr
+++ b/tests/ui/pin_mut_opaque.stderr
@@ -16,19 +16,7 @@
9 | fn v(v: &mut CxxVector<u8>);
| ^^^^^^^^^^^^^^^^^^
-error: needs a cxx::ExternType impl in order to be used as a non-pinned mutable reference argument of f
- --> $DIR/pin_mut_opaque.rs:4:9
- |
-4 | type Opaque;
- | ^^^^^^^^^^^
-
-error: needs a cxx::ExternType impl in order to be used as a non-pinned mutable reference argument of g
- --> $DIR/pin_mut_opaque.rs:4:9
- |
-4 | type Opaque;
- | ^^^^^^^^^^^
-
-error: needs a cxx::ExternType impl in order to be used as a non-pinned mutable reference argument of h
+error: needs a cxx::ExternType impl in order to be used as a non-pinned mutable reference argument of `f`, `g`, `h`
--> $DIR/pin_mut_opaque.rs:4:9
|
4 | type Opaque;
