rustc_symbol_mangling: support structural constants and &str in v0. #87194

eddyb · 2021-07-16T15:33:47Z

This PR should unblock #85530 (except for float const generics, which AFAIK should've never worked).
(cc @tmiasko could the #85530 (comment) failures be retried with a quick crater "subset" run of this PR + changing the default to v0? Just to make sure I didn't miss anything other than the floats)

The encoding is the one suggested before in e.g. #61486 (comment), tho this PR won't by itself finish #61486, before closing that we'd likely want to move to @oli-obk's "valtrees" (i.e. #83234 and other associated work).

EDITs:

switched unit/tuple/braced-with-named-fields <const-fields> prefixes from "u"/"T"/"" to "U"/"T"/"S" to avoid the ambiguity reported by @tmiasko in rustc_symbol_mangling: support structural constants and &str in v0. #87194 (comment).
rustc-demangle PR: v0: demangle structural constants and &str. rustc-demangle#55
RFC amendment PR: [RFC2603] Extend <const> to include str and structural constants. rfcs#3161
- also removed the grammar changes included in that PR, from this description
added tests (temporarily using my fork of rustc-demangle)

r? @michaelwoerister

michaelwoerister · 2021-07-19T08:04:48Z

Nice! Thanks for moving on that so quickly, @eddyb! For reference, here is a link to the current grammar: https://github.com/rust-lang/rfcs/blob/master/text/2603-rust-symbol-name-mangling-v0.md#syntax-of-mangled-names

michaelwoerister · 2021-07-19T08:07:15Z

And here is the relevant part of the grammar that this PR will supersede:

<const> = <type> <const-data>
        | "p" // placeholder, shown as _
        | <backref>

// The encoding of a constant depends on its type. Integers use their value,
// in base 16 (0-9a-f), not their memory representation. Negative integer
// values are preceded with "n". The bool value false is encoded as `0_`, true
// value as `1_`. The char constants are encoded using their Unicode scalar
// value.
<const-data> = ["n"] {<hex-digit>} "_"

michaelwoerister · 2021-07-21T08:09:27Z

This looks great!

So now we just need support for this to land in rustc-demangle so that we can add test cases, right?

tmiasko · 2021-07-21T21:29:51Z

I extended LLVM Rust demangler with support for proposed grammar. Apart from issue with u in <const-fields>, everything else seems good to me. Special casing &str to avoid printing &*".." was easy enough.

Since you are also updating the test cases here. I remember having to change the src/test/codegen/vec-shrink-panic.rs when enabling v0 by default. Is this no longer the case?

michaelwoerister · 2021-07-22T09:05:57Z

The first and third production can start with u because of Punycode encoded identifiers.

Great catch, @tmiasko!

pcwalton · 2021-08-04T01:20:45Z

What remains to be done here? Can I help move this along somehow?

michaelwoerister · 2021-08-09T14:48:43Z

If I understand correctly the sequence for making progress here is:

Add support for the grammar additions to https://github.com/alexcrichton/rustc-demangle
Update rustc's version of rustc-demangle
Add test cases for the things added here to https://github.com/rust-lang/rust/tree/master/src/test/ui/symbol-names
Make the new mangling scheme the default (which will also give us more testing of the changes here in the wild)
Amend the RFC with the grammar changes

Does that sound right, @eddyb & @tmiasko?

EDIT: Only the first three steps are needed to merge this PR.

eddyb · 2021-08-09T15:11:33Z

Apologies for not leaving a comment with the status, I've had an in-progress rustc-demangle branch (with necessary prerequsites - though I've landed some of them separately already - and the hardest parts like str demangling) for the past week or two, I've only not submitted yet because a few other things came up. I'll try to wrap it up soon.

eddyb · 2021-08-10T11:07:32Z

Demangling PR up at rust-lang/rustc-demangle#55 - next step is submitting the RFC amendment, I'll try to get to that later today.

eddyb · 2021-08-10T22:19:49Z

I've opened the RFC amendment PR (rust-lang/rfcs#3161 - should someone FCP it?) and just pushed a few commits with symbol-names tests.

We've already done a crater run through #85530, so I think we're only blocked on the RFC amendment (and reviews, I guess? if any are left).

michaelwoerister

From my point of view this looks good to go! Maybe @oli-obk wants to have a closer look at the interactions with const-eval?

oli-obk · 2021-08-26T08:24:04Z

