Inconsistent const in parameters of RmRegisterResources

[KalleOlaviNiemitalo]

Microsoft.Windows.CsWin32 0.3.106 (using Microsoft.Windows.SDK.Win32Metadata 60.0.34-preview) generates a C# convenience wrapper for RmRegisterResources, with this signature:

internal static unsafe winmdroot.Foundation.WIN32_ERROR RmRegisterResources(
    uint dwSessionHandle,
    ReadOnlySpan<winmdroot.Foundation.PCWSTR> rgsFileNames,
    Span<winmdroot.System.RestartManager.RM_UNIQUE_PROCESS> rgApplications,
    ReadOnlySpan<winmdroot.Foundation.PCWSTR> rgsServiceNames)

The rgsFileNames and rgsServiceNames parameters are ReadOnlySpan but the rgApplications parameter is Span, even though there is no such difference in the C declaration of RmRegisterResources: https://github.com/microsoft/win32metadata/blob/c2b7ea95b5ae31b76db8f45bbe353b48a58a65f7/generation/WinSDK/RecompiledIdlHeaders/um/RestartManager.h#L206-L214

ildasm on the metadata shows that the rgsFileNames and rgsServiceNames parameters have a ConstAttribute but the rgApplications parameter doesn't:

.method public hidebysig static pinvokeimpl("rstrtmgr.dll" nomangle winapi) 
        valuetype Windows.Win32.Foundation.WIN32_ERROR 
        RmRegisterResources([in] uint32 dwSessionHandle,
                            [in] uint32 nFiles,
                            [in][opt] valuetype Windows.Win32.Foundation.PWSTR* rgsFileNames,
                            [in] uint32 nApplications,
                            [in][opt] valuetype Windows.Win32.System.RestartManager.RM_UNIQUE_PROCESS* rgApplications,
                            [in] uint32 nServices,
                            [in][opt] valuetype Windows.Win32.Foundation.PWSTR* rgsServiceNames) cil managed
{
  .custom instance void Windows.Win32.Foundation.Metadata.SupportedOSPlatformAttribute::.ctor(string) = ( 01 00 0F 77 69 6E 64 6F 77 73 36 2E 30 2E 36 30   // ...windows6.0.60
                                                                                                          30 30 00 00 )                                     // 00..
  .custom instance void Windows.Win32.Foundation.Metadata.DocumentationAttribute::.ctor(string) = ( 01 00 62 68 74 74 70 73 3A 2F 2F 6C 65 61 72 6E   // ..bhttps://learn
                                                                                                    2E 6D 69 63 72 6F 73 6F 66 74 2E 63 6F 6D 2F 77   // .microsoft.com/w
                                                                                                    69 6E 64 6F 77 73 2F 77 69 6E 33 32 2F 61 70 69   // indows/win32/api
                                                                                                    2F 72 65 73 74 61 72 74 6D 61 6E 61 67 65 72 2F   // /restartmanager/
                                                                                                    6E 66 2D 72 65 73 74 61 72 74 6D 61 6E 61 67 65   // nf-restartmanage
                                                                                                    72 2D 72 6D 72 65 67 69 73 74 65 72 72 65 73 6F   // r-rmregisterreso
                                                                                                    75 72 63 65 73 00 00 )                            // urces..
  .param [3]
  .custom instance void Windows.Win32.Foundation.Metadata.ConstAttribute::.ctor() = ( 01 00 00 00 ) 
  .custom instance void Windows.Win32.Foundation.Metadata.NativeArrayInfoAttribute::.ctor() = ( 01 00 01 00 53 06 0F 43 6F 75 6E 74 50 61 72 61   // ....S..CountPara
                                                                                                6D 49 6E 64 65 78 01 00 )                         // mIndex..
  .param [5]
  .custom instance void Windows.Win32.Foundation.Metadata.NativeArrayInfoAttribute::.ctor() = ( 01 00 01 00 53 06 0F 43 6F 75 6E 74 50 61 72 61   // ....S..CountPara
                                                                                                6D 49 6E 64 65 78 03 00 )                         // mIndex..
  .param [7]
  .custom instance void Windows.Win32.Foundation.Metadata.ConstAttribute::.ctor() = ( 01 00 00 00 ) 
  .custom instance void Windows.Win32.Foundation.Metadata.NativeArrayInfoAttribute::.ctor() = ( 01 00 01 00 53 06 0F 43 6F 75 6E 74 50 61 72 61   // ....S..CountPara
                                                                                                6D 49 6E 64 65 78 05 00 )                         // mIndex..
}

