Microsoft.Dotnet.Compatibility analyzer doesn't work with the NumericIntPtr feature

[tannergooding]

dotnet/runtime#69627 updated the dotnet/runtime repo to utilize the NumericIntPtr feature from C# 11: dotnet/csharplang#6065

This resulted in the compatibility analyzer misfiring with many errors such as:

/__w/1/s/.packages/microsoft.dotnet.compatibility/2.0.0-alpha.1.21525.11/build/Microsoft.NET.Compatibility.Common.targets(33,5): error CP0002: Member 'Microsoft.Extensions.Caching.Memory.PostEvictionDelegate.PostEvictionDelegate(object, System.IntPtr)' exists on lib/net6.0/Microsoft.Extensions.Caching.Abstractions.dll but not on lib/net7.0/Microsoft.Extensions.Caching.Abstractions.dll [/__w/1/s/src/libraries/Microsoft.Extensions.Caching.Abstractions/src/Microsoft.Extensions.Caching.Abstractions.csproj]
##[error].packages/microsoft.dotnet.compatibility/2.0.0-alpha.1.21525.11/build/Microsoft.NET.Compatibility.Common.targets(33,5): error CP0002: (NETCORE_ENGINEERING_TELEMETRY=Build) Member 'Microsoft.Extensions.Caching.Memory.PostEvictionDelegate.PostEvictionDelegate(object, System.IntPtr)' exists on lib/net6.0/Microsoft.Extensions.Caching.Abstractions.dll but not on lib/net7.0/Microsoft.Extensions.Caching.Abstractions.dll

This is despite the underlying assemblies still containing the correct signatures. I noticed the version was out of date and so grabbed the latest package available, 2.0.0-preview.4.22252.4, to see the same error.

[tannergooding]

This may automatically get fixed when the SDK picks up a compiler version with this feature, but it would be good to have explicit tests/validation that this is correctly handled.

Once there is analyzer with the fix then dotnet/runtime should have its CompatibilitySuppressions.xml files regenerated to no longer include the misfires.

[ViktorHofer]

Spoke with @tannergooding about this offline recently and here's our discussion dump. I think we want to refine the MemberMustExist rule to not flag for the cases mentioned below. Ideally we would do this before we ship .NET 7 to not cause unreasonable noise when customers start using the 7.0 version of ApiCompat / PackageValidation.

@ViktorHofer

I think I understand the issue. The net6.0 assets bind against the old API vs the net7.0 assets now bind against nint and thus the errors are emitted. Updating the version of codeanalysis doesn't help here as it in fact is an API compat difference. Does that make sense?
Should APICompat ignore the member difference of IntPtr and nint?

@tannergooding

On .NET 7 and later, yes
For .NET 6 and prior the difference still matters "somewhat". It's not a binary break, it is in very specific scenarios possibly a behavioral change on recompile
Basically on .NET 7 and later IntPtr and nint are the same in the way Int32 and int are the same. So on .NET 7 and later there is no compat concerns, they are identical in every way.
On .NET 6 nint is [NativeInteger] IntPtr and has additional functionality (operator support) and can't access a few members (like nint.Zero isn't valid). There is an identity conversion (even for generics and arrays) between IntPtr and nint though, so this is normally a non-issue. It won't break binary compat, might subtly impact behavior, and in the very rare case would break source

@ViktorHofer

I see. I wonder if we want to build in an exception when a <= net6.0 asset is compared with a >= net7.0 asset. Currently as you noticed, ApiCompat flags this as a missing member and I wonder if that's actually useful to flag.

@tannergooding

Right, I think that's what we need. For netstandard2.0, net3.1, and net6.0 we want to keep it "as is" (preserve nint vs IntPtr across versions).
Once we're on net7.0+ then we shouldn't care (and actually can't because the compiler won't emit the [NativeInteger] attribute for this case)
so netstandard2.0 vs netstandard2.0 should flag
but net6.0 vs net7.0 shouldn't

@ViktorHofer

Hmm that might be tricky as at this level we don't differentiate based on target frameworks but just mere assemblies. Maybe the CodeAnalysis APIs make it possible to differentiate between the new nint support and the old behavior?
Can you elaborate on the [NativeInteger] attribute? Would that be something that we could search for, to differentiate?

@tannergooding

Roslyn doesn't technically differentiate on TFM either.
It looks for System.Runtime.CompilerServices.RuntimeFeatures.NumericIntPtr being defined

[ericstj]

Basically on .NET 7 and later IntPtr and nint are the same in the way Int32 and int are the same. So on .NET 7 and later there is no compat concerns, they are identical in every way.

@tannergooding is the same true for method signatures and interface implementations? If I define an method as void DoSomething(IntPtr) and it's overridden/implemented as void DoSomething(nint) will the runtime treat this as a legitimate implementation?

[tannergooding]

IntPtr and nint are identical types to the runtime and always have been, even when targeting .NET Framework or .NET Standard. The C# compiler has never emitted any modreq or modopt differentiating them, only the [NativeInteger] attribute.

Other languages will just see IntPtr. C# itself likewise maintained an identity conversion between its own concept of IntPtr and nint, which means that IntPtr[] and nint[] are interchangeable. As are ref IntPtr with ref nint and out IntPtr with out nint, etc.

The only real difference is the members exposed by each. Prior to .NET 7, IntPtr exposed exactly the members declared by the BCL in System.IntPtr. While nint hid a couple properties (like Zero) and exposed language operators that mapped to the IL opcodes that have been around since .NET Framework 1.0. This means prior to .NET 7, we want to continue treating them differently because there are some minor differences for the caller with regards to return values. There aren't really any differences for parameters due to the identity conversion and that the callee sees it as the type in its own signature.

For .NET 7 and later (or more specifically, any runtime that defines System.Runtime.CompilerServices.RuntimeFeatures.NumericIntPtr), the compiler no longer emits the NativeInteger attribute and these types are true aliases of eachother (in the same way Int32 and int are true aliases of eachother). So we don't want or need to treat them differently anymore.

[ericstj]

I see. When we have references provided does roslyn see the net6 nint (IntPtr with NativeIntegerAttribute) and net7 nint (IntPtr) as the same thing? I guess if we don't have references, it might mistake the net7 nint for IntPtr since it can't tell that it has the feature. This is an interesting short-coming to running without references. FWIW we do "disable" other rules that are sensitive to having references present. A possible solution here might be to assume that when references are not present that the feature would have been present and ignore the diff.

[tannergooding]

I'm not sure how that's surfaced when there isn't a corelib available. I would presume that since corelib isn't available it means that System.Runtime.CompilerServices.RuntimeFeatures.NumericIntPtr isn't available either and so it would consider IntPtr to be "not nint" for some .NET 7 assembly (since it also has no [NativeInteger] attribute).

CC. @jaredpar or @cston might be able to answer.

[cston]

If RuntimeFeatures.NumericIntPtr is not found, it looks like the compiler will treat a type reference to System.IntPtr as System.IntPtr unless there is an associated [NativeInteger] attribute.

[ViktorHofer]

When comparing .NET 6 reference assemblies against .NET 7 reference assemblies, I see 1127 differences being logged because of this issue. This is noise that customers might experience as well and thus I think we should fix this in the .NET 7 timeframe.

I don't know yet what a fix would look like. Presumably ignoring the difference in the equality symbol comparer makes most sense.