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

Yet another params proposal: params IReadOnlyList<T> #1366

Open
alfredmyers opened this Issue Mar 9, 2018 · 10 comments

Comments

Projects
None yet
6 participants
@alfredmyers
Contributor

alfredmyers commented Mar 9, 2018

There are several proposals for allowing params to be used with types other than T[]: #178, #179, etc.

This proposal is specifically for IReadOnlyList<T> since T[] implicitly implements both IList<T> and IReadOnlyList<T>

So given a method

public int Sum(params IReadOnlyList<int> values) {
   return values.Sum();
}

Both the following call sites would be valid:

var sum1 = Sum(1, 2, 3);
var sum2 = Sum(GetIntArrayOrListOfIntFromElseWhere());
@alrz

This comment has been minimized.

Contributor

alrz commented Mar 9, 2018

the full list of generic array interfaces is as follow,

IList<T>
ICollection<T>
IEnumerable<T>
IReadOnlyList<T>
IReadOnlyCollection<T>

I think we should support all these as part of #179

we could also add params Span<T> (#535). The only question is that how the overload resolution should behave, e.g.

void M(params int[] p)
void M(params IList<int> p)
void M(params ICollection<int> p)
void M(params IEnumerable<int> p)
void M(params IReadOnlyList<int> p)
void M(params IReadOnlyCollection<int> p)
void M(params Span<int> p)

M(1); // which method should be called?
@alfredmyers

This comment has been minimized.

Contributor

alfredmyers commented Mar 9, 2018

Other interfaces already are subject of other proposals.
I'm trying to focus this proposal on IReadOnlyList<T> (and maybe IList<T>?) as both .NET Core and .NET Framework already make T[] implement them.

@jnm2

This comment has been minimized.

Contributor

jnm2 commented Mar 9, 2018

@alfredmyers Unless you are proposing that we do something contrary to what #179 proposes, what's the point of starting a new discussion?

@portal-chan

This comment has been minimized.

portal-chan commented Mar 9, 2018

void M(params int[] p)
void M(params IList<int> p)
void M(params ICollection<int> p)
void M(params IEnumerable<int> p)
void M(params IReadOnlyList<int> p)
void M(params IReadOnlyCollection<int> p)
void M(params Span<int> p)

M(1); // which method should be called?

If all these params types we're added, I'd say that only one of all the int collection overloads should be allowed to be specified as params.

@jnm2

This comment has been minimized.

Contributor

jnm2 commented Mar 9, 2018

If all these params types we're added, I'd say that only one of all the int collection overloads should be allowed to be specified as params.

I like it, except that removing params from the int[] overload would be a breaking API change and prevent C#7 and lower compilers from being able to use any params overload. So as long as people care about compatibility or params working on older compilers, that would effectively keep people from using params on anything but arrays.

@bondsbw

This comment has been minimized.

bondsbw commented Mar 9, 2018

@portal-chan Then we have to figure out how this is handled:

interface IA
{
    void M(params ICollection<int> p);
}
interface IB
{
    void M(params IReadOnlyCollection<int> p);
}
class C : IA, IB
{
    // Must implement both
    public void M(params ICollection<int> p) {}
    public void M(params IReadOnlyCollection<int> p) {}
}
@svick

This comment has been minimized.

Contributor

svick commented Mar 9, 2018

@bondsbw I don't think that particular case would be an issue. While you can specify params on an interface method, that doesn't mean the implementation has to be params too. For example, the following code compiles fine today:

interface IA
{
    void M(params int[] p);
}
class C : IA
{
    public void M(int[] p) {}
}
@bondsbw

This comment has been minimized.

bondsbw commented Mar 9, 2018

@svick TIL

@jnm2

This comment has been minimized.

Contributor

jnm2 commented Mar 9, 2018

Default values don't have to match or even be present, either.

@alrz

This comment has been minimized.

Contributor

alrz commented Mar 9, 2018

I think we can permit such overloads as long as we have a preference.

For example, we could resolve M(42) to the second method to enable overloading a params parameter with a params Span without breaking existing code:

void M(params int[] p)
void M(params Span<int> p)

and choose the first one below for M(42) based on the most specific type i.e. int:

void M(params IEnumerable<int> p)
void M(params IReadOnlyList<long> p)

But give declaration-site errors for these:

void M(params int[] p)
void M(params IEnumerable<int> p)

EDIT: as folks at gitter mentioned, it would not be necessary to define a precedence, since simply moving params to another method causes recompiled code to resolve to the new method and still existing code doesn't break.

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