Include rustc version in rustc_span::StableCrateId

[pierwill]

rustc_span::def_id::StableCrateId is a hash of various data about a crate during compilation. This PR includes the version of rustc in the input when computing this hash. From a cursory reading of RFC 2603, this appears to be acceptable within that design.

In order to pass the mir-opt and ui test suites, this adds new normalization for hashes and symbol names in compiletest. These are enabled by default, but we might prefer it to be configurable.

In the UI tests, I had to truncate a significant amount of error annotations in v0 symbols (and maybe some legacy) in order to get the normalization to work correctly. (See #90116.)

Closes #85142.

[rust-highfive]

r? @nagisa

(rust-highfive has picked a reviewer for you, use r? to override)

[pierwill]

@bjorn3

[nagisa]

There are still a number of tests that seem like they would need some normalization to not require updating every time trains roll over?

[nagisa]

Preexisting, but I feel like this logic is better placed in argument parsing, so that this is applied everywhere and consistently. As it is right now, it is very plausible that other places in the code won't dedup and consider -Cmetadata=a distinct from -Cmetadata=a -Cmetadata=a.

[bjorn3]

I believe this is the only code reading -Cmetadata. I would rather prefer to keep all logic for computing the stable crate id together like it is now.

[pierwill]

@bjorn3 @cjgillot: It's been a little while since I've looked at this—sorry. Any ideas what might be causing this Windows error?

[cjgillot]

The error comes from a modified line in a backtrace test. The test already normalizes this line because it is useless. I think it's fine to bless the test and remove the comment about normalization.

[pierwill]

The error comes from a modified line in a backtrace test. The test already normalizes this line because it is useless. I think it's fine to bless the test and remove the comment about normalization.

Thanks, @cjgillot. @wesleywiser has been looking at this. Would be good to get their thoughts.

[wesleywiser]

If the line isn't being reported at all any more, then the function name that's reported must be std::sys_common::backtrace::__rust_begin_short_backtrace since that is the function name we stop reporting frames at for short backtraces. That's not ideal, but it wasn't working correctly before anyway so I don't think we need to block this PR on that. 👍 from me.

[pierwill]

This is ready for another go! 🤞 🙏

[cjgillot]

@bors r+

[bors]

📌 Commit 3b2e3bc1d6d184f1bb2e0d0cae5f3e86c168523c has been approved by cjgillot

[matthiaskrgr]

@bors rollup=never I would rather not roll this up in case this has some instr-benchmark side effects or so

[bors]

⌛ Testing commit 3b2e3bc1d6d184f1bb2e0d0cae5f3e86c168523c with merge 04594d11f8343cc626363a9d34ca212bba5ea80b...

[bors]

💔 Test failed - checks-actions

[pierwill]

Grrr! 🤯 #89836 (comment):

- thread 'main' panicked at 'd was called', $DIR/panic-short-backtrace-windows-x86_64.rs:48:5
+ thread 'main' panicked at 'd was called', $DIR/panic-short-backtrace-windows-x86_64.rs:43:5
3    0: std::panicking::begin_panic
4    1: d

5    2: c
5    2: c
6    3: b
7    4: a
+    5: <T as core::any::Any>::type_id
8 note: Some details are omitted, run with `RUST_BACKTRACE=full` for a verbose backtrace.
9 

[pierwill]

Guess we need to keep some normalization the panic-short-backtrace-windows-x86_64.rs test?

[wesleywiser]

It's the same target x86_64-pc-windows-msvc as before so I'd suggest just undoing that change. It's possible some other PR has landed in the mean time which rejiggers the layout of functions in the test binary enough that we (once again) have some function other than __rust_begin_short_backtrace as the 5th frame in the stack.

[pierwill]

Done. 👍

[wesleywiser]

@bors r+

[bors]

📌 Commit 535278a has been approved by wesleywiser

[bors]

⌛ Testing commit 535278a with merge 9e1aff8...

[bors]

☀️ Test successful - checks-actions
Approved by: wesleywiser
Pushing 9e1aff8 to master...

[rust-timer]

Finished benchmarking commit (9e1aff8): comparison url.

Summary: This change led to very large relevant mixed results 🤷 in compiler performance.

• Very large improvement in instruction counts (up to -11.6% on incr-unchanged builds of tuple-stress)
• Large regression in instruction counts (up to 4.7% on incr-unchanged builds of unicode_normalization)

If you disagree with this performance assessment, please file an issue in rust-lang/rustc-perf.

Next Steps: If you can justify the regressions found in this perf run, please indicate this with @rustbot label: +perf-regression-triaged along with sufficient written justification. If you cannot justify the regressions please open an issue or create a new PR that fixes the regressions, add a comment linking to the newly created issue or PR, and then add the perf-regression-triaged label to this PR.

@rustbot label: +perf-regression