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]: Annotate TSource with allows ref struct for both sync/async LINQ #111830

Closed
nietras opened this issue Jan 25, 2025 · 6 comments
Closed
Labels
api-suggestion Early API idea and discussion, it is NOT ready for implementation area-System.Linq

Comments

@nietras
Copy link
Contributor

nietras commented Jan 25, 2025

Background and motivation

Support IEnumerable<TSource>/IAsyncEnumerable<TSource> where TSource is a ref struct in LINQ methods where appropriate/possible to allow using these in such cases. E.g. there are cases where TSource points to internal state of the enumerable source and one may want to prevent ToArray/ToList but would still like to support transformations like Select or reductions like Count or similar.

In general, all LINQ methods that do not involve storing/caching TSource on the heap should annotated with allows ref struct.

A concrete example of such a case is the .NET CSV library https://github.com/nietras/Sep consult README there for more background if necessary.

Currently, the lack of allows ref struct prevents LINQ usage in such cases which either means no support or having to reinvent the wheel for this, which also means a less than ideal user experience since standard and known method cannot be used.

cc: @stephentoub

API Proposal

Example API changes, I do not have time to list all possible methods and this can be expanded over time incl. by others.

namespace System.Linq;

public static partial class Enumerable
{
    public static int Count<TSource>(this IEnumerable<TSource> source)
#if NET9_0_OR_GREATER
        where TSource : allows ref struct
#endif
}

public static partial class AsyncEnumerable
{
    public static ValueTask<int> CountAsync<TSource>(
        this IAsyncEnumerable<TSource> source,
        CancellationToken cancellationToken = default)
#if NET9_0_OR_GREATER
        where TSource : allows ref struct
#endif
}

Above support likely involves annotating support types like:

namespace System.Runtime.CompilerServices

[StructLayout(LayoutKind.Auto)]
public readonly struct ConfiguredCancelableAsyncEnumerable<T>
#if NET9_0_OR_GREATER
    where T : allows ref struct
#endif

API Usage

No new API usage as such just expanded support via annotation.

Alternative Designs

None.

Risks

Given allows ref struct defines a contract, forward compatibility may be a risk if one later decides to rewrite a given LINQ method in a way that is incompatible with this support. Risk should be very low and considered on a method by method basis.

@nietras nietras added the api-suggestion Early API idea and discussion, it is NOT ready for implementation label Jan 25, 2025
@dotnet-policy-service dotnet-policy-service bot added the untriaged New issue has not been triaged by the area owner label Jan 25, 2025
@nietras
Copy link
Contributor Author

nietras commented Jan 25, 2025

As mentioned in #111685 changing this will no doubt require changes to several other parts incl ConfigureAwait/ WithCancellation/ConfiguredCancelableAsyncEnumerable as in:

https://github.com/dotnet/runtime/blob/main/src/libraries/System.Private.CoreLib/src/System/Threading/Tasks/TaskAsyncEnumerableExtensions.cs

@huoyaoyuan
Copy link
Member

Related to #108131

It's currently difficult to implement for many operators.

@eiriktsarpalis
Copy link
Member

I don't believe this would be feasible in practice. Many Linq methods wouldn't be able to support the constraint in principle, resulting in inconsistency between the various methods. Those that could support it currently apply performance optimizations using runtime type checks:

if (source is Iterator<TSource> iterator)
{
return iterator.Select(selector);
}
if (source is IList<TSource> ilist)
{
if (source is TSource[] array)
{
if (array.Length == 0)
{
return [];
}
return new ArraySelectIterator<TSource, TResult>(array, selector);
}
if (source is List<TSource> list)
{
return new ListSelectIterator<TSource, TResult>(list, selector);
}
return new IListSelectIterator<TSource, TResult>(ilist, selector);
}

I don't think it would be feasible to keep those while adding the allows ref struct constraint.

The constraint was added to IEnumerable<T> and IAsyncEnumerable<T> in the spirit of unblocking bespoke applications that need to use the abstractions, but I don't believe it signals any intention of extending it everywhere up the stack.

@nietras
Copy link
Contributor Author

nietras commented Jan 26, 2025

Naturally, ArraySelectIterator or similar optimization that rely on the enumerable being backed by heap memory storing TSource cannot be used given this, but shouldn't this be possible for a enumerable that is not that? If the required "branching" is not expressible in C# I would think this needs to be addressed somehow...

@stephentoub
Copy link
Member

Naturally, ArraySelectIterator or similar optimization that rely on the enumerable being backed by heap memory storing TSource cannot be used given this, but shouldn't this be possible for a enumerable that is not that?

You currently can't express such tests, eg
sharplab

@eiriktsarpalis
Copy link
Member

I think we can close this as not planned, for now. We can revisit assuming future versions of the language permit more versatility.

@eiriktsarpalis eiriktsarpalis closed this as not planned Won't fix, can't repro, duplicate, stale Jan 26, 2025
@dotnet-policy-service dotnet-policy-service bot removed the untriaged New issue has not been triaged by the area owner label Jan 26, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
api-suggestion Early API idea and discussion, it is NOT ready for implementation area-System.Linq
Projects
None yet
Development

No branches or pull requests

4 participants