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]: Implement IEnumerator<T> on ref struct enumerators #105276

Open
stephentoub opened this issue Jul 22, 2024 · 14 comments · May be fixed by #106309
Open

[API Proposal]: Implement IEnumerator<T> on ref struct enumerators #105276

stephentoub opened this issue Jul 22, 2024 · 14 comments · May be fixed by #106309
Labels
api-approved API was approved in API review, it can be implemented area-System.Runtime in-pr There is an active PR which will close this issue when it is merged
Milestone

Comments

@stephentoub
Copy link
Member

Background and motivation

We've shipped multiple ref struct enumerators. Previously they couldn't implement IEnumerator<T>, but now they can. We should have them do so.

API Proposal

namespace System
{
    public static class MemoryExtensions
    {
        public ref struct SpanSplitEnumerator<T>
+           : IEnumerator<Range>
    }
    public readonly ref struct ReadOnlySpan<T>
    {
        public ref struct Enumerator
+           : IEnumerator<T>
    }
    public readonly ref struct Span<T>
    {
        public ref struct Enumerator
+           : IEnumerator<T>
    }
}
namespace System.Text
{
    public ref partial struct SpanLineEnumerator
+       : IEnumerator<ReadOnlySpan<char>>

    public ref partial struct SpanRuneEnumerator
+       : IEnumerator<Rune>
}
namespace System.Numerics.Tensors
{
    public readonly ref struct ReadOnlyTensorSpan<T>
    {
        public ref struct Enumerator : 
+           : IEnumerator<T>
    }
    public readonly ref struct TensorSpan<T>
    {
        public ref struct Enumerator : 
+           : IEnumerator<T>
    }
}
namespace System.Text.RegularExpressions
{
    public ref struct ValueMatchEnumerator
+       : IEnumerator<ValueMatch>

    public ref struct ValueSplitEnumerator
+       : IEnumerator<Range>
}

Notes:

  • All remaining members of the interface not already exposed publicly will be implemented explicitly.
  • Where the T is a ref struct, the IEnumerator.Current will throw NotSupportedException.
  • Where possible with minimal interruption, IEnumerator.Reset will work, otherwise it'll throw NotSupportedException.
  • IDisposable.Dispose nops on all of them.

API Usage

static int Sum<TEnumerator>(TEnumerator enumerator) where TEnumerator : IEnumerator<int>
{
    int sum = 0;
    while (enumerator.MoveNext()) sum += enumerator.Current;
    return sum;
}

Alternative Designs

No response

Risks

No response

@stephentoub stephentoub added api-ready-for-review API is ready for review, it is NOT ready for implementation area-System.Runtime labels Jul 22, 2024
@stephentoub stephentoub added this to the 10.0.0 milestone Jul 22, 2024
Copy link
Contributor

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

@KennethHoff
Copy link

I thought "ref structs implementing interfaces" were being postponed/preview for .Net 9(?)

@stephentoub
Copy link
Member Author

I thought "ref structs implementing interfaces" were being postponed/preview for .Net 9(?)

No, it's planned for C# 13.

And this proposal is for .NET 10.

@KennethHoff
Copy link

KennethHoff commented Jul 23, 2024

I thought "ref structs implementing interfaces" were being postponed/preview for .Net 9(?)

No, it's planned for C# 13.

It sounds to me like it's (at least being set up to be) in preview for C# 13 dotnet/roslyn#73923

And this proposal is for .NET 10.

That's true 😅

@stephentoub
Copy link
Member Author

I thought "ref structs implementing interfaces" were being postponed/preview for .Net 9(?)

No, it's planned for C# 13.

It sounds to me like it's (at least being set up to be) in preview for C# 13 dotnet/roslyn#73923

That is out of date.

@KennethHoff
Copy link

I thought "ref structs implementing interfaces" were being postponed/preview for .Net 9(?)

No, it's planned for C# 13.

It sounds to me like it's (at least being set up to be) in preview for C# 13 dotnet/roslyn#73923

That is out of date.

Welp 😅

@KennethHoff
Copy link

https://github.com/dotnet/csharplang/blob/main/meetings/2024/LDM-2024-07-22.md#ref-structs-implementing-interfaces

If only that had been released just a few hours prior 🫠😂

@huoyaoyuan
Copy link
Member

Even with interfaces on ref structs, we still can't make ref struct implementing IEnumerable<T>, because the enumerator can't be boxed as IEnumerator<T>, right?

There was long-standing discussion to make an abstraction of non-boxing enumerator. Now ref structs forces everything to be non-boxing. Is there any interest to revisit this area?

@timcassell
Copy link

@huoyaoyuan You mean like IEnumerable<T, TEnumerator> where TEnumerator : IEnumerator<T>?

I think that hasn't happened so far because C#'s generic inference doesn't work on it, forcing the full generics to be typed out, which is less than ideal.

@huoyaoyuan
Copy link
Member

@huoyaoyuan You mean like IEnumerable<T, TEnumerator> where TEnumerator : IEnumerator<T>?

I think that hasn't happened so far because C#'s generic inference doesn't work on it, forcing the full generics to be typed out, which is less than ideal.

Yes. But of course if we start to do so, we need also to define an approach to seamless adapt IEnumerable<T> to it, to not create yet another discrepancy.

@alrz
Copy link
Member

alrz commented Aug 2, 2024

static int Sum<TEnumerator>(TEnumerator enumerator)

Is there a proposal to implement linq operators with TEnumerator? (I think that would be related to #64031 somehow)

@stephentoub
Copy link
Member Author

static int Sum<TEnumerator>(TEnumerator enumerator)

Is there a proposal to implement linq operators with TEnumerator? (I think that would be related to #64031 somehow)

No, we don't plan to do that.

@terrajobst
Copy link
Member

terrajobst commented Aug 6, 2024

Video

  • Looks good as proposed
namespace System
{
    public partial static class MemoryExtensions
    {
        public partial ref struct SpanSplitEnumerator<T> : IEnumerator<Range>
        {
        }
    }
    public readonly ref struct ReadOnlySpan<T>
    {
        public partial ref struct Enumerator : IEnumerator<T>
        {
        }
    }
    public readonly ref struct Span<T>
    {
        public partial ref struct Enumerator : IEnumerator<T>
        {
        }
    }
}
namespace System.Text
{
    public partial ref struct SpanLineEnumerator : IEnumerator<ReadOnlySpan<char>>
    {
    }
    public partial ref struct SpanRuneEnumerator : IEnumerator<Rune>
    {
    }
}
namespace System.Numerics.Tensors
{
    public readonly ref struct ReadOnlyTensorSpan<T>
    {
        public partial ref struct Enumerator : IEnumerator<T>
        {
        }
    }
    public readonly ref struct TensorSpan<T>
    {
        public partial ref struct Enumerator : IEnumerator<T>
        {
        }
    }
}
namespace System.Text.RegularExpressions
{
    public partial ref struct ValueMatchEnumerator : IEnumerator<ValueMatch>
    {
    }
    public partial ref struct ValueSplitEnumerator : IEnumerator<Range>
    {
    }
}

@terrajobst terrajobst added 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 labels Aug 6, 2024
@dotnet-policy-service dotnet-policy-service bot added the in-pr There is an active PR which will close this issue when it is merged label Aug 13, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
api-approved API was approved in API review, it can be implemented area-System.Runtime in-pr There is an active PR which will close this issue when it is merged
Projects
None yet
Development

Successfully merging a pull request may close this issue.

6 participants