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

Implement new API GetEnumValuesAsUnderlyingType #73057

Merged
merged 18 commits into from
Aug 2, 2022

Conversation

LakshanF
Copy link
Contributor

@LakshanF LakshanF commented Jul 29, 2022

Fixes #72498,

  • Implemented the new API in Type and the two static API's in Enum calling the Type implementation. The abstract Type implementation throws with the expectation that a derived runtime Type will provide its own implementation.
  • CoreCLR/Mono has an override implementation and NativeAOT has its own implementation that mirrors the original PR Make Enum.GetValues AOT-safe #72236.
  • Also added an override in MetadataAssemblyLoadContext that is reflection friendly
  • Added some test scenarios with the most on EnumTests

@dotnet-issue-labeler
Copy link

Note regarding the new-api-needs-documentation label:

This serves as a reminder for when your PR is modifying a ref *.cs file and adding/modifying public APIs, to please make sure the API implementation in the src *.cs file is documented with triple slash comments, so the PR reviewers can sign off that change.

@ghost
Copy link

ghost commented Jul 29, 2022

Tagging subscribers to this area: @dotnet/area-system-reflection
See info in area-owners.md if you want to be subscribed.

Issue Details

Fixes #72498,

  • Implemented the new API in Type and the two static API's in Enum calling the Type implementation
  • CoreCLR/Mono has an override implementation and NativeAOT has its own implementation that mirrors the original PR Make Enum.GetValues AOT-safe #72236
  • Also added an override in MetadataAssemblyLoadContext that is reflection friendly
  • Added some test scenarios with the most on EnumTests
Author: LakshanF
Assignees: -
Labels:

area-System.Reflection, new-api-needs-documentation, CoreClr

Milestone: -

@eerhardt
Copy link
Member

Can you also change the upstack code to use this? That way we can ensure it is working correctly?

Ex.

Array objValues = Enum.GetValues(EnumType);
long[] ulValues = new long[objValues.Length];
for (int idx = 0; idx < objValues.Length; idx++)
{
ulValues[idx] = GetEnumValue(isUnderlyingTypeUInt64, (Enum)objValues.GetValue(idx)!, culture);
}

LakshanF and others added 2 commits July 29, 2022 09:25
LakshanF and others added 2 commits July 30, 2022 09:00
Co-authored-by: Jan Kotas <jkotas@microsoft.com>
Copy link
Member

@jkotas jkotas 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!

Co-authored-by: Jan Kotas <jkotas@microsoft.com>
LakshanF and others added 2 commits August 1, 2022 08:30
Co-authored-by: Stephen Toub <stoub@microsoft.com>
LakshanF and others added 2 commits August 1, 2022 13:01
Co-authored-by: Jan Kotas <jkotas@microsoft.com>
Co-authored-by: Jan Kotas <jkotas@microsoft.com>
Copy link
Member

@jkotas jkotas left a comment

Choose a reason for hiding this comment

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

Thanks

@LakshanF LakshanF merged commit 06e4c45 into dotnet:main Aug 2, 2022
@LakshanF LakshanF deleted the NewEnumApi branch August 2, 2022 23:24
@ghost ghost locked as resolved and limited conversation to collaborators Sep 2, 2022
@jeffhandley jeffhandley added runtime-coreclr specific to the CoreCLR runtime and removed CoreClr labels Dec 28, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[API Proposal]: Type.GetEnumValuesAsUnderlyingType
6 participants