Skip to content

Move System.ValueType.s_seed into System.Runtime.CompilerServices.RuntimeHelpers #73080

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

Merged
merged 1 commit into from
Jul 30, 2022

Conversation

gregg-miskelly
Copy link
Contributor

@gregg-miskelly gregg-miskelly commented Jul 29, 2022

PR #69723 added a 's_seed' field to System.ValueType. This causes the debugger to show all structs as having static members.

image

This PR moves the field and the function that used it into RuntimeHelpers.

@ghost
Copy link

ghost commented Jul 29, 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 29, 2022
@gregg-miskelly
Copy link
Contributor Author

CC @tmat

@jkotas
Copy link
Member

jkotas commented Jul 29, 2022

This still shows up as clutter in windbg/sos where DebuggerBrowsable does not kick in. For example, !DumpClass on any valuetype will have it:

0:000> !DumpClass /d 00007ffaaba39bb8
Class Name:      System.DateTime
...
              MT    Field   Offset                 Type VT     Attr            Value Name
00007ffaab8eb050  4000305      9b4         System.Int32  1   static                0 s_seed
00007ffaab8ef8f0  4000396        8        System.UInt64  1 instance           _dateData
...

It would be better to move this out of ValueType completely, e.g. to src\coreclr\System.Private.CoreLib\src\System\Runtime\CompilerServices\RuntimeHelpers.CoreCLR.cs.

@jkotas
Copy link
Member

jkotas commented Jul 29, 2022

I can move it around if you do not have time to do it.

@gregg-miskelly
Copy link
Contributor Author

@jkotas Do you mean to move just the field to RuntimeHelpers? Or the field and all the code that uses it? If the later, should I delete the method in ValueType? Or just have the method in ValueType call a method in RuntimeHelpers?

@jkotas
Copy link
Member

jkotas commented Jul 29, 2022

Move the field and the one method that uses it. Update all callers to call the method in the new place.

@gregg-miskelly gregg-miskelly changed the title Mark System.ValueType.s_seed with DebuggerBrowsableState.Never Move System.ValueType.s_seed into System.Runtime.CompilerServices.RuntimeHelpers Jul 30, 2022
…timeHelpers

PR dotnet#69723 added a 's_seed' field to
System.ValueType. This causes the debugger to show all structs as having
static members. This PR moves the field and the function that used it into RuntimeHelpers.
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.

Thank you!

@jkotas jkotas merged commit 0ba3d31 into dotnet:main Jul 30, 2022
@gregg-miskelly gregg-miskelly deleted the HideValueTypeSeed branch July 31, 2022 20:39
@ghost ghost locked as resolved and limited conversation to collaborators Aug 31, 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.

3 participants