@bors r=michaelwoerister,oli-obk

bors · 2021-08-26T08:24:06Z

📌 Commit b4fcf1b has been approved by michaelwoerister,oli-obk

eddyb · 2021-08-26T08:43:26Z

@bors rollup=never

bors · 2021-08-26T14:09:44Z

⌛ Testing commit b4fcf1b with merge 707a1c4d93cbf4ad2886d4327d8d9be3bae4a76a...

bors · 2021-08-26T14:37:37Z

💔 Test failed - checks-actions

eddyb · 2021-08-26T17:21:06Z

Hopefully #87194 (comment) is fixed by the new commit to compiletest.

@bors r=michaelwoerister,oli-obk

bors · 2021-08-26T17:21:07Z

📌 Commit 24526bb has been approved by michaelwoerister,oli-obk

bors · 2021-08-26T19:15:12Z

⌛ Testing commit 24526bb with merge ad02dc4...

bors · 2021-08-26T21:58:42Z

☀️ Test successful - checks-actions
Approved by: michaelwoerister,oli-obk
Pushing ad02dc4 to master...

…ngling-scheme, r=wesleywiser sess: default to v0 symbol mangling on nightly cc #60705 rust-lang/compiler-team#938 Rust's current mangling scheme depends on compiler internals; loses information about generic parameters (and other things) which makes for a worse experience when using external tools that need to interact with Rust symbol names; is inconsistent; and can contain `.` characters which aren't universally supported. Therefore, Rust has defined its own symbol mangling scheme which is defined in terms of the Rust language, not the compiler implementation; encodes information about generic parameters in a reversible way; has a consistent definition; and generates symbols that only use the characters `A-Z`, `a-z`, `0-9`, and `_`. Support for the new Rust symbol mangling scheme has been added to upstream tools that will need to interact with Rust symbols (e.g. debuggers). This pull request changes the default symbol mangling scheme from the legacy scheme to the new Rust mangling scheme on nightly. The following pull requests implemented v0 mangling in rustc (if I'm missing any, let me know): - #57967 - #63559 - #75675 - #77452 - #77554 - #83767 - #87194 - #87789 Rust's symbol mangling scheme has support in the following external tools: - `binutils`/`gdb` (GNU `libiberty`) - [[PATCH] Move rust_{is_mangled,demangle_sym} to a private libiberty header. ](https://gcc.gnu.org/pipermail/gcc-patches/2019-June/523011.html) committed as gcc-mirror/gcc@979526c - [[PATCH] Simplify and generalize rust-demangle's unescaping logic. ](https://gcc.gnu.org/pipermail/gcc-patches/2019-August/527835.html) committed as gcc-mirror/gcc@42bf58b - [[PATCH] Remove some restrictions from rust-demangle. ](https://gcc.gnu.org/pipermail/gcc-patches/2019-September/530445.html) committed as gcc-mirror/gcc@e1cb00d - [[PATCH] Refactor rust-demangle to be independent of C++ demangling. ](https://gcc.gnu.org/pipermail/gcc-patches/2019-November/533719.html) ([original submission](https://gcc.gnu.org/pipermail/gcc-patches/2019-October/532388.html)) committed as gcc-mirror/gcc@32fc371 - [[PATCH] Support the new ("v0") mangling scheme in rust-demangle. ](https://gcc.gnu.org/pipermail/gcc-patches/2020-November/558905.html) ([original submission](https://gcc.gnu.org/pipermail/gcc-patches/2020-March/542012.html)) committed as gcc-mirror/gcc@8409649 - `lldb`/`llvm-objdump`/`llvm-nm`/`llvm-symbolizer`/`llvm-cxxfilt`/etc - llvm/llvm-project@7310403 - llvm/llvm-project@c8c2b46 - llvm/llvm-project@0a2d4f3 - Linux `perf` - `valgrind` - [Update demangler to support Rust v0 name mangling.](https://bugs.kde.org/show_bug.cgi?id=431306) #85530 (comment) contains a summary of the most recent crater run of the v0 mangling, and the remaining issues from that were fixed by #87194 (confirmed by follow-up crater run, #85530 (comment)). `@rustbot` label +T-compiler r? `@michaelwoerister`

rust-highfive assigned michaelwoerister Jul 16, 2021

rust-highfive added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Jul 16, 2021

This was referenced Jul 16, 2021

Tracking issue for RFC 2603, "Rust Symbol Mangling (v0)" #60705

