Skip to content
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

Permit ty::Bool in const generics for v0 mangling #77452

Merged
merged 1 commit into from
Oct 3, 2020

Conversation

Mark-Simulacrum
Copy link
Member

This should unbreak using new-symbol-mangling = true in config.toml (once it lands in beta anyway).

Fixes #76365 (well, it will, but seems fine to close as soon as we have support)

r? @eddyb (for mangling) but I'm okay with some other reviewer too :)

@rust-highfive rust-highfive added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Oct 2, 2020
@@ -504,6 +504,7 @@ impl Printer<'tcx> for SymbolMangler<'tcx> {

match ct.ty.kind() {
ty::Uint(_) => {}
ty::Bool => {}
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If this is all that's required, can we also add ty::Char and ty::Int, or do these need special attention?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ty::Int seems reasonable, though we might want to change mangling to express signed-ness (so you get foo<-1> in perf top or whatever, not foo<255>). I think we do not want to encode char with the bits so I would like to hold off on it for now.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

So both seem harder than just bool, which encoding with 0/1 seems quite reasonable.

@varkor varkor added the F-const_generics `#![feature(const_generics)]` label Oct 2, 2020
@eddyb
Copy link
Member

eddyb commented Oct 2, 2020

@bors r+ Btw do you have a matching rustc-demangle PR?

@bors
Copy link
Contributor

bors commented Oct 2, 2020

📌 Commit 89fdfe6 has been approved by eddyb

@bors 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 Oct 2, 2020
@Mark-Simulacrum
Copy link
Member Author

No, but I will work on that.

@varkor varkor added the A-const-generics Area: const generics (parameters and arguments) label Oct 2, 2020
bors added a commit to rust-lang-ci/rust that referenced this pull request Oct 2, 2020
…as-schievink

Rollup of 8 pull requests

Successful merges:

 - rust-lang#75377 (Fix Debug implementations of some of the HashMap and BTreeMap iterator types)
 - rust-lang#76107 (Write manifest for MAJOR.MINOR channel to enable rustup convenience)
 - rust-lang#76745 (Move Wrapping<T> ui tests into library)
 - rust-lang#77182 (Add missing examples for Fd traits)
 - rust-lang#77251 (Bypass const_item_mutation if const's type has Drop impl)
 - rust-lang#77264 (Only use LOCAL_{STDOUT,STDERR} when set_{print/panic} is used. )
 - rust-lang#77421 (Revert "resolve: Avoid "self-confirming" import resolutions in one more case")
 - rust-lang#77452 (Permit ty::Bool in const generics for v0 mangling)

Failed merges:

r? `@ghost`
@bors bors merged commit eff6398 into rust-lang:master Oct 3, 2020
@rustbot rustbot added this to the 1.48.0 milestone Oct 3, 2020
@varkor
Copy link
Member

varkor commented Oct 6, 2020

Don't worry about the rustc-demangle support: I'm going to add it along with char and signed integer support.

JohnTitor added a commit to JohnTitor/rust that referenced this pull request Oct 22, 2020
Support signed integers and `char` in v0 mangling

Likely we want more tests, to check the output is correct too: however, I wasn't sure what kind of test we needed, so I just added one similar to that added in rust-lang#77452 for now.

r? @eddyb
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-const-generics Area: const generics (parameters and arguments) F-const_generics `#![feature(const_generics)]` S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

ICE on bootstrap with new symbol mangling
6 participants