Skip to content

rust-demangler tool strips crate disambiguators with < 16 digits #77627

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

Merged
merged 1 commit into from
Oct 9, 2020

Conversation

richkadel
Copy link
Contributor

Addresses Issue #77615.

@rust-highfive
Copy link
Contributor

r? @Mark-Simulacrum

(rust_highfive has picked a reviewer for you, use r? to override)

@rust-highfive rust-highfive added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Oct 6, 2020
@Mark-Simulacrum
Copy link
Member

I did not know we had this in tree and am unsure of what exactly it's used for (I guess I can look). r? @eddyb for now though.

@richkadel
Copy link
Contributor Author

I added the tool a while back. It supports llvm-cov coverage tool's option to use an external demangler when generating coverage reports.

@richkadel
Copy link
Contributor Author

Just FYI... @wesleywiser @tmandry

This is the PR to resolve the bug report against AArch64 (though I think the bug isn't really specific to that architecture, and could get triggered on any platform depending on the generated hash value).

@camelid camelid added C-bug Category: This is a bug. O-Arm Target: 32-bit Arm processors (armv6, armv7, thumb...), including 64-bit Arm in AArch32 state labels Oct 6, 2020
@tmandry
Copy link
Member

tmandry commented Oct 7, 2020

It seems pretty clear now that this is a heuristic to remove disambiguators, even if it's one that's very likely to succeed.

That does seem acceptable for the tests we're using it for, but maybe not for everything. I think we should document that it's using a heuristic in the doc comment and help text, just so people know about it before they take on a dependency.

@bors delegate+
r=tmandry when done

@bors
Copy link
Collaborator

bors commented Oct 7, 2020

✌️ @richkadel can now approve this pull request

@richkadel
Copy link
Contributor Author

I'll make the additional changes before submitting to bors. Thanks!

@richkadel
Copy link
Contributor Author

@bors r=tmandry

@bors
Copy link
Collaborator

bors commented Oct 8, 2020

📌 Commit 796e6ac has been approved by tmandry

@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 8, 2020
@bors
Copy link
Collaborator

bors commented Oct 9, 2020

⌛ Testing commit 796e6ac with merge 9a74fb7...

@bors
Copy link
Collaborator

bors commented Oct 9, 2020

☀️ Test successful - checks-actions, checks-azure
Approved by: tmandry
Pushing 9a74fb7 to master...

@bors bors added the merged-by-bors This PR was explicitly merged by bors. label Oct 9, 2020
@bors bors merged commit 9a74fb7 into rust-lang:master Oct 9, 2020
@rustbot rustbot added this to the 1.49.0 milestone Oct 9, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
C-bug Category: This is a bug. merged-by-bors This PR was explicitly merged by bors. O-Arm Target: 32-bit Arm processors (armv6, armv7, thumb...), including 64-bit Arm in AArch32 state 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.

8 participants