Skip to content

Improve error message on a conflicting explicit impl #337

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 2 commits into from
Oct 4, 2020
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
46 changes: 34 additions & 12 deletions macro/src/expand.rs
Original file line number Diff line number Diff line change
Expand Up @@ -4,9 +4,10 @@ use crate::syntax::namespace::Namespace;
use crate::syntax::report::Errors;
use crate::syntax::symbol::Symbol;
use crate::syntax::{
self, check, mangle, Api, Enum, ExternFn, ExternType, Signature, Struct, Type, TypeAlias, Types,
self, check, mangle, Api, Enum, ExternFn, ExternType, Impl, Signature, Struct, Type, TypeAlias,
Types,
};
use proc_macro2::{Ident, TokenStream};
use proc_macro2::{Ident, Span, TokenStream};
use quote::{format_ident, quote, quote_spanned, ToTokens};
use std::mem;
use syn::{parse_quote, Result, Token};
Expand Down Expand Up @@ -62,6 +63,7 @@ fn expand(ffi: Module, apis: &[Api], types: &Types) -> TokenStream {
}

for ty in types {
let explicit_impl = types.explicit_impls.get(ty);
if let Type::RustBox(ty) = ty {
if let Type::Ident(ident) = &ty.inner {
if Atom::from(ident).is_none() {
Expand All @@ -77,20 +79,20 @@ 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) || types.explicit_impls.contains(ty))
&& (explicit_impl.is_some() || !types.aliases.contains_key(ident))
{
expanded.extend(expand_unique_ptr(namespace, ident, types));
expanded.extend(expand_unique_ptr(namespace, ident, types, explicit_impl));
}
}
} 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) || types.explicit_impls.contains(ty))
&& (explicit_impl.is_some() || !types.aliases.contains_key(ident))
{
// Generate impl for CxxVector<T> if T is a struct or opaque
// C++ type. Impl for primitives is already provided by cxx
// crate.
expanded.extend(expand_cxx_vector(namespace, ident));
expanded.extend(expand_cxx_vector(namespace, ident, explicit_impl));
}
}
}
Expand Down Expand Up @@ -784,7 +786,12 @@ fn expand_rust_vec(namespace: &Namespace, elem: &Ident) -> TokenStream {
}
}

fn expand_unique_ptr(namespace: &Namespace, ident: &Ident, types: &Types) -> TokenStream {
fn expand_unique_ptr(
namespace: &Namespace,
ident: &Ident,
types: &Types,
explicit_impl: Option<&Impl>,
) -> TokenStream {
let name = ident.to_string();
let prefix = format!("cxxbridge04$unique_ptr${}{}$", namespace, ident);
let link_null = format!("{}null", prefix);
Expand All @@ -810,8 +817,13 @@ fn expand_unique_ptr(namespace: &Namespace, ident: &Ident, types: &Types) -> Tok
None
};

quote! {
unsafe impl ::cxx::private::UniquePtrTarget for #ident {
let begin_span =
explicit_impl.map_or_else(Span::call_site, |explicit| explicit.impl_token.span);
let end_span = explicit_impl.map_or_else(Span::call_site, |explicit| explicit.brace_token.span);
let unsafe_token = format_ident!("unsafe", span = begin_span);

quote_spanned! {end_span=>
#unsafe_token impl ::cxx::private::UniquePtrTarget for #ident {
const __NAME: &'static dyn ::std::fmt::Display = &#name;
fn __null() -> *mut ::std::ffi::c_void {
extern "C" {
Expand Down Expand Up @@ -857,7 +869,12 @@ fn expand_unique_ptr(namespace: &Namespace, ident: &Ident, types: &Types) -> Tok
}
}

fn expand_cxx_vector(namespace: &Namespace, elem: &Ident) -> TokenStream {
fn expand_cxx_vector(
namespace: &Namespace,
elem: &Ident,
explicit_impl: Option<&Impl>,
) -> TokenStream {
let _ = explicit_impl;
let name = elem.to_string();
let prefix = format!("cxxbridge04$std$vector${}{}$", namespace, elem);
let link_size = format!("{}size", prefix);
Expand All @@ -869,8 +886,13 @@ fn expand_cxx_vector(namespace: &Namespace, elem: &Ident) -> TokenStream {
let link_unique_ptr_release = format!("{}release", unique_ptr_prefix);
let link_unique_ptr_drop = format!("{}drop", unique_ptr_prefix);

quote! {
unsafe impl ::cxx::private::VectorElement for #elem {
let begin_span =
explicit_impl.map_or_else(Span::call_site, |explicit| explicit.impl_token.span);
let end_span = explicit_impl.map_or_else(Span::call_site, |explicit| explicit.brace_token.span);
let unsafe_token = format_ident!("unsafe", span = begin_span);

quote_spanned! {end_span=>
#unsafe_token impl ::cxx::private::VectorElement for #elem {
const __NAME: &'static dyn ::std::fmt::Display = &#name;
fn __vector_size(v: &::cxx::CxxVector<Self>) -> usize {
extern "C" {
Expand Down
38 changes: 37 additions & 1 deletion syntax/impls.rs
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
use crate::syntax::{ExternFn, Receiver, Ref, Signature, Slice, Ty1, Type};
use crate::syntax::{ExternFn, Impl, Receiver, Ref, Signature, Slice, Ty1, Type};
use std::borrow::Borrow;
use std::hash::{Hash, Hasher};
use std::mem;
use std::ops::{Deref, DerefMut};
Expand Down Expand Up @@ -238,3 +239,38 @@ impl Hash for Receiver {
ty.hash(state);
}
}

impl Hash for Impl {
fn hash<H: Hasher>(&self, state: &mut H) {
let Impl {
impl_token: _,
ty,
brace_token: _,
} = self;
ty.hash(state);
}
}

impl Eq for Impl {}

impl PartialEq for Impl {
fn eq(&self, other: &Impl) -> bool {
let Impl {
impl_token: _,
ty,
brace_token: _,
} = self;
let Impl {
impl_token: _,
ty: ty2,
brace_token: _,
} = other;
ty == ty2
}
}

impl Borrow<Type> for &Impl {
fn borrow(&self) -> &Type {
&self.ty
}
}
8 changes: 8 additions & 0 deletions syntax/set.rs
Original file line number Diff line number Diff line change
Expand Up @@ -35,6 +35,14 @@ where
{
self.set.contains(value)
}

pub fn get<Q>(&self, value: &Q) -> Option<&'a T>
where
&'a T: Borrow<Q>,
Q: ?Sized + Hash + Eq,
{
self.set.get(value).copied()
}
}

impl<'s, 'a, T> IntoIterator for &'s OrderedSet<&'a T> {
Expand Down
6 changes: 3 additions & 3 deletions syntax/types.rs
Original file line number Diff line number Diff line change
@@ -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, ExternFn, ExternType, Struct, Type, TypeAlias};
use crate::syntax::{Api, Derive, Enum, ExternFn, ExternType, Impl, Struct, Type, TypeAlias};
use proc_macro2::Ident;
use quote::ToTokens;
use std::collections::{BTreeMap as Map, HashSet as UnorderedSet};
Expand All @@ -15,7 +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>,
pub explicit_impls: Set<&'a Impl>,
}

impl<'a> Types<'a> {
Expand Down Expand Up @@ -137,7 +137,7 @@ impl<'a> Types<'a> {
}
Api::Impl(imp) => {
visit(&mut all, &imp.ty);
explicit_impls.insert(&imp.ty);
explicit_impls.insert(imp);
}
}
}
Expand Down
12 changes: 5 additions & 7 deletions tests/ui/unique_ptr_twice.stderr
Original file line number Diff line number Diff line change
@@ -1,10 +1,8 @@
error[E0119]: conflicting implementations of trait `cxx::private::UniquePtrTarget` for type `here::C`:
--> $DIR/unique_ptr_twice.rs:10:1
--> $DIR/unique_ptr_twice.rs:16:5
|
1 | #[cxx::bridge]
| -------------- first implementation here
7 | impl UniquePtr<C> {}
| ----------------- 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)
16 | impl UniquePtr<C> {}
| ^^^^^^^^^^^^^^^^^ conflicting implementation for `here::C`