Move placeholder handling to a proper preprocessing step

[amandasystems]

This commit breaks out the logic of placheolder rewriting into its own preprocessing step. It's one of the more boring
parts of #130227.

The only functional change from this is that the preprocessing step (where extra r: 'static constraints are added) is performed upstream of Polonius legacy, finally affecting Polonius. That is mostly a by-product, though.

This should be reviewable by anyone in the compiler team, so
r? rust-lang/compiler

[lcnr]

r? lcnr

[amandasystems]

Ok; revised SCC annotations have landed and I think I addressed all of your concerns so far @lcnr?

[amandasystems]

@lcnr Fixed all the code review stuff now, I think! I amended it into the second commit.

[amandasystems]

@lcnr I think I've addressed all of those now! Except for the one that should probably have a PR of its own or be merged into #140737

[lcnr]

a few more comment nits, and then finally r=me ☠️ sry

[lcnr]

@bors delegate+

[rustbot]

⚠️ Warning ⚠️

• This PR is based on an upstream commit that is 33 days old.

  It's recommended to update your branch according to the rustc-dev-guide.

[lcnr]

@bors r+ rollup=never

[bors]

📌 Commit f455c4f has been approved by lcnr

It is now in the queue for this repository.

[bors]

⌛ Testing commit f455c4f with merge 425e142...

[bors]

☀️ Test successful - checks-actions
Approved by: lcnr
Pushing 425e142 to master...

[github-actions]

What is this?

This is an experimental post-merge analysis report that shows differences in test outcomes between the merged PR and its parent PR.

Comparing 81a964c (parent) -> 425e142 (this PR)

Test differences

Show 12 test diffs

12 doctest diffs were found. These are ignored, as they are noisy.

Test dashboard

Run

cargo run --manifest-path src/ci/citool/Cargo.toml -- \
    test-dashboard 425e142686242c7e73f5e32c79071ae266f0f355 --output-dir test-dashboard

And then open test-dashboard/index.html in your browser to see an overview of all executed tests.

Job duration changes

1. dist-aarch64-linux: 7981.6s -> 5665.1s (-29.0%)
2. x86_64-apple-1: 7196.0s -> 5922.1s (-17.7%)
3. mingw-check-1: 1889.2s -> 1566.3s (-17.1%)
4. dist-apple-various: 6840.7s -> 5703.5s (-16.6%)
5. x86_64-apple-2: 4251.8s -> 3552.1s (-16.5%)
6. aarch64-apple: 5077.7s -> 4342.9s (-14.5%)
7. i686-gnu-2: 6156.6s -> 5284.3s (-14.2%)
8. x86_64-rust-for-linux: 2423.3s -> 2106.9s (-13.1%)
9. i686-gnu-1: 8150.7s -> 7091.2s (-13.0%)
10. dist-x86_64-apple: 9736.7s -> 8479.4s (-12.9%)

How to interpret the job duration changes?

Job durations can vary a lot, based on the actual runner instance
that executed the job, system noise, invalidated caches, etc. The table above is provided
mostly for t-infra members, for simpler debugging of potential CI slow-downs.

[rust-timer]

Finished benchmarking commit (425e142): comparison URL.

Overall result: ❌ regressions - please read the text below

Our benchmarks found a performance regression caused by this PR.
This might be an actual regression, but it can also be just noise.

Next Steps:

• If the regression was expected or you think it can be justified,
  please write a comment with sufficient written justification, and add
  @rustbot label: +perf-regression-triaged to it, to mark the regression as triaged.
• If you think that you know of a way to resolve the regression, try to create
  a new PR with a fix for the regression.
• If you do not understand the regression or you think that it is just noise,
  you can ask the @rust-lang/wg-compiler-performance working group for help (members of this group
  were already notified of this PR).

@rustbot label: +perf-regression
cc @rust-lang/wg-compiler-performance

Instruction count

This is the most reliable metric that we have; it was used to determine the overall result at the top of this comment. However, even this metric can sometimes exhibit noise.

	mean	range	count
Regressions ❌ (primary)	0.1%	[0.1%, 0.1%]	1
Regressions ❌ (secondary)	0.2%	[0.2%, 0.3%]	8
Improvements ✅ (primary)	-	-	0
Improvements ✅ (secondary)	-	-	0
All ❌✅ (primary)	0.1%	[0.1%, 0.1%]	1

Max RSS (memory usage)

Results (secondary 5.7%)

This is a less reliable metric that may be of interest but was not used to determine the overall result at the top of this comment.

	mean	range	count
Regressions ❌ (primary)	-	-	0
Regressions ❌ (secondary)	5.7%	[5.7%, 5.7%]	1
Improvements ✅ (primary)	-	-	0
Improvements ✅ (secondary)	-	-	0
All ❌✅ (primary)	-	-	0

Cycles

This benchmark run did not return any relevant results for this metric.

Binary size

This benchmark run did not return any relevant results for this metric.

Bootstrap: 750.423s -> 749.811s (-0.08%)
Artifact size: 371.83 MiB -> 371.77 MiB (-0.02%)

[panstromek]

perf triage:

These regressions look real. One tiny regression in Cargo and two secondary benchmarks - coercions and ucd, which are both based on giant static tables and are somewhat artificial.

I'm inclined to say this is fine as this is a piece of larger important work and the regressions are pretty limited. #130227 (which this PR is part of) does have a bit different perf results, so I guess it will also make more sense asses the perf impact of this work when more pieces of it lands.

Is that a fair assesment?

[amandasystems]

I think so, but it’s really strange that this has any performance impact at all. This PR only moves stuff around, the core logic is unchanged.

[amandasystems]

But yes; it’s supposed to be a part of a larger set of work that would actually change the logic quite substantially. Unfortunately, the first two to three parts are adding redundant mechanisms before the third (or fourth, depending on how you slice it) actually reaps the benefits.

[panstromek]

This PR only moves stuff around, the core logic is unchanged.

Moving stuff around can affect inlining, so this is not that surprising, especially when the perf changes are this small. If this didn't change what the code does, then it's probably just that and it's spurious.

Let's mark this as triaged then, thanks for quick response ;).

@rustbot label: +perf-regression-triaged