Update Marshal.QueryInterface() argument modifier (take 2)

[Sergio0694]

Note

This reverts commit a60b66d (from #91983)

Follow up from a conversation with @AaronRobinsonMSFT offline. Now that .NET 8 is done and we're early in the .NET 9 cycle (and most importantly, all tooling is already updated to correctly handle ref readonly), we can attempt again to change Marshal.QueryInterface to use in for the Guid parameter (as approved in #85911). I've also updated most Marshal.QueryInterface callsites through libraries and tests to leverage the new in parameter modifier.

F# also just got a fix to handle ref readonly method parameters: dotnet/fsharp#16232.

cc. @tannergooding @vzarytovskii

[ghost]

Note regarding the new-api-needs-documentation label:

This serves as a reminder for when your PR is modifying a ref *.cs file and adding/modifying public APIs, please make sure the API implementation in the src *.cs file is documented with triple slash comments, so the PR reviewers can sign off that change.

[ghost]

Tagging subscribers to this area: @dotnet/interop-contrib
See info in area-owners.md if you want to be subscribed.

Issue Details

---

[!NOTE]
This reverts commit a60b66d (from ##91983)

Follow up from a conversation with @AaronRobinsonMSFT offline. Now that .NET 8 is done and we're early in the .NET 9 cycle (and most importantly, all tooling is already updated to correctly handle ref readonly), we can attempt again to change Marshal.QueryInterface to use in for the Guid parameter (as approved in #85911). I've also updated most Marshal.QueryInterface callsites through libraries and tests to leverage the new in parameter modifier.

F# also just got a fix to handle ref readonly method parameters: dotnet/fsharp#16232.

cc. @tannergooding @vzarytovskii

Author:	Sergio0694
Assignees:	-
Labels:	area-System.Runtime.InteropServices, new-api-needs-documentation
Milestone:	-

[AaronRobinsonMSFT]

We also need to loop with any interop source generator teams about this change. Specifically CsWinRT and CsWin32 - @manodasanW and @AArnott.

[Sergio0694]

Alright so to recap:

• F# is fixed and support should drop in .NET 8.0.1 already (Use of ref readonly instead of in in libraries is a breaking change for F# callers #94317 (comment))
• For CsWinRT (cc. @manodasanW) and CsWin32 (cc. @AArnott) there's two approaches based on context:
  • For multi-targeted sources, they can just use ref, and suppress the warning on .NET 8 (CS9191). Even without suppressing it, it'll just be a warning though, and not an actual build error. Of course, suppressing it is recommended, but just saying, it should never fail to built outright in this scenario.
  • For source-generated files, they can either also just use ref and suppress the warning again, or detect the signature of the method, pass it down to the incremental model and use that to decide what modifier to emit. Either is doable, of course the latter is a tiny bit more involved.

@AaronRobinsonMSFT does this sound acceptable? And do we need any specific other action here before merging? I could also open tracking issues for this in the other repos if it helped, so it doesn't risk getting lost?

[AaronRobinsonMSFT]

@Sergio0694 It looks like CsWin32 created some mitigation for addressing microsoft/CsWin32#1014. Therefore, I think CsWin32 is good. Please confirm the fix for that issue makes sense. We should also have a tracking issue in CsWinRT that references this issue.

[Sergio0694]

Ooh that's awesome, I had missed that one in CsWin32! Yeah the fix seems completely fine for this change as well. Using in doesn't require an explicit modifier at the callsite, but just like with C# 11 and lower, explicitly using in is perfectly fine and doesn't result in any warning. So if they already have that split for .NET 8 and greater, changing ref readonly to in for them should not result in any difference with respect to compile errors/warnings.

I'll open a tracking issue in CsWinRT as well 👍

[Sergio0694]

Opened a tracking issue for CsWinRT: microsoft/CsWinRT#1388.

[AaronRobinsonMSFT]

The CsWinRT team acknowledged this change offline and they are good with responding. Given that CsWin32 has responded to similar API changes for this semantic update, I think they should be okay here too.

I'm going to retrigger this PR on the latest just to make sure we are still clean.

[AaronRobinsonMSFT]

I'm good with this. Thanks for trying again.

@tannergooding Any more thoughts on this?

[tannergooding]

@AaronRobinsonMSFT, LGTM. Feel free to merge whenever you think its ready.