Is the const in the PCWSTR type leaking to the wrong level of pointer indirection?

[mikebattista]

Const is applied when the underlying definition uses const, so what you shared looks expected.

Is there a usability issue here? Or are you just pointing out a perceived inconsistency?

[KalleOlaviNiemitalo]

I hoped the _In_reads_opt_(nApplications) could have been treated as implying that the function does not write to the array, and the friendly C# overload would then have used ReadOnlySpan<winmdroot.System.RestartManager.RM_UNIQUE_PROCESS> rgApplications for this parameter so that my application could have called it without needing writable span.

[mikebattista]

It's possible the projection could handle this in a different way. @AArnott?

I wouldn't apply Const here as that represents const, so either the projection could improve how it handles cases like this based on the existing metadata, or perhaps you're proposing a new attribute.

[KalleOlaviNiemitalo]

I kind of expected it might have been a property in NativeArrayInfoAttribute.

[KalleOlaviNiemitalo]

Or perhaps the [in] should be projected to ReadOnlySpan.

[riverar]

The SAL annotation _In_reads_opt_ indicates read-only access, so I think we just need to add [Const] to rgApplications.

[AArnott]

Here is the metadata in a more human parseable fashion (ILSpy C# decompiled):

[Optional][In][Const][NativeArrayInfo(CountParamIndex = 1)] PWSTR* rgsFileNames
[Optional][In][NativeArrayInfo(CountParamIndex = 3)] RM_UNIQUE_PROCESS* rgApplications, 
[Optional][In][Const][NativeArrayInfo(CountParamIndex = 5)] PWSTR* rgsServiceNames

What is the intended difference between [In] and [Const] when they appear in the metadata? I believe [In] by itself is meant to mean the receiver will not change the data pointed to in any way. That would support using ReadOnlySpan for all three parameters. PCWSTR isn't a type in the metadata, so I guess [Const] PWSTR* is meant to infer PCWSTR*, which seems to be how CsWin32 is already interpreting it.

If that all seems correct, then I guess CsWin32 just needs to change to infer that [In] (in the absence of [Out] also appearing) means a native array will never be changed and thus can be declared as ReadOnlySpan.

[mikebattista]

What is the intended difference between [In] and [Const] when they appear in the metadata? I believe [In] by itself is meant to mean the receiver will not change the data pointed to in any way. That would support using ReadOnlySpan for all three parameters. PCWSTR isn't a type in the metadata, so I guess [Const] PWSTR* is meant to infer PCWSTR*, which seems to be how CsWin32 is already interpreting it.

Const just means the keyword const was used in the original header definition (in this case PCWSTR uses const while RM_UNIQUE_PROCESS does not). It's not applied based on any other semantics. In, Optional, and NativeArrayInfo are based on the SAL annotations.

At least from a metadata perspective, it looks like we're staying true to the headers.

If that all seems correct, then I guess CsWin32 just needs to change to infer that [In] (in the absence of [Out] also appearing) means a native array will never be changed and thus can be declared as ReadOnlySpan.

If you have a reliable way to interpret the existing metadata in the projection, then that seems like the right approach. I wouldn't add Const here.

[AArnott]

I'll move the issue over to CsWin32 then. Thanks.