Join GitHub today
GitHub is home to over 31 million developers working together to host and review code, manage projects, and build software together.
Sign upVery confusing error on attempt to pass `path::Path` by value #23286
Comments
kmcallister
added
the
A-diagnostics
label
Mar 11, 2015
This comment has been minimized.
This comment has been minimized.
|
I've hit this too and was completely baffled about what the compiler was saying. |
This comment has been minimized.
This comment has been minimized.
|
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 pub fn assure_config_dir_exists(dir: &str) -> Result<&Path, ArgumentError> {
Ok(Path::new(dir))
} |
This comment has been minimized.
This comment has been minimized.
|
Can confirm that this behaviour is still present in |
This comment has been minimized.
This comment has been minimized.
fcourchesne
commented
Jun 3, 2015
|
Confirmed in rustc 1.0 and 1.2.0-nightly (613e57b 2015-06-01) (built 2015-06-02). |
This comment has been minimized.
This comment has been minimized.
luke-gru
commented
Jun 14, 2015
|
I just hit this and was confused as well. |
aturon
added
the
T-compiler
label
Jun 18, 2015
This comment has been minimized.
This comment has been minimized.
|
Nominating. This seems like an important error to try to improve. triage: I-nominated |
rust-highfive
added
the
I-nominated
label
Jun 18, 2015
This comment has been minimized.
This comment has been minimized.
|
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:
and we require that |
This comment has been minimized.
This comment has been minimized.
|
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 |
rust-highfive
added
P-high
and removed
I-nominated
labels
Jul 9, 2015
This comment has been minimized.
This comment has been minimized.
|
Does #26617 fix this issue? |
apasel422
referenced this issue
Jul 19, 2015
Closed
Misleading error message when trying to use a Vec of an unsized enum as function argument #23371
This comment has been minimized.
This comment has been minimized.
|
No it doesn't |
This comment has been minimized.
This comment has been minimized.
|
On the other hand, just calling
I am semi-worried about a deep chain of types filling the screen with an error, not sure what to do about that. |
arielb1
referenced this issue
Aug 9, 2015
Closed
core: rustc: improve compile message for unsized types #26617
This comment has been minimized.
This comment has been minimized.
|
Here's a possible set of rules to follow which would fix the problem:
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. |
arielb1
added a commit
to arielb1/rust
that referenced
this issue
Sep 13, 2015
arielb1
referenced this issue
Sep 13, 2015
Merged
don't duplicate the code snippet in the "trait unimplemented" error #28393
arielb1
added a commit
to arielb1/rust
that referenced
this issue
Sep 13, 2015
bors
added a commit
that referenced
this issue
Sep 15, 2015
bors
added a commit
that referenced
this issue
Sep 15, 2015
bors
closed this
in
#28393
Sep 15, 2015
This comment has been minimized.
This comment has been minimized.
|
Has this really been fixed? It looks like the error was cleaned up a bit, but it still states that |
This comment has been minimized.
This comment has been minimized.
|
@marcusklaas It should now list all of the steps which result in the Sized bound being violated, including both |
This comment has been minimized.
This comment has been minimized.
|
This is definitely an improvement, but the order of information still seems backwards to me: I feel like I want to see first that |
This comment has been minimized.
This comment has been minimized.
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
reopened this
Sep 30, 2015
This comment has been minimized.
This comment has been minimized.
|
/cc @jonathandturner |
brson
added
the
I-nominated
label
Jul 14, 2016
This comment has been minimized.
This comment has been minimized.
|
cc @imperio @Manishearth @jonathandturner this is actually a pretty horrible diagnostics bug. Anybody want to take it on as P-high? |
brson
added
the
E-help-wanted
label
Jul 14, 2016
This comment has been minimized.
This comment has been minimized.
|
@nikomatsakis can still mentor. |
arielb1
self-assigned this
Jul 17, 2016
This comment has been minimized.
This comment has been minimized.
|
Will mentor @hilasha2 |
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
|
That error message would be far too long a line
|
pnkfelix
removed
the
I-nominated
label
Jul 21, 2016
This comment has been minimized.
This comment has been minimized.
|
Given the current output, would it be enough to present the following?
|
estebank
referenced this issue
Dec 3, 2016
Merged
Point out the known type when field doesn't satisfy bound #38150
bors
added a commit
that referenced
this issue
Dec 20, 2016
alexcrichton
added a commit
to alexcrichton/rust
that referenced
this issue
Dec 20, 2016
bors
closed this
in
#38150
Dec 21, 2016
This comment has been minimized.
This comment has been minimized.
luki
commented
Apr 16, 2017
|
With the latest version of Rust.
|
This comment has been minimized.
This comment has been minimized.
VersBinarii
commented
Feb 25, 2018
|
+1
|
estebank
reopened this
Feb 25, 2018
This comment has been minimized.
This comment has been minimized.
|
Reopening as the output is still confusing. |
XAMPPRocky
added
the
C-enhancement
label
Feb 26, 2018
This comment has been minimized.
This comment has been minimized.
sourcepirate
commented
May 5, 2018
|
Is there any updates on this one ? |
This comment has been minimized.
This comment has been minimized.
|
@sourcepirate I do not believe anyone has tried to address this recently. |
This was referenced Oct 11, 2018
kennytm
added a commit
to kennytm/rust
that referenced
this issue
Oct 18, 2018
bors
added a commit
that referenced
this issue
Oct 18, 2018
This comment has been minimized.
This comment has been minimized.
|
Current output
|
This comment has been minimized.
This comment has been minimized.
|
The current output looks pretty great. Explains what the error is, where it's coming from and how to fix it. |
kmcallister commentedMar 11, 2015
produces
We should give an explicit hint that
Pathshould be passed by reference. Also the error does not mentionPathand the typeu8does 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