Detect multiple crate versions on method not found by estebank · Pull Request #128786 · rust-lang/rust

estebank · 2024-08-07T15:20:49Z

When a type comes indirectly from one crate version but the imported trait comes from a separate crate version, the called method won't be found. We now show additional context:

error[E0599]: no method named `foo` found for struct `dep_2_reexport::Type` in the current scope
--> multiple-dep-versions.rs:8:10
|
8 | Type.foo();
| ^^^ method not found in `Type`
|
note: there are multiple different versions of crate `dependency` in the dependency graph
--> multiple-dep-versions.rs:4:32
|
4 | use dependency::{do_something, Trait};
| ^^^^^ `dependency` imported here doesn't correspond to the right crate version
|
::: ~/rust/build/x86_64-unknown-linux-gnu/test/run-make/crate-loading/rmake_out/multiple-dep-versions-1.rs:4:1
|
4 | pub trait Trait {
| --------------- this is the trait that was imported
|
::: ~/rust/build/x86_64-unknown-linux-gnu/test/run-make/crate-loading/rmake_out/multiple-dep-versions-2.rs:4:1
|
4 | pub trait Trait {
| --------------- this is the trait that is needed
5 | fn foo(&self);
| --- the method is available for `dep_2_reexport::Type` here

Fix#128569, fix#110926, fix#109161, fix#81659, fix#51458, fix#32611. Follow up to #124944.

rustbot · 2024-08-07T15:20:58Z

r? @fee1-dead

rustbot has assigned @fee1-dead.
They will have a look at your PR within the next two weeks and either review your PR or reassign to another reviewer.

Use r? to explicitly pick a reviewer

rustbot · 2024-08-07T15:21:00Z

This PR modifies tests/run-make/. If this PR is trying to port a Makefile
run-make test to use rmake.rs, please update the
run-make port tracking issue
so we can track our progress. You can either modify the tracking issue
directly, or you can comment on the tracking issue and link this PR.

cc @jieyouxu

estebank · 2024-08-07T16:20:02Z

+ if let Some(map) = self.tcx.in_scope_traits_map(owner) {
+ for trait_candidate in map.to_sorted_stable_ord().into_iter().flat_map(|v| v.1.iter()) {


This is the thing I'm slightly uneasy about: for every error of a method not found that triggers the prior conditions, we'll be doing a linear scan of all traits in scope. Don't have a good sense of whether that will be a significant slow-down for compiles with errors (not a good way of testing that difference).

Please take some time to fuzz this. Would it be possible to trigger this in some normal session by removing some imports or upgrading a crate to a newer version (where the trait method is removed in the newer version and the older version is still imported by some transitive dependencies)?
It's very possible that some large project trying to refactor something will hit this if this is a significant slowdown.

I think putting this behind a query that allocates a HashMap<crate name, Vec<trait DefId>> and returns it would reduce the amortized cost significantly and eliminate the risk of causing trouble.

Ok, I changed it to use in_scope_traits instead of in_scope_traits_map, which will be much cheaper (at the cost of only looking at traits in the current scope, which is fine).

@fee1-dead given the current approach (where we're looking at in-scope traits for a specific scope), which is something we're already doing for every method resolution, I feel more comfortable with it. I'm also not quite sure how to synthesize a big enough corpus of "wrong crate" to properly stress this.

estebank · 2024-08-07T22:14:32Z

I think after landing this PR the only error that still needs to check for multiple crate versions is E0277. E0308 already has a check (even though it isn't great), and this addresses E0599 for every case I could get my hands on.

Edit: identified the reason most of the E0277 weren't triggering and fixed it in #128849. I think the only place left where we could do more is E0308.

fee1-dead · 2024-08-09T11:34:14Z

+ if let Some(map) = self.tcx.in_scope_traits_map(owner) {
+ for trait_candidate in map.to_sorted_stable_ord().into_iter().flat_map(|v| v.1.iter()) {


Please take some time to fuzz this. Would it be possible to trigger this in some normal session by removing some imports or upgrading a crate to a newer version (where the trait method is removed in the newer version and the older version is still imported by some transitive dependencies)?
It's very possible that some large project trying to refactor something will hit this if this is a significant slowdown.

When a type comes indirectly from one crate version but the imported trait comes from a separate crate version, the called method won't be found. We now show additional context: ``` error[E0599]: no method named `foo` found for struct `dep_2_reexport::Type` in the current scope --> multiple-dep-versions.rs:8:10 | 8 | Type.foo(); | ^^^ method not found in `Type` | note: you have multiple different versions of crate `dependency` in your dependency graph --> multiple-dep-versions.rs:4:32 | 4 | use dependency::{do_something, Trait}; | ^^^^^ `dependency` imported here doesn't correspond to the right crate version | ::: ~/rust/build/x86_64-unknown-linux-gnu/test/run-make/crate-loading/rmake_out/multiple-dep-versions-1.rs:4:1 | 4 | pub trait Trait { | --------------- this is the trait that was imported | ::: ~/rust/build/x86_64-unknown-linux-gnu/test/run-make/crate-loading/rmake_out/multiple-dep-versions-2.rs:4:1 | 4 | pub trait Trait { | --------------- this is the trait that is needed 5 | fn foo(&self); | --- the method is available for `dep_2_reexport::Type` here ```

When encountering the following, mention the precense of conflicting crates: ``` error[E0599]: no function or associated item named `get_decoded` found for struct `HpkeConfig` in the current scope --> src/main.rs:7:17 | 7 | HpkeConfig::get_decoded(&foo); | ^^^^^^^^^^^ function or associated item not found in `HpkeConfig` | note: if you're trying to build a new `HpkeConfig`, consider using `HpkeConfig::new` which returns `HpkeConfig` --> ~/.cargo/registry/src/index.crates.io-6f17d22bba15001f/janus_messages-0.3.1/src/lib.rs:908:5 | 908 | / pub fn new( 909 | | id: HpkeConfigId, 910 | | kem_id: HpkeKemId, 911 | | kdf_id: HpkeKdfId, 912 | | aead_id: HpkeAeadId, 913 | | public_key: HpkePublicKey, 914 | | ) -> HpkeConfig { | |___________________^ note: there are multiple different versions of crate `prio` in the dependency graph --> src/main.rs:1:5 | 1 | use prio::codec::Decode; | ^^^^^^^^^^^^^^^^^^^ `prio` imported here doesn't correspond to the right crate version | ::: ~/.cargo/registry/src/index.crates.io-6f17d22bba15001f/prio-0.9.1/src/codec.rs:35:1 | 35 | pub trait Decode: Sized { | ----------------------- this is the trait that was imported | ::: ~/.cargo/registry/src/index.crates.io-6f17d22bba15001f/prio-0.10.3/src/codec.rs:35:1 | 35 | pub trait Decode: Sized { | ----------------------- this is the trait that is needed ... 43 | fn get_decoded(bytes: &[u8]) -> Result<Self, CodecError> { | -------------------------------------------------------- the method is available for `HpkeConfig` here help: there is an associated function `decode` with a similar name | 7 | HpkeConfig::decode(&foo); | ~~~~~~ ```

As per the case presented in rust-lang#128569, we should be showing the extra info even if auto-deref is involved.

Make checking slightly cheaper (by restricting to the right item only). Add tests.

fee1-dead

Thanks!

fee1-dead · 2024-08-17T04:50:43Z

@bors r+

bors · 2024-08-17T04:50:46Z

📌 Commit 110b19b has been approved by fee1-dead

It is now in the queue for this repository.

…r=fee1-dead Detect multiple crate versions on method not found When a type comes indirectly from one crate version but the imported trait comes from a separate crate version, the called method won't be found. We now show additional context: ``` error[E0599]: no method named `foo` found for struct `dep_2_reexport::Type` in the current scope --> multiple-dep-versions.rs:8:10 | 8 | Type.foo(); | ^^^ method not found in `Type` | note: there are multiple different versions of crate `dependency` in the dependency graph --> multiple-dep-versions.rs:4:32 | 4 | use dependency::{do_something, Trait}; | ^^^^^ `dependency` imported here doesn't correspond to the right crate version | ::: ~/rust/build/x86_64-unknown-linux-gnu/test/run-make/crate-loading/rmake_out/multiple-dep-versions-1.rs:4:1 | 4 | pub trait Trait { | --------------- this is the trait that was imported | ::: ~/rust/build/x86_64-unknown-linux-gnu/test/run-make/crate-loading/rmake_out/multiple-dep-versions-2.rs:4:1 | 4 | pub trait Trait { | --------------- this is the trait that is needed 5 | fn foo(&self); | --- the method is available for `dep_2_reexport::Type` here ``` Fixrust-lang#128569, fixrust-lang#110926, fixrust-lang#109161, fixrust-lang#81659, fixrust-lang#51458, fixrust-lang#32611. Follow up to rust-lang#124944.

Rollup of 9 pull requests Successful merges: - rust-lang#128786 (Detect multiple crate versions on method not found) - rust-lang#128982 (Re-enable more debuginfo tests on Windows) - rust-lang#128989 (Emit an error for invalid use of the linkage attribute) - rust-lang#129115 (Re-enable `dump-ice-to-disk` for Windows) - rust-lang#129164 (Use `ar_archive_writer` for writing COFF import libs on all backends) - rust-lang#129167 (mir/pretty: use `Option` instead of `Either<Once, Empty>`) - rust-lang#129168 (Return correct HirId when finding body owner in diagnostics) - rust-lang#129173 (Fix `is_val_statically_known` for floats) - rust-lang#129185 (Port `run-make/libtest-json/validate_json.py` to Rust) r? `@ghost` `@rustbot` modify labels: rollup

bors · 2024-08-17T14:40:07Z

⌛ Testing commit 110b19b with merge 9b318d2...

bors · 2024-08-17T17:08:26Z

☀️ Test successful - checks-actions
Approved by: fee1-dead
Pushing 9b318d2 to master...

rust-timer · 2024-08-17T19:10:41Z

Finished benchmarking commit (9b318d2): comparison URL.

Overall result: no relevant changes - no action needed

@rustbot label: -perf-regression

Instruction count

This benchmark run did not return any relevant results for this metric.

Max RSS (memory usage)

Results (primary 2.1%)

This is a less reliable metric that may be of interest but was not used to determine the overall result at the top of this comment.

	mean	range	count
Regressions ❌ (primary)	2.1%	[2.1%, 2.1%]	1
Regressions ❌ (secondary)	-	-	0
Improvements ✅ (primary)	-	-	0
Improvements ✅ (secondary)	-	-	0
All ❌✅ (primary)	2.1%	[2.1%, 2.1%]	1

Cycles

This benchmark run did not return any relevant results for this metric.

Binary size

This benchmark run did not return any relevant results for this metric.

Bootstrap: 749.433s -> 750.64s (0.16%)
Artifact size: 339.08 MiB -> 339.13 MiB (0.01%)

When looking for multiple versions of the same crate, do not blindly construct the receiver type. Follow up to rust-lang#128786. Fixrust-lang#129205.

…errors Do not ICE on non-ADT rcvr type when looking for crate version collision When looking for multiple versions of the same crate, do not blindly construct the receiver type. Follow up to rust-lang#128786. Fixrust-lang#129205.

When looking for multiple versions of the same crate, do not blindly construct the receiver type. Follow up to rust-lang#128786. Fixrust-lang#129205.

…errors Do not ICE on non-ADT rcvr type when looking for crate version collision When looking for multiple versions of the same crate, do not blindly construct the receiver type. Follow up to rust-lang#128786. Fixesrust-lang#129205Fixesrust-lang#129216

Rollup merge of rust-lang#129250 - estebank:issue-129205, r=compiler-errors Do not ICE on non-ADT rcvr type when looking for crate version collision When looking for multiple versions of the same crate, do not blindly construct the receiver type. Follow up to rust-lang#128786. Fixesrust-lang#129205Fixesrust-lang#129216

When looking for multiple versions of the same crate, do not blindly construct the receiver type. Follow up to rust-lang#128786. Fixrust-lang#129205.

rustbot assigned fee1-dead Aug 7, 2024

rustbot added A-run-make Area: port run-make Makefiles to rmake.rs S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. labels Aug 7, 2024

compiler-errors reviewed Aug 7, 2024
View reviewed changes

Comment threadcompiler/rustc_hir_typeck/src/method/suggest.rs Outdated

estebank commented Aug 7, 2024
View reviewed changes

fee1-dead requested changes Aug 9, 2024
View reviewed changes

rustbot added S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Aug 9, 2024

estebank force-pushed the multiple-crate-versions branch from 13a7918 to 882f4a4Compare August 10, 2024 00:38

This comment has been minimized.
Sign in to view

estebank force-pushed the multiple-crate-versions branch from 882f4a4 to 515a88bCompare August 10, 2024 00:56

This comment has been minimized.
Sign in to view

estebank force-pushed the multiple-crate-versions branch from 515a88b to 22bcd5cCompare August 10, 2024 17:56

estebank added 6 commits August 12, 2024 19:29

reword message
5c427b4

Ignore auto-deref for multiple crate version note
4e98553
As per the case presented in rust-lang#128569, we should be showing the extra info even if auto-deref is involved.

Rework suggestion method
6105893
Make checking slightly cheaper (by restricting to the right item only). Add tests.

Properly differentiate between methods and assoc fns
110b19b

estebank force-pushed the multiple-crate-versions branch from 22bcd5c to 110b19bCompare August 12, 2024 19:45

estebank added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. and removed S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. labels Aug 12, 2024

fee1-dead approved these changes Aug 17, 2024
View reviewed changes

bors removed the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Aug 17, 2024

bors added the S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. label Aug 17, 2024

tgross35 mentioned this pull request Aug 17, 2024
Rollup of 8 pull requests #129197
Closed

tgross35 mentioned this pull request Aug 17, 2024
Rollup of 9 pull requests #129198
Closed

bors added the merged-by-bors This PR was explicitly merged by bors. label Aug 17, 2024

bors merged commit 9b318d2 into rust-lang:masterAug 17, 2024

rustbot added this to the 1.82.0 milestone Aug 17, 2024

This was referenced Aug 17, 2024
Make E0599's label more clear for field which is used like a method. #127193
Closed
Tweak detection of multiple crate versions to be more encompassing #128849
Merged

matthiaskrgr mentioned this pull request Aug 17, 2024
ICE: unexpected non-type Node::GenericParam #129205
Closed

estebank mentioned this pull request Aug 18, 2024
Do not ICE on non-ADT rcvr type when looking for crate version collision #129250
Merged

jieyouxu mentioned this pull request Aug 26, 2024
[Nightly ICE] double panic / DefIf does not have a "type_of" / method resolution should've emitted an error #129616
Closed

traviscross added the A-diagnostics Area: Messages for errors, warnings, and lints label Sep 29, 2024

matthiaskrgr mentioned this pull request Jan 22, 2025
ICE: item_name: no name for DefPath#135863
Closed

		if let Some(map) = self.tcx.in_scope_traits_map(owner) {
		for trait_candidate in map.to_sorted_stable_ord().into_iter().flat_map(\|v\| v.1.iter()) {

Uh oh!

Uh oh!

Conversation

estebank commented Aug 7, 2024• edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

rustbot commented Aug 7, 2024

Uh oh!

rustbot commented Aug 7, 2024

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

estebank commented Aug 7, 2024• edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

This comment has been minimized.

This comment has been minimized.

fee1-dead left a comment

Choose a reason for hiding this comment

Uh oh!

fee1-dead commented Aug 17, 2024

Uh oh!

bors commented Aug 17, 2024

Uh oh!

bors commented Aug 17, 2024

Uh oh!

bors commented Aug 17, 2024

Uh oh!

rust-timer commented Aug 17, 2024

Overall result: no relevant changes - no action needed

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

8 participants

estebank commented Aug 7, 2024•
edited
Loading

estebank commented Aug 7, 2024•
edited
Loading