Implement IEquatable to Memory and ReadOnlyMemory

[Gnbrkm41]

Related: #32905

This PR includes the changes required to implement IEquatable to Memory and ReadOnlyMemory, and tests that checks for correct behaviours of the interface.

[Gnbrkm41]

Note: VS went crazy and weren't doing any validation at all for me... Hope not, but there is a slim chance that I might have missed some silly mistakes.

Error	NETSDK1013	The TargetFramework value '' was not recognized. It may be misspelled. If not, then the TargetFrameworkIdentifier and/or TargetFrameworkVersion properties must be specified explicitly.	System.Memory (ref\System.Memory)	C:\Program Files\dotnet\sdk\3.0.100-preview3-010431\Sdks\Microsoft.NET.Sdk\targets\Microsoft.NET.TargetFrameworkInference.targets	93	
Error	NETSDK1013	The TargetFramework value '' was not recognized. It may be misspelled. If not, then the TargetFrameworkIdentifier and/or TargetFrameworkVersion properties must be specified explicitly.	System.Memory.Tests	C:\Program Files\dotnet\sdk\3.0.100-preview3-010431\Sdks\Microsoft.NET.Sdk\targets\Microsoft.NET.TargetFrameworkInference.targets	93	
Error	NETSDK1013	The TargetFramework value '' was not recognized. It may be misspelled. If not, then the TargetFrameworkIdentifier and/or TargetFrameworkVersion properties must be specified explicitly.	System.Memory (src\System.Memory)	C:\Program Files\dotnet\sdk\3.0.100-preview3-010431\Sdks\Microsoft.NET.Sdk\targets\Microsoft.NET.TargetFrameworkInference.targets	93	

Might be nice if I can figure out why this is happening, because it happened on me when I was trying to work on VisualBasic.Core solution as well. But besides... please have a look for possible weird stuffs among the changes.

[Gnbrkm41]

/cc @ahsonkhan

[ahsonkhan]

Depends on dotnet/coreclr#23586 which is why CI is failing.

[ahsonkhan]

Some more interesting test cases/permutations. Otherwise, LGTM.

[ahsonkhan]

Might be nice if I can figure out why this is happening

I am not sure why that's happening. Did you run build at the root of the repo first (from the command line)? Try that and then re-open the solution in VS. I don't see such errors in my instance of VS.

[Gnbrkm41]

Try that and then re-open the solution in VS.

Appears that VS restart fixed it. What a bug. 🤣

[Gnbrkm41]

Thoughts on the current tests.

[Gnbrkm41]

D:\a\1\s\.packages\microsoft.dotnet.apicompat\1.0.0-beta.19177.11\build\Microsoft.DotNet.ApiCompat.targets(72,5): error : CannotRemoveBaseTypeOrInterface : Type 'System.Memory<T>' does not implement interface 'System.IEquatable<System.Memory<T>>' in the implementation but it does in the contract. [D:\a\1\s\src\System.Runtime\src\System.Runtime.csproj]
D:\a\1\s\.packages\microsoft.dotnet.apicompat\1.0.0-beta.19177.11\build\Microsoft.DotNet.ApiCompat.targets(72,5): error : CannotRemoveBaseTypeOrInterface : Type 'System.ReadOnlyMemory<T>' does not implement interface 'System.IEquatable<System.ReadOnlyMemory<T>>' in the implementation but it does in the contract. [D:\a\1\s\src\System.Runtime\src\System.Runtime.csproj]
D:\a\1\s\.packages\microsoft.dotnet.apicompat\1.0.0-beta.19177.11\build\Microsoft.DotNet.ApiCompat.targets(85,5): error : ApiCompat failed for 'D:\a\1\s\artifacts\bin\System.Runtime\uapaot-Windows_NT-Release\System.Runtime.dll' [D:\a\1\s\src\System.Runtime\src\System.Runtime.csproj]

Shouldn't #36501 fix this? it's been merged and they clearly implement these interfaces. (src/Common/src/CoreLib/System/Memory.cs and ReadOnlyMemory.cs)

[ahsonkhan]

Shouldn't #36501 fix this? it's been merged and they clearly implement these interfaces.

