Support byref fields in GenAPI

[AaronRobinsonMSFT]

Support for ref fields is being added in .NET 7 / C# 11.0.
This will impact reference assembly generation as ref fields impact
how C# consumes types that contain them. A ref field represents a
change to the metadata format and that can cause issues with tool
chains that are not updated to understand this metadata change.
A concrete example is C++/CLI which will likely error if it consumes
a ref field. If the ref field is public, it will be by GenAPI - we are required
to expose the real public API surface.

The following invariants must be preserved when not exposing private
ref fields:

• The containing type can never be considered unmanaged
• The type of the field, sans ref, is still important for generic
  expansion calculations

Fixes dotnet/runtime#63751

/cc @jaredpar @ericstj @carlossanlop @jkoritzinsky

[AaronRobinsonMSFT]

@jaredpar or @tannergooding Can I get a sign off on this or should we wait for @ericstj to get back?

[tannergooding]

Changes, minus the ref readonly question, LGTM.

There is generally an ask someone from @dotnet/area-infrastructure-libraries review as well

[ViktorHofer]

The general ask from our side is that you test your changes by building dotnet/runtime with it and that you make sure that the behavior doesn't unexpectedly change. The Cci code base is quite old and not actively developed by us and there are no tests checked into Arcade at all. Hence, we rely on manual tests.

@carlossanlop can you please also take a look?

[AaronRobinsonMSFT]

@ViktorHofer Ran over the existing code base with no issues. Thanks for the suggestion.

[ViktorHofer]

ApiCompat uses CCI and runs as part of dotnet/runtime's build. You would need to make sure that you use the just produced package from arcade. For GenAPI, you want to use it on a few projects to make sure that the changes work correctly.

[AaronRobinsonMSFT]

ApiCompat uses CCI and runs as part of dotnet/runtime's build. You would need to make sure that you use the just produced package from arcade. For GenAPI, you want to use it on a few projects to make sure that the changes work correctly.

I patched my local NuGet with the updated bits and then built the coreclr and libraries subsets from a clean repo state. Is that sufficient?

[ViktorHofer]

If the updated bits contain the ApiCompat package then this should be sufficient to make sure that ApiCompat didn't regress. GenAPI though as mentioned doesn't run as part of the build. You would need to invoke it via the target.

[AaronRobinsonMSFT]

If the updated bits contain the ApiCompat package then this should be sufficient to make sure that ApiCompat didn't regress. GenAPI though as mentioned doesn't run as part of the build. You would need to invoke it via the target.

I have no idea how these things work in dotnet/runtime. Is there a specific command to use? I ran this locally on an IL project by manually including the targets file and setting <GenerateReferenceAssemblySource>true</GenerateReferenceAssemblySource>. What is the process for running this in dotnet/runtime or on a few projects?

[ViktorHofer]

What is the process for running this in dotnet/runtime or on a few projects?

https://github.com/dotnet/runtime/blob/57bfe474518ab5b7cfe6bf7424a79ce3af9d6657/docs/coding-guidelines/updating-ref-source.md

[AaronRobinsonMSFT]

@ViktorHofer There is substantial drift in the GenAPI tooling. For example, the Numerics work is all "incorrect" according to GenAPI - @tannergooding. I'm not sure how useful this tool is based on the deltas I am seeing even after reverting my changes.

-        int System.Numerics.IBinaryInteger<nint>.GetByteCount() { throw null; }
-        int System.Numerics.IBinaryInteger<nint>.GetShortestBitLength() { throw null; }
-        bool System.Numerics.IBinaryInteger<nint>.TryWriteBigEndian(System.Span<byte> destination, out int bytesWritten) { throw null; }
-        bool System.Numerics.IBinaryInteger<nint>.TryWriteLittleEndian(System.Span<byte> destination, out int bytesWritten) { throw null; }
+        int System.Numerics.IBinaryInteger<System.IntPtr>.GetByteCount() { throw null; }
+        int System.Numerics.IBinaryInteger<System.IntPtr>.GetShortestBitLength() { throw null; }
+        bool System.Numerics.IBinaryInteger<System.IntPtr>.TryWriteBigEndian(System.Span<byte> destination, out int bytesWritten) { throw null; }
+        bool System.Numerics.IBinaryInteger<System.IntPtr>.TryWriteLittleEndian(System.Span<byte> destination, out int bytesWritten) { throw null; }

Seems like we are getting to a critical point where dotnet/runtime#58163 needs to be considered higher priority.

[tannergooding]

@AaronRobinsonMSFT that's because #9421 hasn't been merged yet, but the actual ref assembly needed to be correct and so was generated using the local copy of the tool which includes my PR.

[ViktorHofer]

@AaronRobinsonMSFT I just merged @tannergooding's change. Can you please rebase your PR on top of arcade main and try it out again? This specific difference that you were seeing shouldn't exist anymore. Of course there are other known differences.

[AaronRobinsonMSFT]

@ViktorHofer Looks a lot better. There are many changes, but the ones I examined are all movement around. I also confirmed the output is expected based on the IL project I am using until we get C# language support.

[ViktorHofer]

Thanks @AaronRobinsonMSFT