-
Notifications
You must be signed in to change notification settings - Fork 314
[DRAFT] v0 mangling on nightly #1730
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Conversation
|
leaving comments would have been easier if you used semantic line breaks instead of hard wrapping at a column count :) |
| Languages like Rust and C++ define "symbol mangling schemes", leveraging information | ||
| from the type system to give each item a unique symbol name. Otherwise every | ||
| instantiation of a generic or templated function (or an overload in C++), which has | ||
| the same name in the surface language would end up with clashing symbols. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Feels like this should mention paths vs identifiers, e.g. Rust lets you have a::foo and b::foo. It's not just generics.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Added this as an example
cafc482 to
0836ea0
Compare
0836ea0 to
0cab632
Compare
| If that happens, you can use the legacy mangling scheme with | ||
| the `-Csymbol-mangling-version=legacy -Zunstable-options` flag. | ||
| Using the legacy mangling scheme requires nightly, it is not intended | ||
| to be stabilised so that support can eventually be removed. Either by |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I still think we should stabilize it with the intent to deprecate and remove (/make it a no-op) it eventually just to ease the transition for people when it hits stable.
imo making it a no-op after stabilizing is not any more breaking than changing the default in the first place
| +++ | ||
|
|
||
| **TL;DR:** rustc will use its own "v0" mangling scheme by default on nightly | ||
| versions instead of the previous default scheme which re-used C++'s mangling |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I found this long sentence hard to read, without any punctuation, and with repeated "scheme".
Is this more readable?
| versions instead of the previous default scheme which re-used C++'s mangling | |
| versions instead of the previous default, which re-used C++'s mangling |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think the usage instructions could be a bit clearer
| If that happens, you can use the legacy mangling scheme with | ||
| the `-Csymbol-mangling-version=legacy -Zunstable-options` flag. | ||
| Using the legacy mangling scheme requires nightly, it is not intended | ||
| to be stabilised so that support can eventually be removed. Either by |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Adding the "Using the legacy mangling scheme requires nightly" sentence between "you can use the legacy mangling scheme" and "Either by adding it to the usual", makes it harder to work out that the "Either" refers to "using the scheme".
Maybe the "Using the legacy mangling scheme requires nightly" explanation could go up on line 143, where it wouldn't be splitting the usage instructions?
|
|
||
| If that happens, you can use the legacy mangling scheme with | ||
| the `-Csymbol-mangling-version=legacy -Zunstable-options` flag. | ||
| Using the legacy mangling scheme requires nightly, it is not intended |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
| Using the legacy mangling scheme requires nightly, it is not intended | |
| Explicitly setting the legacy mangling scheme requires nightly, it is not intended |
Everyone on stable is using it, they just can't set it explicitly. This clarification seems important, so people understand how they're using it on stable already.
| - Symbol names can contain `.` characters which aren't supported on all platforms | ||
| - Symbol names depend on compiler internals and can't be easily replicated by | ||
| other compilers or tools | ||
|
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't know if its worth expanding this, but a big downside of looking like C++ symbols is that it makes it hard for tools to know whether to apply C++ or Rust demangling to the symbol when both languages are present.
Also that all the interesting stuff is encoded in an opaque hash, but that's more or less "depends on compiler internals".
| but some don't | ||
| - Symbol names can contain `.` characters which aren't supported on all platforms | ||
| - Symbol names depend on compiler internals and can't be easily replicated by | ||
| other compilers or tools |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The crate disambiguator still depends on compiler internals with v0, so you can't predict the symbol name that rustc will use, nor should you be able to IMO.
Blog post for rust-lang/compiler-team#938 once that MCP completes
Rendered