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

Champion "Permit IEnumerable<T> as the type of a params parameter" #179

Closed
5 tasks
gafter opened this issue Feb 24, 2017 · 53 comments
Closed
5 tasks

Champion "Permit IEnumerable<T> as the type of a params parameter" #179

gafter opened this issue Feb 24, 2017 · 53 comments

Comments

@gafter
Copy link
Member

gafter commented Feb 24, 2017

  • Proposal added
  • Discussed in LDM
  • Decision in LDM
  • Finalized (done, rejected, inactive)
  • Spec'ed

See also

Design meetings

https:/dotnet/csharplang/blob/main/meetings/2022/LDM-2022-08-31.md#params-ienumerable
https:/dotnet/csharplang/blob/main/meetings/2022/LDM-2022-09-26.md#params-improvements

@Richiban
Copy link

Richiban commented Mar 22, 2017

I would assume that params was originally introduced so that the programmer didn't have the duality of having to write two methods just so that a piece of functionality could work with either a T or a collection of T, such as below:

public void MyMethod(string item) 
{
    MyMethod(new string[] {item});
}

public void MyMethod(string[] items)
{
    // ... Do something with items
}

While this was great, unfortunately as soon as IEnumerable was introduced and it became the de facto standard base collection type we're back to the duality again:

public void MyMethod(params string[] item) => MyMethod(items);

public void MyMethod(IEnumerable<string> items)
{
    // ... Do something with items
}

Actually, since we're on the subject, I'd like to propose that not only is params IEnumerable<T> allowed, but so should params IReadOnlyList<T>, params IList<T> etc. These types are all allowed because they are all implemented by T[]. So, more formally:

params M<U> identifier is allowed if U[] : M<U> and M<U> : IEnumerable<U>.

This works because at the callsite of method

public void MyMethod(params T<U> items)

the call:

MyMethod(item1, item2, item3)

gets converted to:

MyMethod(new[] {item1, item2, item3})

So the params keyword should be allowed on any type that is assignable from an array.

This is desirable because, as an API designer, it can sometimes feel dangerous to accept IEnumerable<T> as a parameter when you expect an in-memory collection because you don't know that the given argument is actually a collection at runtime. It could be lazily-loaded like a database query, or it might even be infinite, in which case you cannot safely call Count() or even foreach.

@DavidArno DavidArno mentioned this issue Mar 23, 2017
@aluanhaddad
Copy link

@Richiban I'm not sure I agree. There are many extension methods on IEnumerable<T> that will fail if it is an infinite sequence but work fine if it is implemented by an arbitrary but ultimately terminal enumerator. In practice I do not think that this has been a problem.

Also, this introduces a lot of complexity in terms of the number of possible signatures and potentially unexpected type requirements and allocations. What if I want params IImmutableSet<T>? While it would be weird if a method implicitly constructed a one from a varargs call, it is also vaguely justifiable since the callee determines the semantics.

I think params IEnumerable<T> is really all that is needed and provides enough value without support for additional types.

@jnm2
Copy link
Contributor

jnm2 commented Apr 1, 2017

I have so many methods that take IReadOnlyCollection<> or IReadOnlyList<>. Just support for those two would be phenomenal for me.

@CyrusNajmabadi
Copy link
Member

CyrusNajmabadi commented Apr 2, 2017

It would require teh compiler having some baked in knowledge, but i would also love "params ImmutableArray array" support.

Perhaps you could have "params SomeType array" (where SomeType is a concrete type) and the compiler would look for "SomeType.Create(...)" as what it would call in that case.

@yaakov-h
Copy link
Member

yaakov-h commented Apr 2, 2017

I don't see any issue with params IList<> or anything else that Array already implements.

IReadOnlyList<> would require the compiler knowing about AsReadOnly() etc.

<AConcreteType>.Create(...) would work for concrete types, but not for additional interfaces - unless you told the compiler about a specific concrete type, e.g. void Foo([ParameterList(typeof(SomeConcreteType))] params ISomeInterface bar) - but that seems ripe for abuse.

@aluanhaddad
Copy link

aluanhaddad commented Apr 2, 2017

@CyrusNajmabadi that certainly would be useful but if infrastructure is going to be put in place for using such patterns to instantiate params collections, would it make sense to make this part of a larger feature supporting collection literals?

There have been many requests for a collection literals as well as for JSON literals (collection literals) so if such a pattern is codified it would be great it were generalized to enable these scenarios as well.

