Skip to content
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

Send impl error message is too smart #21793

Closed
Gankra opened this issue Jan 31, 2015 · 11 comments
Closed

Send impl error message is too smart #21793

Gankra opened this issue Jan 31, 2015 · 11 comments
Labels
A-diagnostics Area: Messages for errors, warnings, and lints

Comments

@Gankra
Copy link
Contributor

Gankra commented Jan 31, 2015

use std::collections::BTreeMap;
use std::rc::Rc;
fn foo<T: Send>() {}
fn main() {
    foo::<BTreeMap<Rc<()>, Rc<()>>>();
}
<anon>:5:5: 5:36 error: the trait `core::marker::Send` is not implemented for the type `alloc::rc::Rc<()>` [E0277]
<anon>:5     foo::<BTreeMap<Rc<()>, Rc<()>>>();
             ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
<anon>:5:5: 5:36 note: `alloc::rc::Rc<()>` cannot be sent between threads safely
<anon>:5     foo::<BTreeMap<Rc<()>, Rc<()>>>();
             ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
error: aborting due to previous error

The lack of an impl of Send for Rc is the problem, but the more pertinent problem is that BTreeMap itself is not Send. The Rc detail is a great piece of additional information, though.

@Gankra
Copy link
Contributor Author

Gankra commented Jan 31, 2015

cc @Manishearth

@Gankra Gankra added the A-diagnostics Area: Messages for errors, warnings, and lints label Jan 31, 2015
@FillZpp
Copy link

FillZpp commented Jan 31, 2015

I think the Send is not implemented for Rc, you may use Arc instead.

use std::collections::BTreeMap;
use std::sync::Arc;
fn foo<T: Send>() { }
fn main() {
    foo::<BTreeMap<Arc<()>, Arc<()>>>();
}

It would work.

@Manishearth
Copy link
Member

That's not the issue here, the issue is that the error is complaining directly about Rc without mentioning that BTreeMap is Send only for Send parameters

@huonw
Copy link
Member

huonw commented Feb 10, 2015

This is particularly bad when its some type hidden deep in internals that is causing the problem, since it can be very non-obvious where the non-Sync/Sendness is coming from. Notably, this example demonstrates it reaching through privacy barriers into the implementation details of Sender, and, just looking at Bar, one can't determine which field is causing issues without manually digging through the types.

use std::sync::mpsc::Sender;

struct Foo {
    x: Sender<()>,
}

struct SomethingComplicated; /* lots of fields ... */

struct Bar {
    y: SomethingComplicated,
    z: Vec<Foo>,
    // etc.
}

fn test_sync<T: Sync>() {}

fn main() {
    test_sync::<Bar>();
}
<anon>:18:5: 18:21 error: the trait `core::marker::Sync` is not implemented for the type `core::cell::UnsafeCell<std::sync::mpsc::Flavor<()>>` [E0277]
<anon>:18     test_sync::<Bar>();
              ^~~~~~~~~~~~~~~~
<anon>:18:5: 18:21 note: `core::cell::UnsafeCell<std::sync::mpsc::Flavor<()>>` cannot be shared between threads safely
<anon>:18     test_sync::<Bar>();
              ^~~~~~~~~~~~~~~~

It would be nice to do a "call stack", maybe a little like:

error: the trait `core::marker::Sync` is not implemented for type `Bar` because ...
// code snippet/span of call site
error: ... it is not implemented for type `Vec<Foo>` stored in field `z` because ...
// code snippet/span of Bar::z
error: ... it is not implemented for type `Foo` because ...
// code snippet/span of the Foo in Vec<Foo>
error: ... it is not implemented for type `Sender` stored in field `x`.
// code snippet/span of Foo::x

(All the becauses are a little ridiculous...)

In that model, I'm stopping at crate boundaries, but it may also make sense to continue to stop at the first explicit negative impl.

@nikomatsakis
Copy link
Contributor

At some point @flaper87 and I did include a full stack trace. I'm not sure at what point it got lost.

@flaper87
Copy link
Contributor

@nikomatsakis we did it when we landed the negative trait implementation support. I'll check if that code changed but in theory it should still be there.

@Manishearth
Copy link
Member

If you point me to it I can probably reimplement it.

@Manishearth
Copy link
Member

I got some similar errors when playing with OIBIT

#![feature(optin_builtin_traits)]

fn main() {
println!("{} {}", 1u8.foo(), <u32 as SizeOf>::foo(&1u64))
}


trait SizeOf {
    fn foo(&self) -> u8;
}

trait SizeOfOI {}
impl SizeOfOI for .. {
}

impl<T> SizeOf for T where T: SizeOfOI {
    fn foo(&self) -> u8 {0}
}