That just mirrored the sources. We need the binary dependencies to be updated. I believe this PR will update it: #36480

[Gnbrkm41]

The PR seems to have been closed due to merging conflicts; has there been other PRs that updated the binary dependencies?

[ahsonkhan]

Here's the next one: #36596

[Gnbrkm41]

Can we try running tests now? Guess it's merged now (although it seems like Maestro had some hiccups).

[ahsonkhan]

/azp run corefx-ci

[azure-pipelines]

Azure Pipelines successfully started running 1 pipeline(s).

[Gnbrkm41]

Among the checks that have just finished running, there were three failures, of which two (Packaging All Configurations x64_Debug, Windows UWP_NETNative_x86_Release) are ApiCompat failures:

D:\a\1\s\.packages\microsoft.dotnet.apicompat\1.0.0-beta.19205.6\build\Microsoft.DotNet.ApiCompat.targets(72,5): error : CannotRemoveBaseTypeOrInterface : Type 'System.Memory<T>' does not implement interface 'System.IEquatable<System.Memory<T>>' in the implementation but it does in the contract. [D:\a\1\s\src\System.Runtime\src\System.Runtime.csproj]
D:\a\1\s\.packages\microsoft.dotnet.apicompat\1.0.0-beta.19205.6\build\Microsoft.DotNet.ApiCompat.targets(72,5): error : CannotRemoveBaseTypeOrInterface : Type 'System.ReadOnlyMemory<T>' does not implement interface 'System.IEquatable<System.ReadOnlyMemory<T>>' in the implementation but it does in the contract. [D:\a\1\s\src\System.Runtime\src\System.Runtime.csproj]
D:\a\1\s\.packages\microsoft.dotnet.apicompat\1.0.0-beta.19205.6\build\Microsoft.DotNet.ApiCompat.targets(85,5): error : ApiCompat failed for 'D:\a\1\s\artifacts\bin\System.Runtime\uapaot-Windows_NT-Debug\System.Runtime.dll' [D:\a\1\s\src\System.Runtime\src\System.Runtime.csproj]
D:\a\1\s\.packages\microsoft.dotnet.apicompat\1.0.0-beta.19205.6\build\Microsoft.DotNet.ApiCompat.targets(72,5): error : CannotRemoveBaseTypeOrInterface : Type 'System.Memory<T>' does not implement interface 'System.IEquatable<System.Memory<T>>' in the implementation but it does in the contract. [D:\a\1\s\src\System.Memory\src\System.Memory.csproj]
D:\a\1\s\.packages\microsoft.dotnet.apicompat\1.0.0-beta.19205.6\build\Microsoft.DotNet.ApiCompat.targets(72,5): error : CannotRemoveBaseTypeOrInterface : Type 'System.ReadOnlyMemory<T>' does not implement interface 'System.IEquatable<System.ReadOnlyMemory<T>>' in the implementation but it does in the contract. [D:\a\1\s\src\System.Memory\src\System.Memory.csproj]
D:\a\1\s\.packages\microsoft.dotnet.apicompat\1.0.0-beta.19205.6\build\Microsoft.DotNet.ApiCompat.targets(85,5): error : ApiCompat failed for 'D:\a\1\s\artifacts\bin\System.Memory\uapaot-Windows_NT-Debug\System.Memory.dll' [D:\a\1\s\src\System.Memory\src\System.Memory.csproj]

And one (Windows x64_Debug) is test failure that is not related to the changes made in this PR: NullReferenceException in System.Net.Http.Functional.Tests.GetAsync_IPv6LinkLocalAddressUri_Success.

[ahsonkhan]

I believe you have to add the exception to the System.Runtime API compat baseline, for uapaot.

corefx/src/System.Runtime/src/ApiCompatBaseline.uapaot.txt

Line 1 in ce62a08

Compat issues with assembly System.Runtime:

So, just add the following to the file:

CannotRemoveBaseTypeOrInterface : Type 'System.Memory<T>' does not implement interface 'System.IEquatable<System.Memory<T>>' in the implementation but it does in the contract.
CannotRemoveBaseTypeOrInterface : Type 'System.ReadOnlyMemory<T>' does not implement interface 'System.IEquatable<System.ReadOnlyMemory<T>>' in the implementation but it does in the contract. 

