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

[Proposal]: Params Collections #7700

Open
1 of 4 tasks
MadsTorgersen opened this issue Nov 16, 2023 · 29 comments
Open
1 of 4 tasks

[Proposal]: Params Collections #7700

MadsTorgersen opened this issue Nov 16, 2023 · 29 comments
Assignees
Milestone

Comments

@MadsTorgersen
Copy link
Contributor

MadsTorgersen commented Nov 16, 2023

Params Collections

Summary

In C# 12 language added support for creating instances of collection types beyond just arrays.
See collection expressions.
This proposal extends params support to all such collection types.

This is a placeholder issue for this proposal: https://github.com/dotnet/csharplang/blob/main/proposals/params-collections.md

The proposal subsumes earlier proposals for params spans (#1757) and params IEnumerables (#179).

Design meetings

@TonyValenti
Copy link

Great proposal.

It would be great if we could include ranges at the call site via ..list notation.

@Daxode
Copy link

Daxode commented Dec 7, 2023

Something I would love for this proposal to consider too is unmanaged implements. (especially important with game engines like Unity for interop, and e.g. Burst requires it).

Specifically implementing a params into multiple types of a generic should be possible. Here's an example of one way it could work:

// Library code
ref struct SystemState {}
interface ISystem {
  public void Update<multi T>(ref SystemState state, params Span<T> values) where T : IComponent {}
}

// User Code
struct MyGameSystem : ISystem {
  // This part has to be able to be fully unmanaged
  public void Update(ref SystemState state, ComponentA a, ComponentB b, ComponentC c){}
}

// Managed code can call it like:
ComponentA a = default; 
ComponentB b = default; 
ComponentC c = default; 
ISystem mySystem = new MyGameSystem(); 
mySystem.Update(ref state, stackalloc IComponent[]{ a, b, c }); // managed and upon resolve checks length == 3

// Unmanaged code can of course only do
ComponentA a = default; // unmanaged struct
ComponentB b = default; // unmanaged struct
ComponentC c = default; // unmanaged struct
MyGameSystem mySystem = new MyGameSystem(); 
mySystem.Update(ref state, a, b, c );

// note: in Unity most cases will have reflection finding the bursted/native function pointers that actually calls the specified methods from an unmanaged context.

Today we are sadly forced to fake multiple type params. Examples for projects I work on:
Entities.ForEach we generate all possible combinations: https://github.com/needle-mirror/com.unity.entities/blob/master/Unity.Entities/LambdaJobConstruction/UniversalDelegates.gen.cs
SystemAPI.Query<T...> we only support 7 args: https://github.com/needle-mirror/com.unity.entities/blob/master/Unity.Entities/SystemAPI.cs#L32-L153

@HaloFour
Copy link
Contributor

HaloFour commented Dec 7, 2023

@Daxode

I think that would fall under variadic type parameters: dotnet/roslyn#5058

I don't understand how your example would work. The contract stipulates that you can pass 0..x parameters of any IComponent , but the implementation requires 3 of specific types?

@Artromskiy
Copy link

@Daxode
Not sure that this will work
mySystem.Update(ref state, stackalloc IComponent[]{ a, b, c }); // managed and upon resolve checks length == 3
stackalloc of IComponent span containing ComponentA, ComponentB, ComponentC is impossible. Components could have different size - so you wil get cs0208 trying to stackallock interface span. Also to be used as IComponent they will be boxed anyway.
If you are interrested in generics case - C# ECS frameworks mostly use code generation for functions like Update with many parameters (take a look at Arch ECS)

@MovGP0
Copy link

MovGP0 commented Jan 2, 2024

It should also be possible to follow a params variable with an CancellationToken:

public Task<int> FooAsync(params int[] values, CancellationToken ct)

@Sander-Brilman
Copy link

I like the proposal, makes the language feel more natural and complete in my opinion

@heischo
Copy link

heischo commented Jan 2, 2024

params should be usable in the beginning of the parameter list, too. @MovGP0 already mentioned the CancellationToken, but I think it should be more flexible:
public Task<int> FooAsync(params List<int> numbers, int base, string something, CancellationToken ct)
-->
var fooResult = FooAsync(10, 20, 40, 6, "something", ct)
This special example with an Integer following on an Integer-List requires a certain care in use, but should not be a problem for the compiler.

@HaloFour
Copy link
Contributor

HaloFour commented Jan 2, 2024

I could see supporting that via optional parameters, in that they must be named at the callsite. Otherwise, that sounds like that would make overload resolution infinitely more complicated.

var fooResult = FooAsync(10, 20, 40, base: 6, something: "something", ct: ct);

With collection expressions, it also feels unnecessary since you could wrap the params in brackets:

var fooResult = FooAsync([10, 20, 40], 6, "something", ct);

@wolffaayyy
Copy link

Somewhat unrelated, but it would be cool if I could require at least one value when using params. In a way that created an error at compile-time.

Perhaps with an attribute?

@CyrusNajmabadi
Copy link
Member

@wolffaayyy Yup. An attribute + analyzer would be an easy thing to add (and can be done today) :)

@stepanbenes
Copy link

I like the proposal, makes the language feel more natural and complete in my opinion

Yes, the is proposal is better than to simply extend the support for e.g. ReadOnlySpan, it is more consistent with other parts of the language. But if it were possible, I would rather make the keyword params obsolete. Now with collection expressions, the difference is just two characters and the benefit is that it is obvious that the collection is being created at the call-site.

