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

Delete strong name cruft #1006

Merged
merged 3 commits into from
Dec 18, 2019
Merged

Delete strong name cruft #1006

merged 3 commits into from
Dec 18, 2019

Conversation

jkotas
Copy link
Member

@jkotas jkotas commented Dec 18, 2019

The amount of strong name support that CoreCLR needs is very small (really just a method to convert public key to public key token). It is not worth it to build a separate .lib for just this single method. Fold the strong name APIs into metadata and change the API to return HRESULT.

@@ -226,11 +226,7 @@ DLLEXPORT ICorJitCompiler* __stdcall getJit()
// If you are using it more broadly in retail code, you would need to understand the
// performance implications of accessing TLS.

#ifndef __GNUC__
Copy link
Member Author

Choose a reason for hiding this comment

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

Ifdef cleanup that came along for the ride.

@jkotas jkotas requested a review from elinor-fung December 18, 2019 03:43
Copy link
Member

@janvorli janvorli left a comment

Choose a reason for hiding this comment

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

LGTM, thank you!

The amount of strong name support that CoreCLR needs is very small (really just a method to convert public key to public key token). It is not worth it to build a separate .lib for just this single method. Fold the strong name APIs into metadata and change the API to return HRESULT.
@danmoseley
Copy link
Member

Mostly unrelated, but have we recently gone through an exercise of dumping the functions the linker is dropping, and considering whether any represent code that can be deleted?

I know at least some linkers will tell you what they dropped. I could imagine building for all targets, then finding the intersection.

Copy link
Contributor

@nattress nattress left a comment

Choose a reason for hiding this comment

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

Thanks for cleaning this up!

@jkotas jkotas merged commit e695a98 into dotnet:master Dec 18, 2019
@jkotas jkotas deleted the strongname-cruft branch December 21, 2019 06:07
@ghost ghost locked as resolved and limited conversation to collaborators Dec 11, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants