dotnet / pinvoke

A library containing all P/Invoke code so you don't have to import it every time. Maintained and updated to support the latest Windows OS.
MIT License
2.12k stars 222 forks source link

FriendlyFlags.Array should generate Span<T> overloads #444

Closed AArnott closed 4 years ago

AArnott commented 4 years ago

Given a method with such a parameter:

[Friendly(FriendlyFlags.Out | FriendlyFlags.Array)] char* lpString,

We should produce an overload with parameter type Span<char>.

Also if the direction is only In:

[Friendly(FriendlyFlags.In | FriendlyFlags.Array)] char* lpString,

We should produce an overload with parameter type ReadOnlySpan<char>.

AArnott commented 4 years ago

This actually isn't well thought out. Span<T> carries both a pointer and a length in it, but only the pointer could be passed through the argument transformation. If we added a parameter to FriendlyAttribute where we could supply an offset to the length parameter, we could remove the length parameter from the friendly overload and synthesize that in the generated method based on the Span<T>.Length, but that assumes it only goes in one-way. But many p/invoke methods take a length pointer so that they can change the length to whatever length of the buffer was initialized. Still others use a second length parameter for that.

Span<char> would probably be harmful in general because some p/invokes require a length to be passed in while others assume a null-terminated string. Span<char> implies a subset of a string can be supplied but in that case there's no guarantee it's null-terminated and if it takes a length parameter instead the length of the span still doesn't matter.

So in summary, I think this is a bad idea.

AArnott commented 4 years ago

Reconsidering this, because we already generate friendly array overloads for pointers, which have length just like span. So really the concerns I mentioned above seem moot given the array precedent we have.

AArnott commented 4 years ago

Doing this right may also implicitly take care of (part of) #286 since string is implicitly convertable to ReadOnlySpan<char>.

AArnott commented 4 years ago

string is only implicitly convertible to ReadOnlySpan<char> on .NET Core 3.x. So to support .NET Core 2.1 and .NET Framework 4.5+, we may still want to produce string overloads.

AArnott commented 4 years ago

I'm also playing with an idea to make the friendly overload omit the 'length' parameter (or change from ref to out) so that the length can be automatically provided by the length of the array/span.