@CalvinWilkinson
Copy link

This is a great proposal that will make the caller code more readable and flexible as well as the parameter in the method more flexible and readable.

I am personally all for it.

@egvijayanand
Copy link

egvijayanand commented Jan 4, 2024

This is possible even with C#12, all it needs is square brackets surrounding the params parameter during invocation.

//string DisplayActionSheet(string title, params string[] buttons) {} - classic definition
string DisplayActionSheet(string title, IEnumerable<string> buttons) {}
// Invocation
var result = DisplayActionSheet("Unsaved Changes?", ["Save", "Discard", "Cancel"]);

@CyrusNajmabadi
Copy link
Member

@egvijayanand That's the idea. We want to unify so that you can write either the collection form, or the params form, with the same semantics. Thsi greatly simplifies the language and unifies the concepts between these two areas. It also helps given an intuition in terms of how things should behave (both for users and us) since we want these to all be consistent.

@egvijayanand
Copy link

We want to unify so that you can write either the collection form, or the params form, with the same semantics.

I will take it like this. Once implemented, params parameter is going to take any type that implements IEnumerable<T> instead of just T[].

@CyrusNajmabadi
Copy link
Member

CyrusNajmabadi commented Jan 4, 2024

@egvijayanand Not quite. Rather: Once implemented, you can use params for any linear collection that collection expressions support. I call out linear collection as the dictionary expressions we are intending to do for C# 13 will not be part of this.

The distinction here is subtle, but very relevant though. As an example ReadOnlySpan<T> does not implement IEnumerable<T>, but will be supported here.

@egvijayanand
Copy link

I call out linear collection as the dictionary expressions we are intending to do for C# 13 will not be part of this.

Wow, collection expressions are getting further updates in C# 13.

@GabeSchaffer
Copy link

The unification would be really cool if you could take the [] off a collection expression to make an argument to a params parameter. For example:

void L(params List<string> args) {}

L([foo, ..bar]); // works
L(foo, ..bar); // but what about this?

// maybe non-linear support could be added some day?
void D(params IEnumerable<KeyValuePair<string, object>> kwargs) {}

D(["foo": 12, ..bar]); // will work
D("foo": 12, ..bar); // this would be cool
D(foo: 12, ..bar); // this would be super-cool

@KennethHoff
Copy link

KennethHoff commented Mar 6, 2024

Considering params has to be last (I believe) I could see this being feasible, but I don't think they'll do this. As I understood it, params collection is mostly a performance (in combination with overload resolution priority & existing params arrays overloads with params ROS) and consistency kind of thing. If params arrays didn't exist, then params collections wouldn't be worked on, as collection expressions are just better

@TonyValenti
Copy link

I'm really looking forward to this.

It would also be nice if I could use .. in the params section.

@mstefarov
Copy link

This is an ambitious proposal, but I would even be satistied with the simpler original scope of #1757

I'd love to see allocation-free params Span<T> overloads for common system methods — String.Concat, String.Join, Path.Combine, Task.WhenAll, etc. Even better if the compiler could just switch existing source code with written-out parameters to automatically prefer allocation-free overloads, just by upgrading LangVersion/TargetFramework.

@KennethHoff
Copy link

This proposal is already implemented in the compiler

@CyrusNajmabadi
Copy link
Member

@mstefarov thta proposal was subsumed into this one. Which is already being done for c#13 and which the runtime is already utilizing for their APIs.

@UrielZyx
Copy link

UrielZyx commented May 9, 2024

It should also be possible to follow a params variable with an CancellationToken:

public Task<int> FooAsync(params int[] values, CancellationToken ct)

@MovGP0 - What happens if I want to pass in a params List<CancellationToken>?

@MovGP0
Copy link

MovGP0 commented May 9, 2024

@MovGP0 - What happens if I want to pass in a params List<CancellationToken>?

I don't see the use case. Usually you would create a combined CancellationTokenSource and return a single CancellationToken. But that is a synchronous operation.

@UrielZyx
Copy link

UrielZyx commented May 9, 2024

@MovGP0 - What happens if I want to pass in a params List<CancellationToken>?

I don't see the use case. Usually you would create a combined CancellationTokenSource and return a single CancellationToken. But that is a synchronous operation.

The point isn't the use case.
I'm saying that if Params is generic and you want to add something (like a Cancellation token) after the params, then you have to build in support to add anything else after the params, because the compiler needs to be able to handle

public Task<int> FooAsync(params CancellationToken[] values, CancellationToken ct)```

@MovGP0
Copy link

MovGP0 commented May 9, 2024

public Task<int> FooAsync(params CancellationToken[] values, CancellationToken ct)```

Yes, that would be possible. It's just a collection that is passed to the method. So the syntax is fine.

It's just that it doesn't make sense to have a IEnumerable<CancellationToken> (almost) anywhere in your codebase.

@Delsin-Yu
Copy link

public Task<int> FooAsync(params CancellationToken[] values, CancellationToken ct)```

Yes, that would be possible. It's just a collection that is passed to the method. So the syntax is fine.

It's just that it doesn't make sense to have a IEnumerable<CancellationToken> (almost) anywhere in your codebase.

How about default values?

public Task<int> FooAsync(params CancellationToken[] values, CancellationToken? ct = null)

@hez2010
Copy link

hez2010 commented May 31, 2024

Implicit conversion from strings to ROS<char> should be disallowed for params ROS<char>.
See #8172

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests