Rust: Very confusing error on attempt to pass `path::Path` by value

[kmcallister]

use std::path::Path;

fn f(p: Path) { }

produces

foo.rs:3:6: 3:7 error: the trait `core::marker::Sized` is not implemented for the type `[u8]` [E0277]
foo.rs:3 fn f(p: Path) { }
              ^
foo.rs:3:6: 3:7 note: `[u8]` does not have a constant size known at compile-time
foo.rs:3 fn f(p: Path) { }
              ^

We should give an explicit hint that Path should be passed by reference. Also the error does not mention Path and the type u8 does not appear in the program.

UPDATE: The message now includes a full stack trace of types, but it would be better to start out with the type the user knows about (Path, in this case) and only discuss [u8] last. -- @nikomatsakis

[brson]

I've hit this too and was completely baffled about what the compiler was saying.

[Byron]

Same here, this was my code:

pub fn assure_config_dir_exists(dir: &str) -> Result<Path, ArgumentError> {
    Ok(Path::new(dir))
}

This works. Something that I didn't expect is that Path::new(...) returns a borrow.

pub fn assure_config_dir_exists(dir: &str) -> Result<&Path, ArgumentError> {
    Ok(Path::new(dir))
}

[marcusklaas]

Can confirm that this behaviour is still present in rustc 1.2.0-nightly (9cb7b3ffb 2015-06-01) (built 2015-06-01).

[fcourchesne]

Confirmed in rustc 1.0 and 1.2.0-nightly (613e57b44 2015-06-01) (built 2015-06-02).

[luke-gru]

I just hit this and was confused as well. rustc 1.2.0-nightly (c6b148337 2015-06-12)

[aturon]

Nominating. This seems like an important error to try to improve.

triage: I-nominated

[arielb1]

The issue here is how we report errors on trait-matching in the presence of nested obligations - we report the predicate that failed to shallowly match and not the original obligation. We essentially have:

struct Path([u8]);

unsafe impl Sized for Path where foreach_field!(_: T in Struct, T: Sized) {}
// expanded:
unsafe impl Sized for Path where [u8]: Sized {}

and we require that Path: Sized - which then requires [u8]: Sized, which doesn't hold.

[nikomatsakis]

All right, I'll give it P-high. I've been thinking of tackling this issue that @arielb1 is describing as part of another change. This is kind of a dup but I'm going to keep it separate.

triage: P-high

[liigo]

Does https://github.com/rust-lang/rust/pull/26617 fix this issue?

[arielb1]

@liigo

No it doesn't

[arielb1]

On the other hand, just calling note_obligation_cause after displaying the report_on_unimplemented error gives this nice error:

test.rs:3:6: 3:7 error: the trait `core::marker::Sized` is not implemented for the type `[u8]` [E0277]
test.rs:3 fn f(p: Path) {}
               ^
test.rs:3:6: 3:7 help: run `rustc --explain E0277` to see a detailed explanation
test.rs:3:6: 3:7 note: `[u8]` does not have a constant size known at compile-time
test.rs:3 fn f(p: Path) {}
               ^
test.rs:3:6: 3:7 note: required because it appears within the type `std::sys::os_str::Slice`
test.rs:3 fn f(p: Path) {}
               ^
test.rs:3:6: 3:7 note: required because it appears within the type `std::ffi::os_str::OsStr`
test.rs:3 fn f(p: Path) {}
               ^
test.rs:3:6: 3:7 note: required because it appears within the type `std::path::Path`
test.rs:3 fn f(p: Path) {}
               ^
test.rs:3:6: 3:7 note: all local variables must have a statically known size
test.rs:3 fn f(p: Path) {}
               ^

I am semi-worried about a deep chain of types filling the screen with an error, not sure what to do about that.

[Diggsey]

Here's a possible set of rules to follow which would fix the problem:
1) Is the !Sized type derived from a type parameter? If so, treat the type parameter as the source of the error.
2) Is the !Sized field accessible? If so, treat the field as the source of the error (as it does currently)
3) Otherwise, treat the entire type as an opaque, !Sized type which is the source of the error.