Open

Value mangling #61486

Open

Use Rust Symbol Mangling by default #85530

Closed

eddyb mentioned this pull request Jul 20, 2021

v0: replace skip_* methods with print_* methods in a "skip printing" mode. rust-lang/rustc-demangle#53

Merged

This comment has been minimized.

Sign in to view

eddyb force-pushed the const-value-mangling branch from f169162 to af68db1 Compare July 26, 2021 18:22

This was referenced Jul 27, 2021

v0 mangling should avoid backrefs when they're not shorter than their target. #87511

Open

v0 mangling support for const generics EmbarkStudios/rust-gpu#642

Closed

This comment has been minimized.

Sign in to view

eddyb mentioned this pull request Aug 9, 2021

Tracking Issue for const fn type_id #77125

Closed

4 tasks

eddyb mentioned this pull request Aug 10, 2021

v0: demangle structural constants and &str. rust-lang/rustc-demangle#55

Merged

eddyb mentioned this pull request Aug 10, 2021

[RFC2603] Extend <const> to include str and structural constants. rust-lang/rfcs#3161

Open

eddyb force-pushed the const-value-mangling branch from af68db1 to 2c1d655 Compare August 10, 2021 22:14

This comment has been minimized.

Sign in to view

michaelwoerister approved these changes Aug 26, 2021

View reviewed changes

bors added S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Aug 26, 2021

This comment has been minimized.

Sign in to view

bors added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. and removed S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. labels Aug 26, 2021

compiletest: allow . in remote test paths (can arise from revisions).

24526bb

bors added S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Aug 26, 2021

bors added the merged-by-bors This PR was explicitly merged by bors. label Aug 26, 2021

bors merged commit ad02dc4 into rust-lang:master Aug 26, 2021

rustbot added this to the 1.56.0 milestone Aug 26, 2021

eddyb deleted the const-value-mangling branch August 26, 2021 22:41

eddyb mentioned this pull request Aug 27, 2021

rustup: update to nightly-2021-08-27 and re-add -Zsymbol-mangling-version=v0. EmbarkStudios/rust-gpu#737

Merged

Alexendoo mentioned this pull request Aug 29, 2021

ICE: glacier fixed/71348.rs with -Zinstrument-coverage: symbol_names: unsupported constant of type #79565

Closed

davidtwco mentioned this pull request Oct 15, 2021

sess: default to v0 symbol mangling on nightly #89917

Open

lcnr added the A-const-generics Area: const generics (parameters and arguments) label Dec 11, 2021

lcnr mentioned this pull request Dec 11, 2021

Tracking issue for const generics (RFC 2000) #44580

Closed

16 tasks

rustc_symbol_mangling: support structural constants and &str in v0. #87194

rustc_symbol_mangling: support structural constants and &str in v0. #87194

Uh oh!

Conversation

eddyb commented Jul 16, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

michaelwoerister commented Jul 19, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

michaelwoerister commented Jul 19, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

michaelwoerister commented Jul 21, 2021

Uh oh!

This comment has been minimized.

tmiasko commented Jul 21, 2021

Uh oh!

This comment has been minimized.

michaelwoerister commented Jul 22, 2021

Uh oh!

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

pcwalton commented Aug 4, 2021

Uh oh!

michaelwoerister commented Aug 9, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

eddyb commented Aug 9, 2021

Uh oh!

eddyb commented Aug 10, 2021

Uh oh!

eddyb commented Aug 10, 2021

Uh oh!

This comment has been minimized.

This comment has been minimized.

michaelwoerister left a comment

Choose a reason for hiding this comment

Uh oh!

oli-obk commented Aug 26, 2021

Uh oh!

bors commented Aug 26, 2021

Uh oh!

eddyb commented Aug 26, 2021

Uh oh!

bors commented Aug 26, 2021

Uh oh!

This comment has been minimized.

bors commented Aug 26, 2021

Uh oh!

eddyb commented Aug 26, 2021

Uh oh!

bors commented Aug 26, 2021

Uh oh!

bors commented Aug 26, 2021

Uh oh!

bors commented Aug 26, 2021

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

12 participants

eddyb commented Jul 16, 2021 •

edited

Loading

michaelwoerister commented Jul 19, 2021 •

edited

Loading

michaelwoerister commented Jul 19, 2021 •

edited

Loading

michaelwoerister commented Aug 9, 2021 •

edited

Loading