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

Fix up nondeterminism in serializing naming style preferences #45483

Merged

Conversation

jasonmalinowski
Copy link
Member

We have two members in TypeKind that point to the same value, Struct and Structure. Because of this, Enum.ToString(), which under the covers uses a binary search, isn't stable which one it will pick and it can change if other TypeKinds are added. This ensures we keep using the same string consistently.

Fixes #44714

@jasonmalinowski jasonmalinowski requested a review from a team as a code owner June 26, 2020 19:07
@jasonmalinowski jasonmalinowski self-assigned this Jun 26, 2020
Copy link
Member

@dibarbet dibarbet left a comment

Choose a reason for hiding this comment

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

Looks fine to me, one question though. If the enum represents the same thing, should we just stop producing one of them and deprecate it?

@jasonmalinowski
Copy link
Member Author

If the enum represents the same thing, should we just stop producing one of them and deprecate it?

@dibarbet: it's not that there's different entries, there's two entries with the same numeric value. Back in the days we apparently couldn't decide whether TypeKind (which is shared between C# and VB) should use the C# keyword name or the VB keyword name, so we chose "both" as the option and had two entries with the same numeric value. When you call ToString() the framework just has to pick one.

We have two members in TypeKind that point to the same value, Struct
and Structure. Because of this, Enum.ToString(), which under the covers
uses a binary search, isn't stable which one it will pick and it can
change if other TypeKinds are added. This ensures we keep using the
same string consistently.

We also have a few members in MethodKind that are also duplicates, but
it appears those have kept stable. To be safe, we now serialize
explicitly.

Fixes dotnet#44714
Copy link

@ghost ghost left a comment

Choose a reason for hiding this comment

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

Auto-approval

@jasonmalinowski jasonmalinowski merged commit 327c010 into dotnet:master Jun 27, 2020
@ghost ghost added this to the Next milestone Jun 27, 2020
@jasonmalinowski jasonmalinowski deleted the fix-serialization-inconsistency branch June 27, 2020 19:56
@dibarbet dibarbet modified the milestones: Next, 16.7.P4 Jun 30, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Adding SymbolKind.FunctionPointer is changing sort ordering for serialized preferences
6 participants