Try to evaluate constants in legacy mangling

[oli-obk]

Best reviewed commit by commit.

It seems kind of odd to treat literals differently from unevaluated free constants. So let's evaluate those constants and only fall back to _ rendering if that fails to result in an integral constant

[rustbot]

r? @BoxyUwU

rustbot has assigned @BoxyUwU.
They will have a look at your PR within the next two weeks and either review your PR or reassign to another reviewer.

Use r? to explicitly pick a reviewer

[BoxyUwU]

Would it be possible to instead override print_impl_path to normalize the trait ref before printing it. I think that would be the only place we can encounter unnormalized consts/types as the instance's generic args should be fully normalized. This would also mean you don't need to carry around a DefId in the printer.

v0 mangling also normalizes there and not in print_const/type so I think this is somewhat reasonable :3

[BoxyUwU]

Existing but I think this should just be calling self.print_const, if print_const isn't handling everything that this code is then it ought to be updated so that the "better" printing of const args to arrays is also present for all const arguments.

It seems somewhat error prone to be implementing mangling of consts in two separate places

[oli-obk]

Yea, I didn't want to dedup that initially because I couldn't tell if it was somehow load bearing. I'll do that now

[oli-obk]

hmm... the reason this was done was to avoid printing 0_usize for array lengths... I don't see a good way to avoid doing that if we want print_const to normally print the type, too.

[BoxyUwU]

Ah unfortunate

[BoxyUwU]

@bors r+ rollup

[bors]

📌 Commit 9ecdc54 has been approved by BoxyUwU

It is now in the queue for this repository.

[Kobzol]

@rust-timer build 89bc42c

[rust-timer]

Finished benchmarking commit (89bc42c): comparison URL.

Overall result: ❌ regressions - please read the text below

Benchmarking this pull request likely means that it is perf-sensitive, so we're automatically marking it as not fit for rolling up. While you can manually mark this PR as fit for rollup, we strongly recommend not doing so since this PR may lead to changes in compiler perf.

Next Steps: If you can justify the regressions found in this try perf run, please indicate this with @rustbot label: +perf-regression-triaged along with sufficient written justification. If you cannot justify the regressions please fix the regressions and do another perf run. If the next run shows neutral or positive results, the label will be automatically removed.

@bors rollup=never
@rustbot label: -S-waiting-on-perf +perf-regression

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.3%	[0.3%, 0.4%]	7
Regressions ❌ (secondary)	0.4%	[0.4%, 0.4%]	2
Improvements ✅ (primary)	-	-	0
Improvements ✅ (secondary)	-	-	0
All ❌✅ (primary)	0.3%	[0.3%, 0.4%]	7

Max RSS (memory usage)

Results (primary -3.0%, secondary -2.4%)

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)	-	-	0
Improvements ✅ (primary)	-3.0%	[-3.0%, -3.0%]	1
Improvements ✅ (secondary)	-2.4%	[-2.8%, -1.7%]	3
All ❌✅ (primary)	-3.0%	[-3.0%, -3.0%]	1

Cycles

Results (secondary -2.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)	-	-	0
Improvements ✅ (primary)	-	-	0
Improvements ✅ (secondary)	-2.7%	[-2.7%, -2.7%]	1
All ❌✅ (primary)	-	-	0

Binary size

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

Bootstrap: 769.535s -> 770.02s (0.06%)
Artifact size: 333.08 MiB -> 333.16 MiB (0.02%)

[oli-obk]

There likely are ways to implement this without a perf regression, but we don't want the maintenance burden. This is also closer to how v0 mangling works, which is good, but not a highly important motivation. The value of this change is low, so idk if we want to keep it