Runtime: System.Reflection.Emit.Label should implement IEquatable

[terrajobst]

From @marek-safar's comment.

C# namespace System.Reflection.Emit { public partial struct Label : IEquatable<Label> { // Existing members public override bool Equals(object obj); public bool Equals(Label obj); public override int GetHashCode(); public static bool operator ==(Label a, Label b); public static bool operator !=(Label a, Label b); } }

Would be neat for 3.0 so we might be able to pull it into .NET Standard 2.1 as well.

[Foxtrek64]

Implementation seems simple enough, though because this is a cross-project type, I'm not entirely sure on the procedure for implementing this change. Do I need to make a PR across all of the projects below or do I just make a single PR in corefx and let a bot or something sync the changes?

https://github.com/dotnet/corefx/blob/master/src/Common/src/CoreLib/System/Reflection/Emit/Label.cs
https://github.com/dotnet/coreclr/blob/master/src/System.Private.CoreLib/shared/System/Reflection/Emit/Label.cs
https://github.com/dotnet/corert/blob/master/src/System.Private.CoreLib/shared/System/Reflection/Emit/Label.cs

Edit: Once I get clarification on this, I'd like to go ahead and claim the issue

[danmosemsft]

@terrajobst can you clarify, does this still need API review? Looks like it?

[terrajobst]

@Foxtrek64

Edit: Once I get clarification on this, I'd like to go ahead and claim the issue

The issue needs to be API approved, which we can hopefully do next Tuesday. Once down, you can work with @danmosemsft to see whether/how you can claim this.

[danmosemsft]

@Foxtrek64 you are welcome to it. I sent you a collaborator invite, if you accept I will assign it. Note this auto subscribes you to all repo comments, you will likely want to un-watch the repo to avoid the mail storm.

[danmosemsft]

BTW you can start now if you don't mind the risk it might not get approved (or in a different form)

[Foxtrek64]

I'll hold off until the review meeting. As noted in my last comment, this component seems to be spread into a few places, among them corefx, corecrl, and corert, so I want to make sure I'm focusing my time in the right place and that I follow all of the correct procedures. In the mean time though, I'll get the CLA taken care of.

[terrajobst]

@Foxtrek64

Are you still willing to do the work?

[Foxtrek64]

@terrajobst

Yes, though I still need clarification as described above before I can begin.

[terrajobst]

With respect to repo? @danmosemsft should point you the right way but I suspect you start with the implemention work in the CoreCLR repo and after the PR got merged, the reference assemblies in CoreFX. Not sure what the process for those are but @danmosemsft would know.

[danmosemsft]

Hello @Foxtrek64 you would please create two PR's at once

• In coreclr to implement it in https://github.com/dotnet/coreclr/blob/master/src/System.Private.CoreLib/shared/System/Reflection/Emit/Label.cs
• In corefx to (a) expose it in https://github.com/dotnet/corefx/blob/master/src/System.Reflection.Emit.ILGeneration/ref/System.Reflection.Emit.ILGeneration.cs#L60 (the reference assembly) and (b) to test it in https://github.com/dotnet/corefx/blob/master/src/System.Reflection.Emit.ILGeneration/tests/Label/LabelEquals.cs

General dev docs are rooted at
https://github.com/dotnet/corefx/tree/master/Documentation
specifically
https://github.com/dotnet/corefx/blob/master/Documentation/project-docs/developer-guide.md
and in that doc there is a section on how to test private CoreFX changes with private CoreCLR changes
https://github.com/dotnet/corefx/blob/master/Documentation/project-docs/developer-guide.md#testing-with-private-coreclr-bits

Once you have the two PR's up (please link them and this issue) then we will review them, then submit the CoreCLR PR, then a few hours later it will be ingested by CoreFX and we will be able to submit PR there.

[danmosemsft]

@Foxtrek64 in case it wasn't clera, the API was approved (hence the new label)

[Foxtrek64]

@danmosemsft Thanks for the info! I'll start on this in a few hours.

[Foxtrek64]

Sorry for the delay on this guys. I had a lot of trouble getting my environment to work. You should have the requested changes now though. Do let me know if I forgot anything.

[danmosemsft]

WAiting on https://github.com/dotnet/corefx/pull/35673.