Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Consider providing a string.Create method which accepts a ROS<T> or ROS<char> as state #30175

Closed
john-h-k opened this issue Jul 7, 2019 · 5 comments
Labels
api-needs-work API needs work before it is approved, it is NOT ready for implementation area-System.Runtime
Milestone

Comments

@john-h-k
Copy link
Contributor

john-h-k commented Jul 7, 2019

Currently, string.Create only accepts a generic TState as the state, which means ReadOnlySpan<T> and Span<T> can't be used as state. In situations where, for example, the state is a stackalloc'd buffer which can't be converted to a ROM. It can be unpleasantly worked around with pinning and passing pointer and length manually, but it is ugly and clunky. It seems simple enough to add

public delegate void SpanReadOnlySpanAction<TSpan, TReadOnlySpan>(Span<TSpan> span, ReadOnlySpan<TReadOnlySpan> readOnlySpan)

( with potentially nicer naming )

and provide a string.Create<TState>(int length, ReadOnlySpan<TState> state, SpanReadOnlySpanAction action that is basically the same as the current impl.

@msftgits msftgits transferred this issue from dotnet/corefx Feb 1, 2020
@msftgits msftgits added this to the 5.0 milestone Feb 1, 2020
@maryamariyan maryamariyan added the untriaged New issue has not been triaged by the area owner label Feb 23, 2020
@joperezr
Copy link
Member

joperezr commented Jul 6, 2020

In order to consider this we would need a formal API proposal with rationale and usage examples.

@joperezr joperezr removed the untriaged New issue has not been triaged by the area owner label Jul 6, 2020
@joperezr joperezr modified the milestones: 5.0.0, Future Jul 6, 2020
@AlexeiScherbakov
Copy link

AlexeiScherbakov commented Jun 22, 2022

I think this issue is not about string.Create. CLR has limit that ref struct cannot be used as type arg.
Even if we add string.Create - it will not help, actually real solution must be allowing for public delegate void SpanAction<T, in TArg>(Span<T> span, TArg arg) have an any ref struct as TArg type argument.

Most simple syntetic example for is hex converting. Let we need to implement two extension methods:

public static unsafe string ToHexStringUpperCase(this ReadOnlySpan<byte> array)
{
	string ret = null;
	fixed (void* fix = array)
	{
		byte* ptr = (byte*) fix;
		ret = string.Create(array.Length * 2, array.Length, (span, len) =>
		{
			CharSpanWriteHexExtension.WriteByteArrayAsHexUpperCase(span, ptr, len);
		});
	}
	return ret;
}

public static unsafe string ToHexStringUpperCase(this ReadOnlySpan<byte> array, char separator)
{
	string ret = null;
	fixed (void* fix = array)
	{
		byte* ptr = (byte*) fix;
		ret = string.Create(array.Length * 3, array.Length, (span, len) =>
		{
			ReadOnlySpan<byte> arr = new ReadOnlySpan<byte>(ptr, len);
			CharSpanWriteHexExtension.WriteByteArrayAsHexUpperCase(span, arr, separator);
		});
	}
	return ret;
}

For second method we need pass ReadOnlySpan with int and char together, for compiler it will be optimal to create something like new ref struct ValueTuple and pass it to this method as state.

So this issue is related to dotnet/csharplang#2584

@Aragas
Copy link

Aragas commented Dec 8, 2022

As a workaround for people trying to solve the issue without copying the context - pinning the span via fixed and then passing the pointer as IntPtr can also work!
You can also pass the length via a tuple ((IntPtr) pOutput, length) if needed.

int outputLength = 128;
Span<char> output = stackalloc char[outputLength];
// Some operation on output

fixed (void* pOutput = output)
{
    var str = string.Create(outputLength, (IntPtr) pOutput, static (span, ptr) =>
    {
        new Span<char>(ptr.ToPointer(), span.Length).CopyTo(span);
    });
}

And if I understand correctly, it should be possible to do the same thing without pinning now

int outputLength = 128;
Span<char> output = stackalloc char[outputLength];
// Some operation on output

ref var outputRef = ref MemoryMarshal.GetReference(output);
var outputPtr = (IntPtr) Unsafe.AsPointer(ref outputRef);
var str = string.Create(outputLength, outputPtr, static (span, ptr) =>
{
    new Span<char>(ptr.ToPointer(), span.Length).CopyTo(span);
});

@stephentoub
Copy link
Member

it should be possible to do the same thing without pinning now

Yes, e.g.

int outputLength = 128;
Span<char> output = stackalloc char[outputLength];
...
var str = string.Create(outputLength, &output, static (span, ptr) =>
{
    Span<char> output = *(ReadOnlySpan<char>*)ptr;
    ...
});

@dakersnar
Copy link
Contributor

dakersnar commented Dec 13, 2022

In order to consider this we would need a formal API proposal with rationale and usage examples.

Closing this issue for now given this. If anyone is interested in driving it forward themselves, please feel free to open a new issue that follows the API suggestion template. Our API review process and a link to the template/good example is here: https://github.com/dotnet/runtime/blob/main/docs/project/api-review-process.md#steps.

@ghost ghost locked as resolved and limited conversation to collaborators Jan 13, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
api-needs-work API needs work before it is approved, it is NOT ready for implementation area-System.Runtime
Projects
None yet
Development

No branches or pull requests

8 participants