Skip to content

Conversation

@mathieubourgeois
Copy link
Contributor

Type.GetTypeCode on Mono was calling GetCorElementType, which, when invoked on an enum nested in a generic class, would return TypeCode.Object instead of TypeCode.Int32. Ensuring we grab the underlying type of the enum fixes the issue. This would provoke Enum parsing to trigger an assert in a debug runtime and go through a slower, rare path for parsing as well.

Fixes #72543

@ghost
Copy link

ghost commented Jul 22, 2022

I couldn't figure out the best area label to add to this PR. If you have write-permissions please help me learn by adding exactly one area label.

@ghost ghost added the community-contribution Indicates that the PR has been added by a community member label Jul 22, 2022
@mathieubourgeois
Copy link
Contributor Author

hmmm ok it failed some tests. Looks like IsPrimitive is not matching because it looks like an enum is not considered to be a primitive even though the underlying type actually is, but the documentation doesn't really mention anything about enum's underlying types...

Based on further analysis, I think we may have been wrong in our earlier assumptions : GetTypeCodeImpl calls GetCorElementType, and has the following case :

                case CorElementType.ELEMENT_TYPE_VALUETYPE:
                    if (ReferenceEquals(this, typeof(decimal)))
                        typeCode = TypeCode.Decimal;
                    else if (ReferenceEquals(this, typeof(DateTime)))
                        typeCode = TypeCode.DateTime;
                    else if (IsActualEnum)
                        typeCode = GetTypeCode(Enum.InternalGetUnderlyingType(this));
                    else
                        typeCode = TypeCode.Object;
                    break;

So, it looks like GetCorElementType is not supposed to return the underlying type for the enum, but is actually supposed to return VALUETYPE. I can try and change this to just do that directly in the case of an enum instead of returning the underlying type.

Copy link
Member

@lambdageek lambdageek left a comment

Choose a reason for hiding this comment

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

thanks!

`Type.GetTypeCode` on Mono was calling `GetCorElementType`, which, when invoked on an enum nested in a generic class, would return `TypeCode.Object` instead of `TypeCode.Int32`. Matching to CoreClr, we change `GetCorElementType` to return `VALUETYPE` for enums to fix the issue. The original issue would also provoke Enum parsing to trigger an assert in a debug runtime and go through a slower, rare path for parsing as well.

Fixes dotnet#72543
@mathieubourgeois mathieubourgeois force-pushed the fix/gettypecode-enum-in-nested-generic-class branch from 0a86d8f to f281616 Compare July 22, 2022 19:57
@mathieubourgeois
Copy link
Contributor Author

Pushed an adapted version of the code which is more limited and should (hopefully) pass testing this time around :) GetCorElementType will now return VALUETYPE instead of the underlying enum type, which seems to track more with what CoreClr is doing on their end.

Also moved the tests in System.Runtime.Tests since it's a more logical location for it in my opinion.

@mathieubourgeois
Copy link
Contributor Author

I suppose the test failures (System.Net.Http.Functional.Tests.WorkItemExecution) are "expected", since they happen only on net7.0-Linux-Debug-x64-CoreCLR_release-Ubuntu.1804.Amd64.Open and net7.0-windows-Debug-x64-CoreCLR_release-Windows.81.Amd64.Open, and I only modified the Mono runtime. Those seem to be the only test failures afaik.

@lambdageek
Copy link
Member

Yea, the test failures are unrelated.

@lambdageek lambdageek merged commit a1468ed into dotnet:main Jul 25, 2022
@ghost ghost locked as resolved and limited conversation to collaborators Aug 24, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

community-contribution Indicates that the PR has been added by a community member

Projects

None yet

Development

Successfully merging this pull request may close these issues.

In Mono runtime, Type.GetTypeCode returns improper value for an enum nested in a generic class

2 participants