Unrelated changes make multiple-definitions test fail spuriously

[fee1-dead]

The test was originally added in #86419, see the comments under that for more discussion.

#86922 and #86857 fails with this diff:

10 error: multiple definitions of external function `f` from library `foo.dll` have different calling conventions
-   --> $DIR/multiple-definitions.rs:8:5
12    |
12    |
- LL |     fn f(x: i32);
-    |     ^^^^^^^^^^^^^
+ LL |         fn f(x: i32);

#87075 was opened, removing the ^^^ and adding indentation (I did not see that it suggested removing --> $DIR/...). It fails with this diff:

10 error: multiple definitions of external function `f` from library `foo.dll` have different calling conventions
12    |
12    |
- LL |         fn f(x: i32);
+ LL |     fn f(x: i32);
14 
15 error: aborting due to previous error; 1 warning emitted

I just want to say that I don't think there was some nondeterministic code causing the span to be different (at least in the file that emitted the diagnostic), as shown on these lines, where they get sorted, deterministically:

rust/compiler/rustc_codegen_ssa/src/back/link.rs

Lines 483 to 487 in a867dd4

	imports.sort_unstable_by_key(|x: &DllImport| x.name.as_str());
	(lib_name, imports)
	})
	.collect::<Vec<_>>();
	result.sort_unstable_by(|a: &(String, Vec<DllImport>), b: &(String, Vec<DllImport>)| {

@rustbot label A-spurious C-bug T-compiler

[joshtriplett]

At this point I think we may want to revert the addition of the test until this issue can be debugged, to avoid spuriously breaking other PRs.

[fee1-dead]

They are being removed in #87087.

@rustbot label F-raw_dylib

[ricobbe]

@rustbot claim

[RalfJung]

where they get sorted, deterministically:

Could the use of sort_unstable be the problem here?

[ricobbe]

Could the use of sort_unstable be the problem here?

I don't think so, no. The problem is that the error message only occurs when we have multiple DllImports in the same FxHashSet that have the same name. So sorting the elements of an FxHashSet<DllImport> based solely on their names, as is the case on master, isn't sufficient to guarantee a unique result when we select the second DllImport that has a duplicate name. Just using a stable sort wouldn't help either, IIUC, since the original order in the hash set is effectively nondeterministic.

Sorting the DllImport values based on all of their fields instead of just the name, as in the linked PR, should remove this nondeterminism.