Add v0 symbol mangling for f16 and f128

[tgross35]

As discussed at #122106, use the crate encoding to represent new primitives.

[rustbot]

r? @lcnr

rustbot has assigned @lcnr.
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

[tgross35]

r? @michaelwoerister

This should probably have a test, what does this need to look like?

@rustbot label +F-f16_and_f128

[wargio]

In our demangler i have added these tests: https://github.com/rizinorg/rz-libdemangle/pull/65/files#diff-2714036a391c793a3dc93f3a3194c9d3998b9af31cc717a026e43058cd5b5e99R75-R78

[tgross35]

That is awesome, thanks for being on the cutting edge of Rust demangling. It really is helpful to know that these changes work.

[michaelwoerister]

Thanks for opening the PR! Yes, tests will be needed. There are UI tests in https://github.com/rust-lang/rust/blob/master/tests/ui/symbol-names. I'm a bit surprised that the test case for simple types does not test seem to test v0 mangling at all. It would be best to fix that. Let me know if you get stuck, then I can look into updating the existing tests in a separate PR.

[tgross35]

I added the v0 checks to the types test you linked, and added f16 and f128. I must be doing something with the ERROR checks though, I can't seem to get the attributes to check correctly even though the stderr files look alright.

[michaelwoerister]

It seems that the test case parser cannot deal with - inside revision names in error annotations. I suggest we fix this asap so that others don't waste time debugging this again. Updating this regular expression in compiletest should do the trick:

rust/src/tools/compiletest/src/errors.rs

Line 121 in 63f70b3

Lazy::new(|| Regex::new(r"//(?:\[(?P<revs>[\w,]+)])?~(?P<adjust>\||\^*)").unwrap());

That needs to be (?P<revs>[\w\-,]+) because \w apparently does not cover -.

[tgross35]

Missed that comment, thanks! Really had me confused.

Opened #124137 to fix this

[michaelwoerister]

@tgross35, if you want to make progress with this PR, independently of what the fix for compiletest turns out to be, I'd suggest to just rename the verbose-legacy revision to something that currently works.

[tgross35]

That's my plan if #124137 doesn't land before I circle back to this. Do the changes look fine, assuming I get the tests to actually pass? I probably also have to change ERROR demangling(<a[HASH]::b::Type<bool>>) to ERROR ::b::Type<bool>>) since normalization doesn't get passed to directives, unfortunately

[tgross35]

Ah sorry this slipped my mind, thanks for the poke.

@rustbot ready

[michaelwoerister]

Thank you, @tgross35!

@bors r+

[bors]

📌 Commit 792a9bd has been approved by michaelwoerister

It is now in the queue for this repository.

[bors]

⌛ Testing commit 792a9bd with merge bdfd941...

[bors]

☀️ Test successful - checks-actions
Approved by: michaelwoerister
Pushing bdfd941 to master...

[rust-timer]

Finished benchmarking commit (bdfd941): comparison URL.

Overall result: ✅ improvements - no action needed

@rustbot label: -perf-regression

Instruction count

This is a highly reliable metric that was used to determine the overall result at the top of this comment.

	mean	range	count
Regressions ❌ (primary)	-	-	0
Regressions ❌ (secondary)	-	-	0
Improvements ✅ (primary)	-0.4%	[-0.5%, -0.2%]	2
Improvements ✅ (secondary)	-	-	0
All ❌✅ (primary)	-0.4%	[-0.5%, -0.2%]	2

Max RSS (memory usage)

Results (secondary 4.3%)

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)	4.3%	[4.3%, 4.3%]	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: 675.763s -> 676.581s (0.12%)
Artifact size: 315.97 MiB -> 315.96 MiB (-0.00%)

[tgross35]

@rustbot label +beta-nominated

Nominating the first commit of this PR (809b84e) for beta backport as not having it in stage0 is blocking updates to compiler-builtins #125016. Only the first commit since the second adds tests that depend on a rustc-demangle update (part of #124213, which is much larger).

Unsure what the policy is for this kind of less essential update so I asked on Zulip. Risk is qutie low so figure it doesn't hurt to ask.

[apiraino]

Beta backport approved as per compiler team on Zulip. A backport PR will be authored by the release team at the end of the current development cycle.

@rustbot label +beta-accepted