@ericstj, @safern, please correct me if I am mistaken.

[ahsonkhan]

Do you mind filing an issue for that (labeled area-System.Net.Http)?

System.Net.Http.Functional.Tests.SocketsHttpHandlerTest_HttpClientHandlerTest_Http2/GetAsync_IPv6LinkLocalAddressUri_Success
System.Net.Http.Functional.Tests.PlatformHandler_HttpClientHandlerTest/GetAsync_IPv6LinkLocalAddressUri_Success
System.Net.Http.Functional.Tests.PlatformHandler_HttpClientHandlerTest/GetAsync_IPv6LinkLocalAddressUri_Success
https://mc.dot.net/#/user/dotnet-bot/pr~2Fdotnet~2Fcorefx~2Frefs~2Fpull~2F36497~2Fmerge/test~2Ffunctional~2Fcli~2F/20190411.32/workItem/System.Net.Http.Functional.Tests/analysis/xunit/System.Net.Http.Functional.Tests.PlatformHandler_HttpClientHandlerTest~2FGetAsync_IPv6LinkLocalAddressUri_Success
Windows.10.Amd64.ClientRS4.ES.Open-x64-Debug

Unhandled Exception of Type System.NullReferenceException
Message :
System.NullReferenceException : Object reference not set to an instance of an object.
Stack Trace :
   at System.Net.Test.Common.LoopbackServer.CreateServerAsync(Func`2 funcAsync, Options options) in D:\a\1\s\src\Common\tests\System\Net\Http\LoopbackServer.cs:line 59
   at System.Net.Http.Functional.Tests.HttpClientHandlerTest.GetAsync_IPv6LinkLocalAddressUri_Success() in D:\a\1\s\src\System.Net.Http\tests\FunctionalTests\HttpClientHandlerTest.cs:line 301
--- End of stack trace from previous location where exception was thrown ---

[Gnbrkm41]

I think 39a3f2a should do from what I can see, but the checks for Packaging All Configurations x64_Debug, Windows UWP_NETNative_x86_Release are still failing due to the same reason (ApiCompat).

[ahsonkhan]

I am not sure why that is still failing.

@ericstj, Any ideas? Do we need to update the baseline under shims too?

corefx/src/shims/ApiCompatBaseline.uapaot.netstandard.txt

Line 78 in a114f57

Compat issues with assembly System.Runtime:

I don't think it matters, but other files have a total issue count in the file that the S.Runtime one is missing.

corefx/src/System.Runtime.Loader/src/ApiCompatBaseline.uapaot.txt

Line 31 in ce62a08

Total Issues: 29

[ericstj]

It looks to me like this is happening for System.Memory:
https://dev.azure.com/dnceng/public/_build/results?buildId=152393&view=logs&jobId=45bce49c-c830-5a83-092f-833753df204f&taskId=a227182a-5109-5e62-9f74-d28307a2d6d2&lineStart=1627&lineEnd=1629&colStart=1&colEnd=279

D:\a\1\s.packages\microsoft.dotnet.apicompat\1.0.0-beta.19205.6\build\Microsoft.DotNet.ApiCompat.targets(72,5): error : CannotRemoveBaseTypeOrInterface : Type 'System.Memory' does not implement interface 'System.IEquatable<System.Memory>' in the implementation but it does in the contract. [D:\a\1\s\src\System.Memory\src\System.Memory.csproj]

[ahsonkhan]

System.Memory.csproj

Lol, totally missed that! I thought both errors were from System.Runtime.csproj. Good eye :)

@Gnbrkm41, so, the solution is to add the API compat baseline exception there as well.

The baseline file for System.Memory was removed earlier by @MichalStrehovsky (here - #33956) since it wasn't necessary anymore. Add it back with the two entries.

https://github.com/dotnet/corefx/blob/c9e735d2569d9322453314fd3b0d015f62157113/src/System.Memory/src/ApiCompatBaseline.uapaot.txt

[Gnbrkm41]

Thought I could remove ones in in System.Runtime, apparently I need both 🦆