This avoids breaking encapsulation: outside its own module it becomes impossible to tell from an error message that a Path is represented as a [u8] - for all intents and purposes it's a new unsized type, unrelated to any others.

[marcusklaas]

Has this really been fixed? It looks like the error was cleaned up a bit, but it still states that [u8] is unsized, instead of Path..

[Diggsey]

@marcusklaas It should now list all of the steps which result in the Sized bound being violated, including both Path and [u8]

[aturon]

This is definitely an improvement, but the order of information still seems backwards to me: I feel like I want to see first that Path isn't sized -- which is the immediate problem -- and then a kind of "backtrace" for _why_ Path isn't sized. Starting off with the inner reason (about [u8] here) makes it very confusing at first, even if all of the needed information is there.

cc @nikomatsakis

[nikomatsakis]

@aturon

Starting off with the inner reason (about [u8] here) makes it very confusing at first, even if all of the needed information is there.

Hmm, I see your point. The current message reflects more how the compiler operators internally than the mental model of the user. We could invert the message, I imagine, though what I'd really prefer to do is to keep more information than we have now (which @jroesch has been working on) in terms of keeping the full "obligation stack". But in any case that if we wanted to invert the stack trace I imagine we could, with a little bit of legwork. I'd be happy to mentor this. I'll re-open for now.

[nikomatsakis]

triage: P-medium

However, I'm demoting to P-medium.

[arielb1]

@nikomatsakis

The bottom "obligation" is often a cause ("all local variables must have a statically known size"). How would reverting it would look?

[nikomatsakis]

@arielb1 So, I think part of the confusion arises because the compiler currently "commits" an impl and then loads its dependencies as subworkitems. When one of those dependencies (transitively) gets a failure, we report the "backtrace" that is here. It would probably be more intuitive if we started from the top of the backtrace -- the original requirement -- and said something like:

• Error: Path: Sized does not hold
• Path must be sized because "all local variables must have a statically known size"
• Path: Sized does not hold because:
  
  
  
  • Path contains [u8], and [u8] is not Sized
  
  

That's the high-level information I think we ought to convey, but I'm not sure just how to format it for maximum comprehension. It's worth keeping in mind that dense error messages are intimidating and hard-to-follow. Sometimes there can be such a thing as too much information. But I'd like to hear from others what order they think the info should be presented in etc. I think for now I wouldn't worry about how the compiler is implemented interally -- we can restructure things to keep the info that we need around for when we need it...

[steveklabnik]

I like those three error messages. The second two could just be note.

[arielb1]

So

Error: `Path: Sized` does not hold [E0277]
test.rs:3 fn f(p: Path) {}
               ^
test.rs:3:6: 3:7 help: run `rustc --explain E0277` to see a detailed explanation
test.rs:3:6: 3:7 note: all local variables must have a statically known size
test.rs:3:6: 3:7 note: `Path` does not have a constant size known at compile-time
test.rs:3:6: 3:7 note: as it requires `std::sys::os_str::Slice: Sized`
test.rs:3:6: 3:7 note: as it requires `std::ffi::os_str::OsStr: Sized`
test.rs:3:6: 3:7 note: as it requires `[u8]: Sized`

[nikomatsakis]

@arielb1 something like that, yeah. IMO it'd be nice if we could go a bit further and actually combine the error and first note, so something like:

Error: `Path: Sized` required because all local variables must have a statically known size [E0277]
test.rs:3 fn f(p: Path) {}
               ^
test.rs:3:6: 3:7 help: run `rustc --explain E0277` to see a detailed explanation
test.rs:3:6: 3:7 note: `Path: Sized` does not hold...
test.rs:3:6: 3:7 note: ...as it requires `std::sys::os_str::Slice: Sized`...
test.rs:3:6: 3:7 note: ...as it requires `std::ffi::os_str::OsStr: Sized`...
test.rs:3:6: 3:7 note: ...as it requires `[u8]: Sized`.

