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

Do not call extern_crate on current trait on crate mismatch errors #133585

Merged
merged 3 commits into from
Nov 30, 2024

Conversation

estebank
Copy link
Contributor

When we encounter an error caused by traits/types of different versions of the same crate, filter out the current crate when collecting spans to add to the context so we don't call extern_crate on the DefId of the current crate, which is meaningless and ICEs.

Produced output with this filter:

error[E0277]: the trait bound `foo::Struct: Trait` is not satisfied
  --> y.rs:13:19
   |
13 |     check_trait::<foo::Struct>();
   |                   ^^^^^^^^^^^ the trait `Trait` is not implemented for `foo::Struct`
   |
note: there are multiple different versions of crate `foo` in the dependency graph
  --> y.rs:7:1
   |
4  | extern crate foo;
   | ----------------- one version of crate `foo` is used here, as a direct dependency of the current crate
5  |
6  | pub struct Struct;
   | ----------------- this type implements the required trait
7  | pub trait Trait {}
   | ^^^^^^^^^^^^^^^ this is the required trait
   |
  ::: x.rs:4:1
   |
4  | pub struct Struct;
   | ----------------- this type doesn't implement the required trait
5  | pub trait Trait {}
   | --------------- this is the found trait
   = note: two types coming from two different versions of the same crate are different types even if they look the same
   = help: you can use `cargo tree` to explore your dependency tree
note: required by a bound in `check_trait`
  --> y.rs:10:19
   |
10 | fn check_trait<T: Trait>() {}
   |                   ^^^^^ required by this bound in `check_trait`

Fix #133563.

When we encounter an error caused by traits/types of different versions of the same crate, filter out the current crate when collecting spans to add to the context so we don't call `extern_crate` on the `DefId` of the current crate, which is meaningless and ICEs.

Produced output with this filter:

```
error[E0277]: the trait bound `foo::Struct: Trait` is not satisfied
  --> y.rs:13:19
   |
13 |     check_trait::<foo::Struct>();
   |                   ^^^^^^^^^^^ the trait `Trait` is not implemented for `foo::Struct`
   |
note: there are multiple different versions of crate `foo` in the dependency graph
  --> y.rs:7:1
   |
4  | extern crate foo;
   | ----------------- one version of crate `foo` is used here, as a direct dependency of the current crate
5  |
6  | pub struct Struct;
   | ----------------- this type implements the required trait
7  | pub trait Trait {}
   | ^^^^^^^^^^^^^^^ this is the required trait
   |
  ::: x.rs:4:1
   |
4  | pub struct Struct;
   | ----------------- this type doesn't implement the required trait
5  | pub trait Trait {}
   | --------------- this is the found trait
   = note: two types coming from two different versions of the same crate are different types even if they look the same
   = help: you can use `cargo tree` to explore your dependency tree
note: required by a bound in `check_trait`
  --> y.rs:10:19
   |
10 | fn check_trait<T: Trait>() {}
   |                   ^^^^^ required by this bound in `check_trait`
```

Fix rust-lang#133563.
@rustbot
Copy link
Collaborator

rustbot commented Nov 28, 2024

r? @jieyouxu

rustbot has assigned @jieyouxu.
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 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 Nov 28, 2024
@rustbot
Copy link
Collaborator

rustbot commented Nov 28, 2024

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
Copy link
Contributor Author

We could also add some additional output mentioning that the current crate depends on a different version of itself, but I can't think of a good way of stating that and presumably that fact will be clear enough to the crate author that did that with the current output.

Copy link
Member

@jieyouxu jieyouxu left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks, the suggestion looks good to me, r=me after a nit for the stderr snapshot handling.

@jieyouxu
Copy link
Member

@rustbot author

@rustbot 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 Nov 29, 2024
@rustbot
Copy link
Collaborator

rustbot commented Nov 29, 2024

The run-make-support library was changed

cc @jieyouxu

Copy link
Member

@jieyouxu jieyouxu left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks, r=me once PR CI is green.

@rust-log-analyzer

This comment has been minimized.

@estebank
Copy link
Contributor Author

@bors r=jieyouxu

@bors
Copy link
Contributor

bors commented Nov 29, 2024

📌 Commit 998ff2f has been approved by jieyouxu

It is now in the queue for this repository.

@bors bors added S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. and removed S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. labels Nov 29, 2024
jieyouxu added a commit to jieyouxu/rust that referenced this pull request Nov 30, 2024
Do not call `extern_crate` on current trait on crate mismatch errors

When we encounter an error caused by traits/types of different versions of the same crate, filter out the current crate when collecting spans to add to the context so we don't call `extern_crate` on the `DefId` of the current crate, which is meaningless and ICEs.

Produced output with this filter:

