Skip to content

the order of tcx.implementations_of_trait is unstable #120371

New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

Closed
lcnr opened this issue Jan 26, 2024 · 2 comments · Fixed by #120812
Closed

the order of tcx.implementations_of_trait is unstable #120371

lcnr opened this issue Jan 26, 2024 · 2 comments · Fixed by #120812
Labels
A-metadata Area: Crate metadata C-bug Category: This is a bug. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue.

Comments

@lcnr
Copy link
Contributor

lcnr commented Jan 26, 2024

given the following extern crate dep:

mod private {
    pub trait Hidden {}
    impl Hidden for &u128 {}
    impl Hidden for &u64 {}
    impl Hidden for &u32 {}
    impl Hidden for &u16 {}
    impl Hidden for &u8 {}
}

pub trait Sealed: private::Hidden {}

and the following root crate:

struct Local;
impl dep::Sealed for Local {} 

fn main() {}

The order in trait_impls_of depends on crate metadata of dep. The rest is only necessary to reproduce this issue.

with the following cargo.toml file

[package]
name = "test2"
version = "0.1.0"
edition = "2021"

[profile.dev]
codegen-units = <insert num codegen-units here>

[dependencies]
dep = { path = "dep" }

the output of cargo c is unstable:

with 1 codegen unit it's

error[E0277]: the trait bound `Local: dep::private::Hidden` is not satisfied
  --> src/main.rs:2:22
   |
2  | impl dep::Sealed for Local {} 
   |                      ^^^^^ the trait `dep::private::Hidden` is not implemented for `Local`
   |
   = help: the following other types implement trait `dep::private::Hidden`:
             &u128
             &u64
             &u32
             &u16
             &u8
note: required by a bound in `Sealed`
  --> /home/lcnr/test2/dep/src/lib.rs:10:19
   |
10 | pub trait Sealed: private::Hidden {}
   |                   ^^^^^^^^^^^^^^^ required by this bound in `Sealed`
   = note: `Sealed` is a "sealed trait", because to implement it you also need to implement `dep::private::Hidden`, which is not accessible; this is usually done to force you to use one of the provided types that already implement it
   = help: the following types implement the trait:
             &u32
             &u8
             &u16
             &u64
             &u128

while with 2 it is

error[E0277]: the trait bound `Local: dep::private::Hidden` is not satisfied
  --> src/main.rs:2:22
   |
2  | impl dep::Sealed for Local {} 
   |                      ^^^^^ the trait `dep::private::Hidden` is not implemented for `Local`
   |
   = help: the following other types implement trait `dep::private::Hidden`:
             &u128
             &u64
             &u32
             &u16
             &u8
note: required by a bound in `Sealed`
  --> /home/lcnr/test2/dep/src/lib.rs:10:19
   |
10 | pub trait Sealed: private::Hidden {}
   |                   ^^^^^^^^^^^^^^^ required by this bound in `Sealed`
   = note: `Sealed` is a "sealed trait", because to implement it you also need to implement `dep::private::Hidden`, which is not accessible; this is usually done to force you to use one of the provided types that already implement it
   = help: the following types implement the trait:
             &u8
             &u16
             &u64
             &u128
             &u32
@lcnr lcnr added the C-bug Category: This is a bug. label Jan 26, 2024
@rustbot rustbot added the needs-triage This issue may need triage. Remove it if it has been sufficiently triaged. label Jan 26, 2024
@lcnr
Copy link
Contributor Author

lcnr commented Jan 26, 2024

cc #120012

@lcnr lcnr changed the title the order implementations_of_trait is unstable the order of tcx.implementations_of_trait is unstable Jan 26, 2024
@fmease fmease added A-metadata Area: Crate metadata T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. and removed needs-triage This issue may need triage. Remove it if it has been sufficiently triaged. labels Jan 26, 2024
@lcnr
Copy link
Contributor Author

lcnr commented Feb 5, 2024

discussed in https://rust-lang.zulipchat.com/#narrow/stream/131828-t-compiler/topic/codegen-units.20impacts.20order.20of.20impls

root cause is

// Bring everything into deterministic order for hashing
all_impls.sort_by_cached_key(|&(trait_def_id, _)| tcx.def_path_hash(trait_def_id));
let all_impls: Vec<_> = all_impls
.into_iter()
.map(|(trait_def_id, mut impls)| {
// Bring everything into deterministic order for hashing
impls.sort_by_cached_key(|&(index, _)| {
tcx.hir().def_path_hash(LocalDefId { local_def_index: index })
});
TraitImpls {
trait_id: (trait_def_id.krate.as_u32(), trait_def_id.index),
impls: self.lazy_array(&impls),
}
})
.collect();
self.lazy_array(&all_impls)
}

@cjgillot said:

The input by calling tcx.hir().items is already in deterministic order. We can just remove the sorting.

bors added a commit to rust-lang-ci/rust that referenced this issue Jul 21, 2024
Remove unnecessary impl sorting in queries and metadata

Removes unnecessary impl sorting because queries already return their keys in HIR definition order: rust-lang#120371 (comment)

r? `@cjgillot` or `@lcnr` -- unless I totally misunderstood what was being asked for here? 😆

fixes rust-lang#120371
@bors bors closed this as completed in 0f8534e Jul 22, 2024
github-actions bot pushed a commit to rust-lang/miri that referenced this issue Jul 24, 2024
Remove unnecessary impl sorting in queries and metadata

Removes unnecessary impl sorting because queries already return their keys in HIR definition order: rust-lang/rust#120371 (comment)

r? `@cjgillot` or `@lcnr` -- unless I totally misunderstood what was being asked for here? 😆

fixes #120371
flip1995 pushed a commit to flip1995/rust-clippy that referenced this issue Jul 25, 2024
Remove unnecessary impl sorting in queries and metadata

Removes unnecessary impl sorting because queries already return their keys in HIR definition order: rust-lang/rust#120371 (comment)

r? `@cjgillot` or `@lcnr` -- unless I totally misunderstood what was being asked for here? 😆

fixes #120371
lnicola pushed a commit to lnicola/rust-analyzer that referenced this issue Jul 28, 2024
Remove unnecessary impl sorting in queries and metadata

Removes unnecessary impl sorting because queries already return their keys in HIR definition order: rust-lang/rust#120371 (comment)

r? `@cjgillot` or `@lcnr` -- unless I totally misunderstood what was being asked for here? 😆

fixes #120371
# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
A-metadata Area: Crate metadata C-bug Category: This is a bug. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants