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

[API Proposal]: MemoryExtensions.AsSpan(this string? text, Range range) #77955

Closed
Tracked by #79053
stephentoub opened this issue Nov 6, 2022 · 6 comments · Fixed by #82794
Closed
Tracked by #79053

[API Proposal]: MemoryExtensions.AsSpan(this string? text, Range range) #77955

stephentoub opened this issue Nov 6, 2022 · 6 comments · Fixed by #82794
Assignees
Labels
api-approved API was approved in API review, it can be implemented area-System.Runtime
Milestone

Comments

@stephentoub
Copy link
Member

stephentoub commented Nov 6, 2022

Background and motivation

Some APIs return Range instances to specify subsets. You can use such a Range with a string like str[range] to get the substring, but that allocates a string. MemoryExtensions exposes AsSpan methods that take Ranges, but only for T[] and ArraySegment<T>, not string. It does have a Range-based string extension, but only AsMemory.

API Proposal

namespace System;

public static class MemoryExtensions
{
    public static ReadOnlySpan<char> AsSpan(this string? text, Range range);
}

API Usage

ReadOnlySpan<char> span = string.AsSpan(range);

Alternative Designs

No response

Risks

No response

@stephentoub stephentoub added area-System.Runtime api-ready-for-review API is ready for review, it is NOT ready for implementation labels Nov 6, 2022
@stephentoub stephentoub added this to the 8.0.0 milestone Nov 6, 2022
@ghost
Copy link

ghost commented Nov 6, 2022

Tagging subscribers to this area: @dotnet/area-system-runtime
See info in area-owners.md if you want to be subscribed.

Issue Details

Background and motivation

Some APIs return Range instances to specify subsets. You can use such a Range with a string like str[range] to get the substring, but that allocates a string. MemoryExtensions exposes AsSpan methods that take Ranges, but only T[] and ArraySegment<T>, not string. It does have a Range-based string extension, but only AsMemory.

API Proposal

namespace System;

public static class MemoryExtensions
{
    public static ReadOnlySpan<char> AsSpan(this string? text, Range range);
}

API Usage

ReadOnlySpan<char> span = string.AsSpan(range);

Alternative Designs

No response

Risks

No response

Author: stephentoub
Assignees: -
Labels:

area-System.Runtime, api-ready-for-review

Milestone: 8.0.0

@stephentoub
Copy link
Member Author

stephentoub commented Nov 6, 2022

(I have a memory of discussing this at some point, and deciding that str.AsSpan()[range] was sufficient, but I can't find that now, and if that was the decision, the presence of the array-based span methods and string-based memory method is strangely inconsistent.)

@Tornhoof
Copy link
Contributor

Tornhoof commented Nov 6, 2022

Why is the string nullable in your proposal?

@stephentoub
Copy link
Member Author

Why is the string nullable in your proposal?

For consistency with the other such methods, where null is permitted in certain circumstances, e.g.

public static ReadOnlySpan<char> AsSpan(this string? text, int start, int length)

@eerhardt
Copy link
Member

eerhardt commented Dec 6, 2022

There are some other minor inconsistencies with MemoryExtensions.

public static System.ReadOnlySpan<char> AsSpan(this string? text) { throw null; }
public static System.ReadOnlySpan<char> AsSpan(this string? text, int start) { throw null; }
public static System.ReadOnlySpan<char> AsSpan(this string? text, int start, int length) { throw null; }
public static System.Span<T> AsSpan<T>(this System.ArraySegment<T> segment) { throw null; }
public static System.Span<T> AsSpan<T>(this System.ArraySegment<T> segment, System.Index startIndex) { throw null; }
public static System.Span<T> AsSpan<T>(this System.ArraySegment<T> segment, int start) { throw null; }
public static System.Span<T> AsSpan<T>(this System.ArraySegment<T> segment, int start, int length) { throw null; }
public static System.Span<T> AsSpan<T>(this System.ArraySegment<T> segment, System.Range range) { throw null; }
public static System.Span<T> AsSpan<T>(this T[]? array) { throw null; }
public static System.Span<T> AsSpan<T>(this T[]? array, System.Index startIndex) { throw null; }
public static System.Span<T> AsSpan<T>(this T[]? array, int start) { throw null; }
public static System.Span<T> AsSpan<T>(this T[]? array, int start, int length) { throw null; }
public static System.Span<T> AsSpan<T>(this T[]? array, System.Range range) { throw null; }

  • The Array and ArraySegment overloads also have System.Index startIndex overloads along with the System.Range overload. Should we add a System.Index overload for string as well?

public static System.ReadOnlyMemory<char> AsMemory(this string? text) { throw null; }
public static System.ReadOnlyMemory<char> AsMemory(this string? text, System.Index startIndex) { throw null; }
public static System.ReadOnlyMemory<char> AsMemory(this string? text, int start) { throw null; }
public static System.ReadOnlyMemory<char> AsMemory(this string? text, int start, int length) { throw null; }
public static System.ReadOnlyMemory<char> AsMemory(this string? text, System.Range range) { throw null; }
public static System.Memory<T> AsMemory<T>(this System.ArraySegment<T> segment) { throw null; }
public static System.Memory<T> AsMemory<T>(this System.ArraySegment<T> segment, int start) { throw null; }
public static System.Memory<T> AsMemory<T>(this System.ArraySegment<T> segment, int start, int length) { throw null; }
public static System.Memory<T> AsMemory<T>(this T[]? array) { throw null; }
public static System.Memory<T> AsMemory<T>(this T[]? array, System.Index startIndex) { throw null; }
public static System.Memory<T> AsMemory<T>(this T[]? array, int start) { throw null; }
public static System.Memory<T> AsMemory<T>(this T[]? array, int start, int length) { throw null; }
public static System.Memory<T> AsMemory<T>(this T[]? array, System.Range range) { throw null; }

  • The ArraySegment AsMemory methods don't have Index or Range overloads, like we have for string and T[]. Should we add them as well to make these 2 sets of methods be consistent?

@stephentoub stephentoub removed their assignment Dec 10, 2022
@bartonjs
Copy link
Member

Since there's already an AsMemory overload with the same shape, this seems like it's just properly squaring off the feature.

Since AsSpan(string?, Index) isn't present, but AsSpan(T[], Index) and AsSpan(ArraySegment<T>, Index) do, we've added that to further square this off.

namespace System;

public static partial class MemoryExtensions
{
    public static ReadOnlySpan<char> AsSpan(this string? text, Range range);
    public static ReadOnlySpan<char> AsSpan(this string? text, Index startIndex);
}

@bartonjs bartonjs added api-needs-work API needs work before it is approved, it is NOT ready for implementation api-approved API was approved in API review, it can be implemented and removed api-ready-for-review API is ready for review, it is NOT ready for implementation api-needs-work API needs work before it is approved, it is NOT ready for implementation labels Feb 28, 2023
@stephentoub stephentoub self-assigned this Feb 28, 2023
@ghost ghost added the in-pr There is an active PR which will close this issue when it is merged label Feb 28, 2023
@ghost ghost removed the in-pr There is an active PR which will close this issue when it is merged label Mar 2, 2023
@ghost ghost locked as resolved and limited conversation to collaborators Apr 1, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
api-approved API was approved in API review, it can be implemented area-System.Runtime
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants