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

Two different versions of a crate interacting leads to unhelpful error messages #22750

Open
huonw opened this issue Feb 24, 2015 · 49 comments

Comments

Projects
None yet
@huonw
Copy link
Member

commented Feb 24, 2015

https://github.com/huonw/bad-error-messages-a contains a crate a. https://github.com/huonw/bad-error-messages-bc contains two crates, b and c.

  • a contains a trait Foo and a function test that requires that trait.
  • b depends on a specific revision (8b532f5, not the HEAD) of a and contains a type Bar that implements a::Foo.
  • c depends on a's HEAD (happens to be 84cfe230) and b, and tries to use b::Bar with a::test.

c fails to compile despite Bar "clearly" implementing Foo... it's even displayed in the docs!

Cloning https://github.com/huonw/bad-error-messages-bc and running cargo build in the root (which is c) gives

   Compiling a v0.0.1 (https://github.com/huonw/bad-error-messages-a#84cfe230)
   Compiling a v0.0.1 (https://github.com/huonw/bad-error-messages-a?rev=8b532f5#8b532f51)
   Compiling b v0.0.1 (file:///home/huon/projects/test-rust/error-messages/c)
   Compiling c v0.0.1 (file:///home/huon/projects/test-rust/error-messages/c)
src/lib.rs:5:5: 5:12 error: the trait `a::Foo` is not implemented for the type `b::Bar` [E0277]
src/lib.rs:5     a::test(b::Bar);
                 ^~~~~~~
error: aborting due to previous error

There's no indication of the fundamental problem from either rustdoc or rustc: that there's two different versions of a being used, meaning a#84cfe230::Foo and a#8b532f5::Foo are different. Bar only implements the latter, but in c, the name a refers to a#84cfe230 so a::test(...) requires the former. (Using name#rev to represent the crate with that version.)

There is an indication of the difference in the example above, since a is compiled twice, but that comes from cargo, not rustc. This issue is focusing on the fact that rustc itself does not give helpful errors and/or allow tools like cargo to control those errors.

It would be nice if rustc:

  • disambiguated ambiguous names, probably by filepath/name by default
  • allowed cargo to specify semantically relevant info for the disambiguation e.g. pass in the version/revision/..., so that rustc can print foo#0.2.3 instead of the filepath/name
  • detected errors that may be caused by ambiguous names and noted it explicitly, e.g. when there is a error involving an ambiguous crate print (once) some extra lines like "note: there's multiple versions of crate ... in use": along with a listing of the possibilities.

(NB. I've used cargo to construct this example because it is the easiest way I can get two different versions of a crate. This appears in rust-lang/rust's makefiles too, where the std test runner is called "std" internally, but links against another crate called "std", i.e. the conventional standard library.)

@huonw huonw added the A-diagnostics label Feb 24, 2015

@Hoverbear

This comment has been minimized.

Copy link
Contributor

commented Feb 24, 2015

This bit me today with uuid! It took me a few minutes before I even understood that it wasn't "my" code that was giving this off.

@Kimundi

This comment has been minimized.

Copy link
Member

commented Feb 27, 2015

Related, its also annoying if only crate local paths get emitted, like in backtraces.

We need a way for crates to identify themself uniquely, and not just by an identifier. Maybe a scheme like <crate foo #ef12f212>::bar::baz?

@golddranks

This comment has been minimized.

Copy link
Contributor

commented Mar 10, 2015

I was bitten by this too.

Kimundi: Agreed for the need to unique identification. But then again, in some cases the local path tells more directly where the problem is. I wonder if there are cases where path is better and cases where a hash is better. Or maybe some kind of hybrid: If version information from cargo is available, use that. If not, if version control information is available, use commit SSH, and if no other information exists, use a path...?

@huonw

This comment has been minimized.

Copy link
Member Author

commented Sep 8, 2015

Nominating for high priority. I think this is one of the main pain points people have when using cargo, and one of the major reasons for people complaining about cargo allowing multiple versions of a crate. It's usually easy to identify once you've seen it a few times, but it is a horribly confusing and annoying experience.

(Tagged with both compiler and tools since cargo presumably plays into this, but this is probably most relevant to the compiler subteam.)

@golddranks: the question of connecting semantically relevant info to crates is discussed a little in the issue. I think the best plan would external tools to pass in arbitrary strings connected to each crate, defaulting to the path, meaning rustc doesn't have to actually understand anything, just print strings (the compiler already has an --extern NAME=PATH argument that tools like cargo use, it could be extended to also allow --extern NAME#"ID"=PATH or something).

@steveklabnik

This comment has been minimized.

Copy link
Member

commented Sep 8, 2015

I've also seen an increasing number of IRC questions about this.

On Sep 7, 2015, at 20:58, Huon Wilson notifications@github.com wrote:

Nominating for high priority. I think this is one of the main pain points people have when using cargo, and one of the major reasons for people complaining about cargo allowing multiple versions of a crate. It's usually easy to identify once you've seen it a few times, but it is a horribly confusing and annoying experience.

(Tagged with both compiler and tools since cargo presumably plays into this, but this is probably most relevant to the compiler subteam.)

@golddranks: the question of connecting semantically relevant info to crates is discussed a little in the issue. I think the best plan would external tools to pass in arbitrary strings connected to each crate, defaulting to the path, meaning rustc doesn't have to actually understand anything, just print strings (the compiler already has an --extern NAME=PATH argument that tools like cargo use, it could be extended to also allow --extern NAME#"ID"=PATH or something).


Reply to this email directly or view it on GitHub.

@Manishearth Manishearth self-assigned this Sep 8, 2015

@Manishearth

This comment has been minimized.

Copy link
Member

commented Sep 8, 2015

I'll take a crack at this

@Manishearth

This comment has been minimized.

Copy link
Member

commented Sep 8, 2015

I think at the very least, for mismatched type and unimplemented trait errors, if we detect a crate mismatch, we should mention it. Any other such errors?

@steveklabnik

This comment has been minimized.

Copy link
Member

commented Sep 8, 2015

Mismatched type is the big one.

Being able to understand this would also help Rustdoc a lot, I think there are bugs open. (That way, we could know if something is a re-export or not)

Manishearth added a commit to Manishearth/rust that referenced this issue Sep 8, 2015

bors added a commit that referenced this issue Sep 9, 2015

Auto merge of #28300 - Manishearth:crate_err, r=eddyb
Partially fixes #22750

I'll write a test for this when I figure out how to.

r? @eddyb

cc @steveklabnik

@bors bors closed this in #28300 Sep 9, 2015

@Manishearth

This comment has been minimized.

Copy link
Member

commented Sep 9, 2015

No no no github, I said "partial fix"

@Manishearth Manishearth reopened this Sep 9, 2015

@Manishearth

This comment has been minimized.

Copy link
Member

commented Sep 9, 2015

Anyway, #28300 fixes the error when there's a type mismatch. "Can't find trait" will be a tad trickier

@hexsel

This comment has been minimized.

Copy link

commented Sep 9, 2015

I had a corner-case posted on Reddit that was even more confusing:

src/main.rs:9:45: 9:47 error: mismatched types:
 expected `&cookie::jar::CookieJar<'_>`,
 found `&cookie::jar::CookieJar<'_>`

I was using Hyper with the latest cookie crate. I did it because even though 'hyper' depends on 'cookie=0.1.21' or somesuch, I couldn't use 'cookie' on my program. I'll suggest a change on cargo if it doesn't yet exist.

@Manishearth

This comment has been minimized.

Copy link
Member

commented Sep 9, 2015

Yeah, this should be fixed but the just-landed PR.

@nikomatsakis

This comment has been minimized.

Copy link
Contributor

commented Sep 17, 2015

triage: P-medium -- we think that the most important cases here have been covered. If this issue continues to arise frequently on IRC, we could bump up the priority to high.

@crumblingstatue

This comment has been minimized.

Copy link
Contributor

commented May 19, 2017

I suggest we at least temporarily mitigate this by applying @huonw's third suggestion from the list, which shouldn't require interfacing with anything external.

detected errors that may be caused by ambiguous names and noted it explicitly, e.g. when there is a error involving an ambiguous crate print (once) some extra lines like "note: there's multiple versions of crate...in use": along with a listing of the possibilities.

@jmesmon

This comment has been minimized.

Copy link
Contributor

commented May 19, 2017

Some of the issues here (around misleading module paths being reported) may be related to #34769

@shepmaster

This comment has been minimized.

Copy link
Member

commented Jun 9, 2017

This has started popping up more frequently again on Stack Overflow due to a slow shift towards Serde 1.0 as some crates depend on Serde 0.8, 0.9, or 1.0, all of which are incompatible.

Mark-Simulacrum added a commit to Mark-Simulacrum/rust that referenced this issue Jun 28, 2017

Rollup merge of rust-lang#42826 - Yorwba:type-mismatch-same-absolute-…
…paths, r=arielb1

Note different versions of same crate when absolute paths of different types match.

The current check to address rust-lang#22750 only works when the paths of the mismatched types relative to the current crate are equal, but this does not always work if one of the types is only included through an indirect dependency. If reexports are involved, the indirectly included path can e.g. [contain private modules](rust-lang#22750 (comment)).

This PR takes care of these cases by also comparing the *absolute* path, which is equal if the type hasn't moved in the module hierarchy between versions. A more coarse check would be to compare only the crate names instead of full paths, but that might lead to too many false positives.

Additionally, I believe it would be helpful to show where the differing crates came from, i.e. the information in `rustc::middle::cstore::CrateSource`, but I'm not sure yet how to nicely display all of that, so I'm leaving it to a future PR.

frewsxcv added a commit to frewsxcv/rust that referenced this issue Jun 29, 2017

Rollup merge of rust-lang#42826 - Yorwba:type-mismatch-same-absolute-…
…paths, r=arielb1

Note different versions of same crate when absolute paths of different types match.

The current check to address rust-lang#22750 only works when the paths of the mismatched types relative to the current crate are equal, but this does not always work if one of the types is only included through an indirect dependency. If reexports are involved, the indirectly included path can e.g. [contain private modules](rust-lang#22750 (comment)).

This PR takes care of these cases by also comparing the *absolute* path, which is equal if the type hasn't moved in the module hierarchy between versions. A more coarse check would be to compare only the crate names instead of full paths, but that might lead to too many false positives.

Additionally, I believe it would be helpful to show where the differing crates came from, i.e. the information in `rustc::middle::cstore::CrateSource`, but I'm not sure yet how to nicely display all of that, so I'm leaving it to a future PR.

Mark-Simulacrum added a commit to Mark-Simulacrum/rust that referenced this issue Jul 12, 2017

Rollup merge of rust-lang#42826 - Yorwba:type-mismatch-same-absolute-…
…paths, r=arielb1

Note different versions of same crate when absolute paths of different types match.

The current check to address rust-lang#22750 only works when the paths of the mismatched types relative to the current crate are equal, but this does not always work if one of the types is only included through an indirect dependency. If reexports are involved, the indirectly included path can e.g. [contain private modules](rust-lang#22750 (comment)).

This PR takes care of these cases by also comparing the *absolute* path, which is equal if the type hasn't moved in the module hierarchy between versions. A more coarse check would be to compare only the crate names instead of full paths, but that might lead to too many false positives.

Additionally, I believe it would be helpful to show where the differing crates came from, i.e. the information in `rustc::middle::cstore::CrateSource`, but I'm not sure yet how to nicely display all of that, so I'm leaving it to a future PR.

@Mark-Simulacrum Mark-Simulacrum marked this as a duplicate of #43138 Jul 19, 2017

@Mark-Simulacrum

This comment has been minimized.

Copy link
Member

commented Jul 22, 2017

@Manishearth I'm unassigning you since I don't think you are actively working on this and probably don't have it in a todo list or anything like that?

@frankmcsherry

This comment has been minimized.

Copy link
Contributor

commented Sep 10, 2017

Experienced this again ("doesn't implement trait", due to disagreement about which type we were talking about). Asked about status on IRC and got "Lints are not really a priority", which I think would be too bad.

@Manishearth

This comment has been minimized.

Copy link
Member

commented Sep 10, 2017

This isn't a lint.

@frankmcsherry

This comment has been minimized.

Copy link
Contributor

commented Sep 10, 2017

I'm not sure what we are talking about, but what I meant by "this" is the missed opportunity to explain an error to the user. I agree that this is in no sense a lint, and in the future I'll explain this to people on IRC who try and speak for the project?

@zmanian

This comment has been minimized.

Copy link
Contributor

commented Sep 10, 2017

What @Manishearth is trying to communicate is that there is a policy level of pushing most lints to Clippy or other tools rather than integrating them into the compiler.

This seems more like an area where the type errors could be improved.

@steveklabnik

This comment has been minimized.

Copy link
Member

commented Sep 10, 2017

New lints need an RFC, but this isn't a lint; it's an error message that should be improved. They're different things.

@frankmcsherry

This comment has been minimized.

Copy link
Contributor

commented Sep 10, 2017

Let me try again. I experienced a form of the issue described, though the version where a trait is implemented for one version of a type and not another (which may not be what was partially fixed). When asking about this issue on IRC, it was stated (by a regular, possibly not in a formal capacity) that "Lints are not really a priority".

This is only meant to be another "ping" about how Rust's usually quite good error messages still have some limitations, and wasn't meant to be about who thinks it was a lint, or whether its standing as a lint makes it less interesting, or anything like that. Obviously, feel free to hash that out amongst yourselves, but not on my account. :)

@Manishearth

This comment has been minimized.

Copy link
Member

commented Sep 10, 2017

I'm not sure what we are talking about,

I'm meant that this is not a lint, this is diagnostics, so "lints are not a priority" doesn't apply here and whoever gave you that status was wrong about it. Though it's possible that diagnostics are no longer a priority here either (I'd be surprised, given the ergonomics initiative and prior focus on diagnostics by the docs team).

@JustAPerson

This comment has been minimized.

Copy link
Contributor

commented Mar 10, 2018

So it appears there should be a note for this, but I don't see it in the case when you try to call a trait method and the trait is implemented for the wrong version of the struct.

error[E0599]: no method named `build_vk_surface` found for type `winit::WindowBuilder` in the current scope
  --> src/main.rs:27:46
   |
27 |     let window = winit::WindowBuilder::new().build_vk_surface(&events_loop, Arc::clone(&instance));
   |                                              ^^^^^^^^^^^^^^^^

Here my crate was accidentally depending on winit 0.11 whereas the trait was defined on version 0.7.
No chance at better diagnostics here?

@Kixunil

This comment has been minimized.

Copy link

commented May 1, 2018

My friend just ran into this and one thing I'd like to see in error messages is explanation where those crates came from.

E.g. the crate depends on a and b, a depends on c 1.0 and b depends on c 1.1.

I'd like to see error message like this:

Note: two versions of the crate `c` are present
Note: crate `c` of version 1.0 required by crate `a`
Note: crate `a` required by this crate
Note: crate `c` of version 1.1 required by crate `a`
Note: crate `a` required by this crate

I'd like to see both dependency chains in full to be able to investigate.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.