Personally I think collection literals would be great, but only if you could customize the target type to be, as you say, ImmutableArray.

I don't think JSON literals make any sense at all but people ask for them.

@Richiban
Copy link

Richiban commented Apr 3, 2017

I think that talk of the compiler automatically inserting calls to extension methods is taking it a bit far...

What is very easy to understand and (I'm assuming) easy to implement would be that anywhere you could previously write this:

public void MyMethod<U>(M<U> items)
{
	// Do stuff...
}

public void Callee()
{
	MyMethod(new [] { 1, 2, 3 });
}

You can now write this:

public void MyMethod<U>(params M<U> items)
{
	// Do stuff...
}

public void Callee()
{
	MyMethod(1, 2, 3);
}

and there's no further magic than that.

@alrz
Copy link
Member

alrz commented Dec 29, 2017

@Richiban

So the params keyword should be allowed on any type that is assignable from an array.

I think accepting any type that is assignable from T[] is not possible because we can't always determine T (so the interface must have a generic parameter).

@alrz
Copy link
Member

alrz commented Dec 29, 2017

if that's desirable, it's pretty easy to implement, I should see if I can handle stackalloc optimization for Span (#535). scratch that, Span doesn't implement any interface because it's a ref struct.

@alrz
Copy link
Member

alrz commented Jan 4, 2018

there is a special case that does not clearly translate to IEnumerable,

(EDIT: used two null args to trigger array creation)

void M(params string[] args) {}
M(null, null); // OK args = new string[] { null, null }

void M(params IEnumerable args) {}
M(null, null); // ERROR? args = new[] { null, null } won't help

@gafter
Copy link
Member Author

gafter commented Jan 4, 2018

void M(params string[] args) {}
M(null); // OK args = new string[] { null }

That is not how it works. A method will be called in unexpanded (normal) form preferentially to being called in expanded form. M(null) will pass a null array.

@rossdempster
Copy link

rossdempster commented Mar 12, 2018

Given the variety of solutions now proposed, and especially considering the latest #1366 issue, would it make sense for the compiler to support any type that supports collection initialization?

This would mean we could put params in front of any type.
public void SomeMethodCall(params CustomType p) { ... }

The callsite is where the magic happens. Anywhere it would be possible to type:
SomeMethodCall(new CustomType {p1, p2, p3})

It would also be possible to type:
SomeMethodCall(p1, p2, p3)

Or is that crazy?

EDIT: Actually, it is crazy, and doesn't really help with all the interfaces. Ignore me!

@airbreather
Copy link

airbreather commented Mar 17, 2018

It would require teh compiler having some baked in knowledge, but i would also love "params ImmutableArray array" support.

I like that too, though dotnet/runtime#22928 would give most of the benefits with params ReadOnlySpan<T>.

Speaking of which, I'm in favor of params ReadOnlySpan<T>, and even having just that one instead of params Span<T> if it has to be just one or the other. It doesn't feel like the ability to write back to the original array is an important part of the params story, and the fact that you can do that today is probably just an unfortunate consequence of the tools that were available during the original .NET Framework design.

I'd be perfectly satisfied if all I had were:

  • params T[] so existing things don't break
  • params IEnumerable<T> for most things
  • params ReadOnlySpan<T> for new things where I worry about performance at all

Given those, even if I found myself in a situation where I could really, really benefit from knowing the count in advance if available without forcing ReadOnlySpan<T> (and this feels like a stretch), this looks OK enough:

void Do(params IEnumerable<int> args)
{
    int count;
    switch (args)
    {
        case null:
            throw new ArgumentNullException(nameof(args));

        case ICollection<int> coll1:
            count = coll1.Count;
            break;

        case IReadOnlyCollection<int> coll2:
            count = coll2.Count;
            break;

        default:
            List<int> argsList = args.ToList();
            args = argsList;
            count = argsList.Count;
            break;
    }

    // count is now available to us
    // same idea works for IList<T> / IReadOnlyList<T> if the indexer matters too
}

@TonyValenti
Copy link

I can't tell you how much I would love this feature.

@alrz
Copy link
Member

alrz commented Mar 20, 2018

I'm working on a prototype for this (dotnet/roslyn#24080). here's my open questions for LDM to confirm:

  • do we want to support all array generic interfaces? (supporting all these types is not nontrivial)
  • do we want to support params Span<T> and params ReadOnlySpan<T>? (Feature suggestion: zero-allocation "params" via Span<T> / ReadOnlySpan<T> #535) (we can reuse stackalloc initializer lowering in codegen where possible, otherwise we fallback to arrays)
  • do we want to forbid conflicting params overloads with identical element types? (otherwise all overloads would be always applicable in the expanded form and therefore ambiguous; note: the compiler wouldn't give ambiguous result for that case "for free", if we permit that, the overload resolution should be adjusted accordingly)

@CyrusNajmabadi
Copy link
Member

(supporting all these types is not nontrivial).

Do you mean that it's trivial? Or that it's nontrivial? The 'not' in there is throwing me off.

@CyrusNajmabadi
Copy link
Member

do we want to support params Span and params ReadOnlySpan

I think this would be fantastic. One of hte primary concerns around params in teh first place (and thus extending it to more situations) is that it incurs a array allocation. If we could have nice synchronous variadic APIs that didn't require allocations, i think that would be fantastic for the ecosystem.

@alrz
Copy link
Member

alrz commented Mar 20, 2018

Do you mean that it's trivial? Or that it's nontrivial?

I meant the former; just not wanted to stress on "trivial". I've implemented that part by relaxing some checks in the compiler (and because an implicit conversion exists, there was no need to adjust lowering), but since it's not thoroughly tested I wasn't confident that's all it would take to support it.

@TonyValenti
Copy link

Perhaps instead of using params IEnumerable<T> we should think about explicitly specifying the type that should be constructed. For example:

params Hashset<T> or params List<T>. In situations like that, the compiler would basically follow the rules for collection initializers (ie. calling any acceptable ````Add``` member/extension method).

@IngmarBitter
Copy link

@HaloFour OK. Forget all my comments here. I instead created the new issue #3133.

@StefanBertels
Copy link

@TonyValenti: nice idea. Ideally this would work with immutable data structures, too. Maybe search for some conversion operator?

@canton7
Copy link

canton7 commented Jan 31, 2020

@TonyValenti The downside with that is that if the user already has a variable of type IEnumerable<T>, they wouldn't be able to pass it. That is one of the main pain points which this proposal is trying to solve.

@crfrolik
Copy link

This should have been done years ago. Please make it happen.

@CleanCodeX
Copy link

CleanCodeX commented Sep 25, 2020

It is disappointing that you can't pass a ReadOnlySpan/IEnumerable/IReadOnlyList variable as the item collection itself for a params parameter as it demands to be an array and therefore in most cases an unnecessary copy needs to be created.

@333fred 333fred modified the milestones: X.X candidate, X.0 candidate Oct 14, 2020
@john-h-k
Copy link

john-h-k commented Nov 8, 2020

Personally, I like the idea of params T foo where there is an implicit conversion from U[] -> T, where U is the compiler resolved best-fit type of all provided params (i am not sure how tricky it is for the compiler to do this, tho), to be fine. That way, you support Span<T>, ROS<T>, IEnumerable<T>, IReadOnlyList<T> for free, as well as custom types and non generic types. You can then also get stack-based span params at a later point

If not that, then params T<U> foo where there is an implicit conversion from U[] -> T<U>

@yaakov-h
Copy link
Member

yaakov-h commented Nov 9, 2020

It gets you Span and ReadOnlySpan, but with allocations. I’d rather params Span and params ReadOnlySpan were allocation-free...

@AroglDarthu
Copy link

@MadsTorgersen Would be nice to get this in C#10, but I'm guessing the X in X.0 does not stand for 10 in this case?

@BreyerW
Copy link

BreyerW commented Sep 29, 2021

@AroglDarthu no the X in this case stands for ANY major version eg. It can equally likely happen in c# 20 or 11. (Btw c# 10 is practically done by now they ship GA in october if i remember correctly)

@AroglDarthu
Copy link

@BreyerW Thanks 😊
Just trying to get some traction on this feature. If I understood correctly, it got specified at the time C#6 was still in development.

It's labeled "Smallish Feature" (don't know if that means "small amount of work" or if it's considered "no huge benefit"). Perhaps it can be considered for the 11.0 milestone.

@TonyValenti
Copy link

I would like to be able to use any collection type as the type of Params. Ie. params HashSet.

@alrz
Copy link
Member

alrz commented Oct 11, 2021

There's a number of types that could be considered as part of this feature:

  • Array generic interfaces: this would be trivial as the compiler already emits arrays so there's nothing new for codegen.
  • Any type with collection initializer support: while this is possible, it will scatter API surfaces because of too many possibilities.
  • Immutable types: collection initializers don't support this scenario at this moment, but that may change with "list literals"

I wonder if this feature interacts with params Span<T> which is already under development for the next major release. @jcouv?

@Shadowblitz16
Copy link

Shadowblitz16 commented Jun 8, 2022

This would help make math libraries easier especially with INumber

public static class Math
{
    public static float   Mean(params float  [] a)
    {
        var result = 0f;
        var count = 0;
        foreach (var target in a)
        {
            result += target;
            count++;
        }

        return result / count;
    }
    public static Vector2 Mean(params Vector2[] a)
    {
        var result = Vector2.Zero;
        var count = 0;
        foreach (var target in a)
        {
            result += target;
            count++;
        }

        return result / count;
    }

to...

public static class Math
{
    public static T Mean<T>(params IEnumerable<T> a) where T : INumber
    {
        var result = default(T);
        var count = 0;
        foreach (var target in a)
        {
            result += target;
            count++;
        }

        return result / count;
    }

@TahirAhmadov
Copy link

TahirAhmadov commented Sep 18, 2022

I came up with a quick prototype for the holy grail "one params signature to cover all bases":

Definitions

class ReadOnlyMemoryEnumerable<T> : IEnumerable<T>
{
  public ReadOnlyMemoryEnumerable(ReadOnlyMemory<T> value) => this._value = value;

  public IEnumerator<T> GetEnumerator() => new Enumerator(this._value);
  IEnumerator IEnumerable.GetEnumerator() => this.GetEnumerator();

  class Enumerator : IEnumerator<T>
  {
    public Enumerator(ReadOnlyMemory<T> value) => this._value = value;

    public T Current => this._value.Span[this._index];
    object? IEnumerator.Current => this.Current;

    public void Dispose() { }
    public bool MoveNext() => ++this._index < this._value.Length;
    public void Reset() => this._index = -1;

    readonly ReadOnlyMemory<T> _value;
    int _index = -1;
  }

  readonly ReadOnlyMemory<T> _value;
}
public ref struct Params<T>
{
  public ref struct Enumerator
  {
    public Enumerator(Params<T> owner)
    {
      if (owner._enumerable != null)
      {
        this._enumerator = owner._enumerable.GetEnumerator();
        this._spanEnumerator = default;
      }
      else
      {
        this._spanEnumerator = owner._span.GetEnumerator();
        this._enumerator = null;
      }
    }

    public T Current
    {
      get
      {
        if (this._enumerator != null) return this._enumerator.Current;
        return this._spanEnumerator.Current;
      }
    }

    public bool MoveNext()
    {
      if (this._enumerator != null) return this._enumerator.MoveNext();
      return this._spanEnumerator.MoveNext();
    }

    readonly ReadOnlySpan<T>.Enumerator _spanEnumerator;
    readonly IEnumerator<T>? _enumerator;
  }

  public static implicit operator Params<T>(T[] array) => new(new ReadOnlyMemory<T>(array));
  public static implicit operator Params<T>(List<T> list) => new(list);

  public Params(IEnumerable<T> enumerable)
  {
    this._enumerable = enumerable;
    this._memory = default;
    this._span = default;
  }
  public Params(ReadOnlyMemory<T> memory)
  {
    this._memory = memory;
    this._span = memory.Span;
    this._enumerable = null;
  }

  public Enumerator GetEnumerator() => new(this);
  public IEnumerable<T> AsEnumerable() => this._enumerable ?? new ReadOnlyMemoryEnumerable<T>(this._memory);

  readonly IEnumerable<T>? _enumerable;
  readonly ReadOnlyMemory<T> _memory;
  readonly ReadOnlySpan<T> _span;
}
public ref struct ParamsList<T>
{
  public ref struct Enumerator
  {
    public Enumerator(ParamsList<T> owner)
    {
      if (owner._list != null)
      {
        this._enumerator = owner._list.GetEnumerator();
        this._spanEnumerator = default;
      }
      else
      {
        this._spanEnumerator = owner._span.GetEnumerator();
        this._enumerator = null;
      }
    }

    public T Current
    {
      get
      {
        if (this._enumerator != null) return this._enumerator.Current;
        return this._spanEnumerator.Current;
      }
    }

    public bool MoveNext()
    {
      if (this._enumerator != null) return this._enumerator.MoveNext();
      return this._spanEnumerator.MoveNext();
    }

    readonly ReadOnlySpan<T>.Enumerator _spanEnumerator;
    readonly IEnumerator<T>? _enumerator;
  }

  public static implicit operator ParamsList<T>(T[] array) => new(new ReadOnlyMemory<T>(array));
  public static implicit operator ParamsList<T>(List<T> list) => new(list);

  public ParamsList(IList<T> list)
  {
    this._list = list;
    this._memory = default;
    this._span = default;
  }
  public ParamsList(ReadOnlyMemory<T> memory)
  {
    this._memory = memory;
    this._span = memory.Span;
    this._list = null;
  }

  public int Count => this._list?.Count ?? this._span.Length;
  public T this[int index] => this._list != null ? this._list[index] : this._span[index];

  public Enumerator GetEnumerator() => new(this);
  public IEnumerable<T> AsEnumerable() => (IEnumerable<T>?)this._list ?? new ReadOnlyMemoryEnumerable<T>(this._memory);
  public IList<T> ToList()
  {
    if (this._list != null) return this._list; 
    var res = new List<T>();
    for (int i = 0; i < this._span.Length; ++i) res.Add(this._span[i]);
    return res;
  }

  readonly IList<T>? _list;
  readonly ReadOnlyMemory<T> _memory;
  readonly ReadOnlySpan<T> _span;
}

Usage:

static void Foo(/*params*/ Params<int> list)
{
  Console.WriteLine("Foo");
  foreach (int x in list)
  {
    Console.WriteLine(x);
  }
}
static void Bar(/*params*/ ParamsList<int> list)
{
  Console.WriteLine("Bar: " + list.Count);
  Console.WriteLine(list[0]);
  Console.WriteLine(list[1]);
  Console.WriteLine(list[2]);
}

int[] array = new[] { 1, 2, 3, };
var list = new List<int> { 4, 5, 6 };
IEnumerable<int> enumerable = new[] { 7, 8, 9 };
Foo(array);
Foo(list);
Foo(new Params<int>(enumerable));
//Foo(11, 12, 13);
Bar(array);
Bar(list);
//Bar(11, 12, 13);

Basically, the method's author chooses what the method needs - enumeration only (Params<T>), or accessing individual elements (ParamsList<T>), as well.
Then, then the method's caller can pass in any of the types: IEnumerable<T> (unless it's ParamsList<T> for obvious reasons), T[], Span<T>, etc.
Language support is added for params, which are done with the to be developed stack operations, but can in the meantime be added to the language with array allocation fallback.
Additional methods can be defined on Params(List)<T> types to convert to IEnumerable<T> and other collection types, to allow passing it around; these may incur an allocation, if the source was a stack-based Span<T>, but it's still a total of 1 allocation, which is no worse than the old params T[].
For interfaces like IEnumerable<T>, we can either a) relax the conversion rule for interfaces so that implicit operators can convert from interfaces to non-interface types; or b) create a special trans-piling rule which only allows IEnumerable<T>/etc. to Params<T>/ParamsList<T> conversion.

@TonyValenti
Copy link

In light of collection expressions coming up, I wanted to revitalize this thread and suggest that params be a "hint" that instructs the compiler to use the collection expression rules to construct the desired collection.

ie.

public static void Foo(params IEnumerable<int> x);
Foo(1, 2, 3); //same as Foo([1,2, 3])

This would play nice with target typing for collection expressions and solve the question of "What is the type of params IEnumerable<T>?"

@HaloFour
Copy link
Contributor

In light of collection expressions coming up, I wanted to revitalize this thread and suggest that params be a "hint" that instructs the compiler to use the collection expression rules to construct the desired collection.

My understanding is that with collection literals the need for this proposal has kinda gone away as it's very light syntactically to just wrap the arguments in a collection literal rather than adding params as a feature for less performant use cases.

@CyrusNajmabadi
Copy link
Member

Correct. We don't see substantive value here in allowing this anymore, as you can always just do [x, y, z] instead of x, y, z.

We do see substantive value in params ReadOnlySpan<T> as that means code does not need any modification to just start running faster.

@MadsTorgersen
Copy link
Contributor

This proposal is now subsumed by the championed proposal #7700 for Params Collections.

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