```
error[E0277]: the trait bound `foo::Struct: Trait` is not satisfied
  --> y.rs:13:19
   |
13 |     check_trait::<foo::Struct>();
   |                   ^^^^^^^^^^^ the trait `Trait` is not implemented for `foo::Struct`
   |
note: there are multiple different versions of crate `foo` in the dependency graph
  --> y.rs:7:1
   |
4  | extern crate foo;
   | ----------------- one version of crate `foo` is used here, as a direct dependency of the current crate
5  |
6  | pub struct Struct;
   | ----------------- this type implements the required trait
7  | pub trait Trait {}
   | ^^^^^^^^^^^^^^^ this is the required trait
   |
  ::: x.rs:4:1
   |
4  | pub struct Struct;
   | ----------------- this type doesn't implement the required trait
5  | pub trait Trait {}
   | --------------- this is the found trait
   = note: two types coming from two different versions of the same crate are different types even if they look the same
   = help: you can use `cargo tree` to explore your dependency tree
note: required by a bound in `check_trait`
  --> y.rs:10:19
   |
10 | fn check_trait<T: Trait>() {}
   |                   ^^^^^ required by this bound in `check_trait`
```

Fix rust-lang#133563.
bors added a commit to rust-lang-ci/rust that referenced this pull request Nov 30, 2024
Rollup of 10 pull requests

Successful merges:

 - rust-lang#116161 (Stabilize `extended_varargs_abi_support`)
 - rust-lang#132750 ([AIX] handle libunwind native_libs)
 - rust-lang#133488 (tests: Add regression test for self referential structs with cow as last field)
 - rust-lang#133569 (Bump `ruzstd` to 0.7.3)
 - rust-lang#133585 (Do not call `extern_crate` on current trait on crate mismatch errors)
 - rust-lang#133587 (Fix target_feature handling in freg of LoongArch inline assembly)
 - rust-lang#133599 (Add `+forced-atomics` feature to esp32s2 no_std  target)
 - rust-lang#133620 (Simplify hir_typeck_pass_to_variadic_function)
 - rust-lang#133623 (Improve span handling in `parse_expr_bottom`.)
 - rust-lang#133625 (custom MIR: add doc comment for debuginfo)

r? `@ghost`
`@rustbot` modify labels: rollup
bors added a commit to rust-lang-ci/rust that referenced this pull request Nov 30, 2024
Rollup of 10 pull requests

Successful merges:

 - rust-lang#116161 (Stabilize `extended_varargs_abi_support`)
 - rust-lang#132750 ([AIX] handle libunwind native_libs)
 - rust-lang#133488 (tests: Add regression test for self referential structs with cow as last field)
 - rust-lang#133569 (Bump `ruzstd` to 0.7.3)
 - rust-lang#133585 (Do not call `extern_crate` on current trait on crate mismatch errors)
 - rust-lang#133587 (Fix target_feature handling in freg of LoongArch inline assembly)
 - rust-lang#133599 (Add `+forced-atomics` feature to esp32s2 no_std  target)
 - rust-lang#133620 (Simplify hir_typeck_pass_to_variadic_function)
 - rust-lang#133623 (Improve span handling in `parse_expr_bottom`.)
 - rust-lang#133625 (custom MIR: add doc comment for debuginfo)

r? `@ghost`
`@rustbot` modify labels: rollup
@bors bors merged commit 70e71f5 into rust-lang:master Nov 30, 2024
6 checks passed
@rustbot rustbot added this to the 1.85.0 milestone Nov 30, 2024
rust-timer added a commit to rust-lang-ci/rust that referenced this pull request Nov 30, 2024
Rollup merge of rust-lang#133585 - estebank:issue-133563, r=jieyouxu

Do not call `extern_crate` on current trait on crate mismatch errors

When we encounter an error caused by traits/types of different versions of the same crate, filter out the current crate when collecting spans to add to the context so we don't call `extern_crate` on the `DefId` of the current crate, which is meaningless and ICEs.

Produced output with this filter:

```
error[E0277]: the trait bound `foo::Struct: Trait` is not satisfied
  --> y.rs:13:19
   |
13 |     check_trait::<foo::Struct>();
   |                   ^^^^^^^^^^^ the trait `Trait` is not implemented for `foo::Struct`
   |
note: there are multiple different versions of crate `foo` in the dependency graph
  --> y.rs:7:1
   |
4  | extern crate foo;
   | ----------------- one version of crate `foo` is used here, as a direct dependency of the current crate
5  |
6  | pub struct Struct;
   | ----------------- this type implements the required trait
7  | pub trait Trait {}
   | ^^^^^^^^^^^^^^^ this is the required trait
   |
  ::: x.rs:4:1
   |
4  | pub struct Struct;
   | ----------------- this type doesn't implement the required trait
5  | pub trait Trait {}
   | --------------- this is the found trait
   = note: two types coming from two different versions of the same crate are different types even if they look the same
   = help: you can use `cargo tree` to explore your dependency tree
note: required by a bound in `check_trait`
  --> y.rs:10:19
   |
10 | fn check_trait<T: Trait>() {}
   |                   ^^^^^ required by this bound in `check_trait`
```

Fix rust-lang#133563.
@compiler-errors
Copy link
Member

Beta nominating due to #134705

@compiler-errors compiler-errors added the beta-nominated Nominated for backporting to the compiler in the beta channel. label Dec 23, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-run-make Area: port run-make Makefiles to rmake.rs beta-nominated Nominated for backporting to the compiler in the beta channel. S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Cargo check panic on nightly
6 participants