Sort "the following types implement the trait" suggestion

[madsmtm]

This should help external UI tests using trybuild by being less non-deterministic.

This is somewhat hard to reproduce, as it basically requires two different machines, but I've been consistently running into this for the past few months with the output from my CI differing from my local setup (example of a failed workflow in here).

[rustbot]

r? @petrochenkov

(rustbot has picked a reviewer for you, use r? to override)

[madsmtm]

Actually, thinking about it, this can be reproduced on macOS by using Rosetta. I don't have a smaller test-case at hand, but in https://github.com/madsmtm/objc2, running the following commands generate different .stderr files.

TRYBUILD=overwrite cargo run --features=run --bin test-ui --target=aarch64-apple-darwin
TRYBUILD=overwrite cargo run --features=run --bin test-ui --target=x86_64-apple-darwin

[petrochenkov]

If non_blanket_impls() or blanket_impls() return a non-deterministic result, then maybe there's a deeper issue here that would be better to track to its root cause and resolve properly instead of hiding it with sorting in this specific diagnostic.

The diff from CI:

             (dyn NSObjectProtocol + 'static)
             (dyn NSObjectProtocol + Sync + 'static)
-            (dyn NSObjectProtocol + Send + 'static)
             (dyn NSObjectProtocol + Send + Sync + 'static)
+            (dyn NSObjectProtocol + Send + 'static)
             (dyn NSCopying + 'static)
             (dyn NSMutableCopying + 'static)
             (dyn NSCacheDelegate + 'static)

cc @rust-lang/types
@rustbot author

[lcnr]

I can't test this myself, but collecting impls should be deterministic

rust/compiler/rustc_middle/src/ty/trait_def.rs

Line 216 in fa0dc20

pub(super) fn trait_impls_of_provider(tcx: TyCtxt<'_>, trait_id: DefId) -> TraitImpls {

HOWEVER: impls in extern crates are iterated using tcx.crates(()).iter() and local impls via tcx.hir().trait_impls which relies on rustc_resolve which iirc walks the ast. Both of these can change the order of impls due to optional dependencies and cfgs.

[petrochenkov]

Both of these can change the order of impls due to optional dependencies and cfgs.

That's true, but CI failure example doesn't look like something originating from that?
Looks more like an issue with sorting of dyn Trait + AutoTraits + 'lifetimes types.

It would be good if @madsmtm made a minimized reproduction for this issue.

[madsmtm]

maybe there's a deeper issue here that would be better to track to its root cause and resolve properly instead of hiding it with sorting in this specific diagnostic.

Fair enough. I went with the sorting here because it was a simple fix, and is done elsewhere to make errors stable across invocations.

[madsmtm]

It would be good if @madsmtm made a minimized reproduction for this issue.

Sorry, I didn't see your comment before now. I'll try to narrow down a reproduction, can't guarantee any timeline on that though ;).

[lcnr]

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

i encountered the underlying issue separately, it can be triggered using the following extern crate

pub trait Trait {}

impl Trait for &u32 {}
impl Trait for &u16 {}

the order of these two impls is different depending on the number of codegen-units (probably depends on the crate hash somehow)

[lcnr]

opened #120371 for the underlying bug here

[bors]

☔ The latest upstream changes (presumably #120649) made this pull request unmergeable. Please resolve the merge conflicts.

[JohnCSimon]

@madsmtm

ping from triage - can you post your status on this PR? This PR has not received an update in a few months.

FYI: when a PR is ready for review, send a message containing
@rustbot ready to switch to S-waiting-on-review so the PR is in the reviewer's backlog.

Or if you're not going to continue, please close it. Thank you!

[madsmtm]

I think the problem I was having will be resolved by #120812, so am going to close this, thanks for the ping!