Merge pull request #337 from dtolnay/impl

dtolnay · web-flow · commit debd9d5bf325 · 2020-10-04T00:48:22.000-07:00
Improve error message on a conflicting explicit impl
diff --git a/macro/src/expand.rs b/macro/src/expand.rs
@@ -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};
@@ -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() {
@@ -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));
                 }
             }
         }
@@ -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);
@@ -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" {
@@ -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);
@@ -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" {
diff --git a/syntax/impls.rs b/syntax/impls.rs
@@ -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};
@@ -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
+    }
+}
diff --git a/syntax/set.rs b/syntax/set.rs
@@ -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> {
diff --git 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, 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};
@@ -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> {
@@ -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);
                 }
             }
         }
diff --git a/tests/ui/unique_ptr_twice.stderr b/tests/ui/unique_ptr_twice.stderr
@@ -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`

Original file line number	Diff line number	Diff line change
`@@ -35,6 +35,14 @@ where`
`35`	`35`	`{`
`36`	`36`	`self.set.contains(value)`
`37`	`37`	`}`
	`38`	`+`
	`39`	`+ pub fn get<Q>(&self, value: &Q) -> Option<&'a T>`
	`40`	`+ where`
	`41`	`+ &'a T: Borrow<Q>,`
	`42`	`+ Q: ?Sized + Hash + Eq,`
	`43`	`+ {`
	`44`	`+ self.set.get(value).copied()`
	`45`	`+ }`
`38`	`46`	`}`
`39`	`47`
`40`	`48`	`impl<'s, 'a, T> IntoIterator for &'s OrderedSet<&'a T> {`
Original file line number	Diff line number	Diff line change
`@@ -1,7 +1,7 @@`
`1`	`1`	`use crate::syntax::atom::Atom::{self, *};`
`2`	`2`	`use crate::syntax::report::Errors;`
`3`	`3`	`use crate::syntax::set::OrderedSet as Set;`
`4`		`-use crate::syntax::{Api, Derive, Enum, ExternFn, ExternType, Struct, Type, TypeAlias};`
	`4`	`+use crate::syntax::{Api, Derive, Enum, ExternFn, ExternType, Impl, Struct, Type, TypeAlias};`
`5`	`5`	`use proc_macro2::Ident;`
`6`	`6`	`use quote::ToTokens;`
`7`	`7`	`use std::collections::{BTreeMap as Map, HashSet as UnorderedSet};`
`@@ -15,7 +15,7 @@ pub struct Types<'a> {`
`15`	`15`	`pub aliases: Map<&'a Ident, &'a TypeAlias>,`
`16`	`16`	`pub untrusted: Map<&'a Ident, &'a ExternType>,`
`17`	`17`	`pub required_trivial: Map<&'a Ident, TrivialReason<'a>>,`
`18`		`- pub explicit_impls: Set<&'a Type>,`
	`18`	`+ pub explicit_impls: Set<&'a Impl>,`
`19`	`19`	`}`
`20`	`20`
`21`	`21`	`impl<'a> Types<'a> {`
`@@ -137,7 +137,7 @@ impl<'a> Types<'a> {`
`137`	`137`	`}`
`138`	`138`	`Api::Impl(imp) => {`
`139`	`139`	`visit(&mut all, &imp.ty);`
`140`		`- explicit_impls.insert(&imp.ty);`
	`140`	`+ explicit_impls.insert(imp);`
`141`	`141`	`}`
`142`	`142`	`}`
`143`	`143`	`}`