create ReadOnlySpan<char> from string#7276
Conversation
| /// <param name="text">The target string.</param> | ||
| /// <exception cref="System.ArgumentNullException">Thrown when <paramref name="text"/> is a null | ||
| /// reference (Nothing in Visual Basic).</exception> | ||
| public static ReadOnlySpan<char> Slice(this string text) |
There was a problem hiding this comment.
I am not sure that Slice is a good name for this method. @KrzysztofCwalina ?
There was a problem hiding this comment.
Would it be better to expose this as an implicit cast?
There was a problem hiding this comment.
Ah, this is not on String. Well, in this case case won't work. Given that, we would have to add another name, which is probably not worth it, i.e. Slice() seems like the next best choice besides the operator.
There was a problem hiding this comment.
Maybe I should add the implicit cast operator and keep the Slice method?
There was a problem hiding this comment.
@KrzysztofCwalina I can not add implicit cast. I get compiler error User-defined conversion must convert to or from the enclosing type (if I add implicit cast to ReadOnlySpan of T to cast to ROS of char)
There was a problem hiding this comment.
Yeah, that's what I meant. In this case, I am fine with Slice(), i.e I have no better naming suggestions.
| if (text == null) | ||
| ThrowHelper.ThrowArgumentNullException(ExceptionArgument.text); | ||
|
|
||
| return new ReadOnlySpan<char>(ref JitHelpers.GetStringData(text), text.Length); |
There was a problem hiding this comment.
You should not need a new JitHelpers method. Adding a new method like ref char GetFirstCharRef() { return ref m_firstChar; } should be enough.
There was a problem hiding this comment.
@jkotas this has simplified things a lot! I pushed an update
src/mscorlib/model.xml
Outdated
| <Member Name="NonPortableCast<TFrom,TTo>(System.Span<TFrom>)" /> | ||
| <Member Name="NonPortableCast<TFrom,TTo>(System.ReadOnlySpan<TFrom>)" /> | ||
| </Type> | ||
| <Type Name="System.ReadOnlySpanExtensions"> |
There was a problem hiding this comment.
It may be useful to add this with Condition="FEATURE_SPAN_OF_T" like the other span apis.
There was a problem hiding this comment.
@jkotas I have pushed commit with the condition
|
LGTM otherwise |
|
looks good to me too. |
|
Thanks! |
@KrzysztofCwalina @jkotas the possibility of creating readonly span from string was missing, so I implemented it