This is more concise (note also the ... to indicate to the reader that something follows). One implication however would be that the error code E0277 would be used for a variety of distinct error messages, all of which are of course some kind of unimplemented trait. I actually LIKE the idea of using an error code to group distinct but related error messages together, but I know some people disagree.

[steveklabnik]

/cc @jonathandturner

[brson]

cc @imperio @Manishearth @jonathandturner this is actually a pretty horrible diagnostics bug. Anybody want to take it on as P-high?

[brson]

@nikomatsakis can still mentor.

[arielb1]

Will mentor @hilasha2

[brson]

Thanks @hilasha2 and @arielb1!

[arielb1]

That error message would be far too long a line

error: all local variables must have a statically known size [E0277]
test.rs:3 fn f(p: Path) {}
               ^
test.rs:3:6: 3:7 help: run `rustc --explain E0277` to see a detailed explanation
test.rs:3:6: 3:7 note: requirement `std::path::Path: std::marker::Sized` does not hold...
test.rs:3:6: 3:7 note: ...as it requires `std::sys::os_str::Slice: std::marker::Sized`...
test.rs:3:6: 3:7 note: ...as it requires `std::ffi::os_str::OsStr: std::marker::Sized`...
test.rs:3:6: 3:7 note: ...as it requires `[u8]: std::marker::Sized`.

[estebank]

Given the current output, would it be enough to present the following?

error[E0277]: the trait bound `[u8]: std::marker::Sized` is not satisfied in type `std::path::Path`
 --> <anon>:3:6
  |
3 | fn f(p: Path) { }
  |      ^ within type `std::path::Path`, the trait `std::marker::Sized` is not implemented for `[u8]`
  |
  = note: `[u8]` does not have a constant size known at compile-time
  = note: required because it appears within the type `std::path::Path`
  = note: all local variables must have a statically known size

[luki]

With the latest version of Rust.

error[E0277]: the trait bound `[u8]: std::marker::Sized` is not satisfied
 --> src/logic.rs:5:5
  |
5 |     product_path: Path,
  |     ^^^^^^^^^^^^^^^^^^ the trait `std::marker::Sized` is not implemented for `[u8]`
  |
  = note: `[u8]` does not have a constant size known at compile-time
  = note: required because it appears within the type `std::path::Path`
  = note: only the last field of a struct may have a dynamically sized type

[VersBinarii]

+1
This still appears to be an issue:

error[E0277]: the trait bound `[u8]: std::marker::Sized` is not satisfied in `std::path::Path`
 --> src/main.rs:8:5
  |
8 |     scan_dir: Path,
  |     ^^^^^^^^^^^^^^ `[u8]` does not have a constant size known at compile-time
  |
  = help: within `std::path::Path`, the trait `std::marker::Sized` is not implemented for `[u8]`
  = note: required because it appears within the type `std::path::Path`
  = note: only the last field of a struct may have a dynamically sized type

[estebank]

Reopening as the output is still confusing.

[sourcepirate]

Is there any updates on this one ?

[estebank]

@sourcepirate I do not believe anyone has tried to address this recently.

[estebank]

Current output

error[E0277]: the size for values of type `[u8]` cannot be known at compilation time
 --> src/lib.rs:3:6
  |
3 | fn f(p: Path) { }
  |      ^ borrow the `Path` instead
  |
  = help: within `std::path::Path`, the trait `std::marker::Sized` is not implemented for `[u8]`
  = note: to learn more, visit <https://doc.rust-lang.org/book/second-edition/ch19-04-advanced-types.html#dynamically-sized-types-and-the-sized-trait>
  = note: required because it appears within the type `std::path::Path`
  = note: all local variables must have a statically known size
  = help: unsized locals are gated as an unstable feature

[marcusklaas]

The current output looks pretty great. Explains what the error is, where it's coming from and how to fix it.