impl !SizeOfOI for u8 {}

impl SizeOf for u8{
    fn foo(&self) -> u8 {1}
}

impl !SizeOfOI for u32 {}

playpen

(I'm using this code for a heap-size deriving which needed an overridable blanket impl)

Instead of telling me that SizeOf isn't implemented for u64, it tells me that SizeOfOI isn't implemented for u64, which is pretty confusing when found in a larger context.

@arielb1
Copy link
Contributor

arielb1 commented Sep 13, 2015

On nightly, the error message is

<anon>:5:5: 5:36 error: the trait `core::marker::Send` is not implemented for the type `alloc::rc::Rc<()>` [E0277]
<anon>:5     foo::<BTreeMap<Rc<()>, Rc<()>>>();
             ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
<anon>:5:5: 5:36 help: see the detailed explanation for E0277
<anon>:5:5: 5:36 note: `alloc::rc::Rc<()>` cannot be sent between threads safely
<anon>:5     foo::<BTreeMap<Rc<()>, Rc<()>>>();
             ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
<anon>:5:5: 5:36 note: required because it appears within the type `collections::btree::node::Node<alloc::rc::Rc<()>, alloc::rc::Rc<()>>`
<anon>:5     foo::<BTreeMap<Rc<()>, Rc<()>>>();
             ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
<anon>:5:5: 5:36 note: required because it appears within the type `collections::btree::map::BTreeMap<alloc::rc::Rc<()>, alloc::rc::Rc<()>>`
<anon>:5     foo::<BTreeMap<Rc<()>, Rc<()>>>();
             ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
<anon>:5:5: 5:36 note: required by `foo`
<anon>:5     foo::<BTreeMap<Rc<()>, Rc<()>>>();
             ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
error: aborting due to previous error

Slightly verbose (e.g., we can avoid repeating the span every time) but I think this is solved.

@jdm
Copy link
Contributor

jdm commented Sep 13, 2015

#28378 has a testcase that shows poor behaviour on a rustc from ~12 days ago; is that any better now?

@arielb1
Copy link
Contributor

arielb1 commented Sep 13, 2015

<anon>:27:5: 27:17 error: the trait `core::marker::Sync` is not implemented for the type `*mut libc::types::common::c95::c_void` [E0277]
<anon>:27     require_sync(&x);
              ^~~~~~~~~~~~
<anon>:27:5: 27:17 help: see the detailed explanation for E0277
<anon>:27:5: 27:17 note: `*mut libc::types::common::c95::c_void` cannot be shared between threads safely
<anon>:27     require_sync(&x);
              ^~~~~~~~~~~~
<anon>:27:5: 27:17 note: required because it appears within the type `OneStruct`
<anon>:27     require_sync(&x);
              ^~~~~~~~~~~~
<anon>:27:5: 27:17 note: required because it appears within the type `TwoStruct`
<anon>:27     require_sync(&x);
              ^~~~~~~~~~~~
<anon>:27:5: 27:17 note: required by `require_sync`
<anon>:27     require_sync(&x);
              ^~~~~~~~~~~~

I guess we should just not print the span multiple times and we are golden. In general, compiler stack traces are not good error messages, but the stack traces here should be short most times and I can't think of a better message.

I mean, after trimming, it would be

<anon>:27:5: 27:17 error: the trait `core::marker::Sync` is not implemented for the type `*mut libc::types::common::c95::c_void` [E0277]
<anon>:27     require_sync(&x);
              ^~~~~~~~~~~~
<anon>:27:5: 27:17 help: see the detailed explanation for E0277
<anon>:27:5: 27:17 note: `*mut libc::types::common::c95::c_void` cannot be shared between threads safely
<anon>:27:5: 27:17 note: required because it appears within the type `OneStruct`
<anon>:27:5: 27:17 note: required because it appears within the type `TwoStruct`
<anon>:27:5: 27:17 note: required by `require_sync`

arielb1 added a commit to arielb1/rust that referenced this issue Sep 13, 2015
new error style:
```
path.rs:4:6: 4:7 error: the trait `core::marker::Sized` is not implemented for the type `[u8]` [E0277]
path.rs:4 fn f(p: Path) {}
               ^
path.rs:4:6: 4:7 help: run `rustc --explain E0277` to see a detailed explanation
path.rs:4:6: 4:7 note: `[u8]` does not have a constant size known at compile-time
path.rs:4:6: 4:7 note: required because it appears within the type `std::sys::os_str::Slice`
path.rs:4:6: 4:7 note: required because it appears within the type `std::ffi::os_str::OsStr`
path.rs:4:6: 4:7 note: required because it appears within the type `std::path::Path`
path.rs:4:6: 4:7 note: all local variables must have a statically known size
path.rs:7:5: 7:36 error: the trait `core::marker::Send` is not implemented for the type `alloc::rc::Rc<()>` [E0277]
path.rs:7     foo::<BTreeMap<Rc<()>, Rc<()>>>();
              ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
path.rs:7:5: 7:36 help: run `rustc --explain E0277` to see a detailed explanation
path.rs:7:5: 7:36 note: `alloc::rc::Rc<()>` cannot be sent between threads safely
path.rs:7:5: 7:36 note: required because it appears within the type `collections::btree::node::Node<alloc::rc::Rc<()>, alloc::rc::Rc<()>>`
path.rs:7:5: 7:36 note: required because it appears within the type `collections::btree::map::BTreeMap<alloc::rc::Rc<()>, alloc::rc::Rc<()>>`
path.rs:7:5: 7:36 note: required by `foo`
error: aborting due to 2 previous errors
```

This improves the rust-lang#21793/rust-lang#23286 situation
bors added a commit that referenced this issue Sep 15, 2015
…sakis

new error style:
```
path.rs:4:6: 4:7 error: the trait `core::marker::Sized` is not implemented for the type `[u8]` [E0277]
path.rs:4 fn f(p: Path) {}
               ^
path.rs:4:6: 4:7 help: run `rustc --explain E0277` to see a detailed explanation
path.rs:4:6: 4:7 note: `[u8]` does not have a constant size known at compile-time
path.rs:4:6: 4:7 note: required because it appears within the type `std::sys::os_str::Slice`
path.rs:4:6: 4:7 note: required because it appears within the type `std::ffi::os_str::OsStr`
path.rs:4:6: 4:7 note: required because it appears within the type `std::path::Path`
path.rs:4:6: 4:7 note: all local variables must have a statically known size
path.rs:7:5: 7:36 error: the trait `core::marker::Send` is not implemented for the type `alloc::rc::Rc<()>` [E0277]
path.rs:7     foo::<BTreeMap<Rc<()>, Rc<()>>>();
              ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
path.rs:7:5: 7:36 help: run `rustc --explain E0277` to see a detailed explanation
path.rs:7:5: 7:36 note: `alloc::rc::Rc<()>` cannot be sent between threads safely
path.rs:7:5: 7:36 note: required because it appears within the type `collections::btree::node::Node<alloc::rc::Rc<()>, alloc::rc::Rc<()>>`
path.rs:7:5: 7:36 note: required because it appears within the type `collections::btree::map::BTreeMap<alloc::rc::Rc<()>, alloc::rc::Rc<()>>`
path.rs:7:5: 7:36 note: required by `foo`
error: aborting due to 2 previous errors
```

This improves the #21793/#23286 situation

r? @nikomatsakis
bors added a commit that referenced this issue Sep 15, 2015
…sakis

new error style:
```
path.rs:4:6: 4:7 error: the trait `core::marker::Sized` is not implemented for the type `[u8]` [E0277]
path.rs:4 fn f(p: Path) {}
               ^
path.rs:4:6: 4:7 help: run `rustc --explain E0277` to see a detailed explanation
path.rs:4:6: 4:7 note: `[u8]` does not have a constant size known at compile-time
path.rs:4:6: 4:7 note: required because it appears within the type `std::sys::os_str::Slice`
path.rs:4:6: 4:7 note: required because it appears within the type `std::ffi::os_str::OsStr`
path.rs:4:6: 4:7 note: required because it appears within the type `std::path::Path`
path.rs:4:6: 4:7 note: all local variables must have a statically known size
path.rs:7:5: 7:36 error: the trait `core::marker::Send` is not implemented for the type `alloc::rc::Rc<()>` [E0277]
path.rs:7     foo::<BTreeMap<Rc<()>, Rc<()>>>();
              ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
path.rs:7:5: 7:36 help: run `rustc --explain E0277` to see a detailed explanation
path.rs:7:5: 7:36 note: `alloc::rc::Rc<()>` cannot be sent between threads safely
path.rs:7:5: 7:36 note: required because it appears within the type `collections::btree::node::Node<alloc::rc::Rc<()>, alloc::rc::Rc<()>>`
path.rs:7:5: 7:36 note: required because it appears within the type `collections::btree::map::BTreeMap<alloc::rc::Rc<()>, alloc::rc::Rc<()>>`
path.rs:7:5: 7:36 note: required by `foo`
error: aborting due to 2 previous errors
```

Fixes #21793 
Fixes #23286

r? @nikomatsakis
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-diagnostics Area: Messages for errors, warnings, and lints
Projects
None yet
Development

No branches or pull requests

8 participants