Fix splat v0 mangling#158890
Conversation
|
rustbot has assigned @hanna-kruppe. Use Why was this reviewer chosen?The reviewer was selected based on:
|
This comment has been minimized.
This comment has been minimized.
|
Oops, forgot to implement demangling, I'll get to it tomorrow @rustbobt author |
There was a problem hiding this comment.
Just so you know, changes to the mangling scheme typically also have a corresponding PR to https://github.com/rust-lang/rustc-demangle 🙂
| // FIXME(splat): this letter is currently completely unused, but for | ||
| // efficiently we might want to use a letter that can't occur in any | ||
| // type instead | ||
| p.push("W"); |
There was a problem hiding this comment.
FYI, "W" is currently used by pattern types
rust/compiler/rustc_symbol_mangling/src/v0.rs
Lines 522 to 523 in f10db29
There was a problem hiding this comment.
Oops, I meant "w", thanks for picking this up
805f25d to
a30c0d8
Compare
a30c0d8 to
afaae5b
Compare
| // FIXME(splat): remove this once splat demangling has been added to rustc | ||
| //@ exec-env: RUST_BACKTRACE=0 |
There was a problem hiding this comment.
Hopefully this should skip demangling in panic backtraces, until the demangling impl in rust-lang/rustc-demangle#90 is added to rustc
@rustbot ready
There was a problem hiding this comment.
Just so you know, changes to the mangling scheme typically also have a corresponding PR to rust-lang/rustc-demangle
@wesleywiser, do we usually wait for the demangling PR to merge, or is there another way to disable demangling in backtraces?
Another alternative is dropping or modifying this test until the demangling merges.
@rustbot author
There was a problem hiding this comment.
I believe in the past, we've usually sequenced this kind of thing as:
- Add new mangling to rustc-demangle (done)
- Release new rustc-demangle to crates.io (done,
v0.1.28) - Update rust-lang/rust to use new rustc-demangle version
- Land the equivalent of this PR
Please also add an item to the tracking issue to update the v0 mangling documentation to include this new form when we stabilize it.
|
The job Click to see the possible cause of the failure (guessed by this bot) |
|
Reminder, once the PR becomes ready for a review, use |
|
r? wesleywiser |
|
|
Tracking issue: #153629
Typecheck considers a splatted and non-splatted function as distinct types, so mangling has to include splatting. If it doesn't, we get symbol clashes. See the ticket and the tests in this PR for details.
Demangling PR: rust-lang/rustc-demangle#90
@rustbot label +C-bug +F-splat +T-compiler +A-name-mangling
Fixes #158644