Skip to content
This repository has been archived by the owner on Jul 26, 2023. It is now read-only.

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

Closed
AArnott opened this issue Dec 22, 2019 · 5 comments · Fixed by #519
Closed

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

AArnott opened this issue Dec 22, 2019 · 5 comments · Fixed by #519
Assignees

Comments

@AArnott
Copy link
Collaborator

AArnott commented Dec 22, 2019

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 AArnott self-assigned this Dec 23, 2019
@AArnott
Copy link
Collaborator Author

AArnott commented Dec 24, 2019

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 AArnott closed this as completed Dec 24, 2019
@AArnott
Copy link
Collaborator Author

AArnott commented Jul 12, 2020

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 AArnott reopened this Jul 12, 2020
@AArnott AArnott removed the wontfix label Jul 12, 2020
@AArnott
Copy link
Collaborator Author

AArnott commented Jul 13, 2020

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

@AArnott
Copy link
Collaborator Author

AArnott commented Jul 13, 2020

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
Copy link
Collaborator Author

AArnott commented Jul 13, 2020

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.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

1 participant