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

Improve symbol normalization in binary_stats #1931

Merged
merged 2 commits into from
Aug 12, 2024

Conversation

Kobzol
Copy link
Contributor

@Kobzol Kobzol commented Jun 26, 2024

A few improvements and tests added when I was investigating rust-lang/rust#126965 (comment).

@Kobzol Kobzol requested a review from lqd June 26, 2024 09:45
Copy link
Member

@lqd lqd left a comment

Choose a reason for hiding this comment

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

(Oooops I didn't see this, sorry.)

I'm not sure we can be fully accurate and without conflating symbols to be perfect in all cases. Depending on the tasks, sometimes in the past I've had to compare using mangled symbols because the monomorphizations were important, e.g. when using const generics.

Here we're best effort so these are improvements to me.

@Kobzol Kobzol force-pushed the binary-stats-improve branch from 4fc3e48 to 87347af Compare August 11, 2024 12:05
Copy link
Member

@lqd lqd left a comment

Choose a reason for hiding this comment

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

one last typo fix.

and if we don't want to do anything yet about the pre-demangling dot because of the other issue, then we should be good

@Kobzol Kobzol force-pushed the binary-stats-improve branch from e0ab36a to c1216b5 Compare August 12, 2024 06:52
@Kobzol Kobzol enabled auto-merge August 12, 2024 06:53
@Kobzol Kobzol merged commit 47dcf23 into rust-lang:master Aug 12, 2024
11 checks passed
@Kobzol Kobzol deleted the binary-stats-improve branch August 12, 2024 10:43
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants