Skip to content

Add generic types to "new Collection.from" and "new Collection.unmodifiable" arguments #26383

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

Closed
nex3 opened this issue May 3, 2016 · 6 comments
Labels
area-core-library SDK core library issues (core, async, ...); use area-vm or area-web for platform specific libraries. core-2

Comments

@nex3
Copy link
Member

nex3 commented May 3, 2016

Right now, new List.from() and similar APIs take arguments with no generic types. This interferes with inference when using strong mode in two ways:

  • If downward inference doesn't apply in the situation where new List.from() is used, the user has to explicitly write the generic parameter since upwards inference can't work.
  • If downward inference does apply, then the argument could have the wrong type and the user wouldn't be able to tell until runtime.

Neither of these situations is particularly useful. As we go forth into a strong-mode-by-default world, these APIs should get updated.

@floitschG
Copy link
Contributor

One of the important use cases of new List.from is to allow changing the generic type. The default toList() that lives on Iterable gives you a List that has the same generic type as the lhs. You need to go through new List.from to change it to something else.

Clearly, this would also be a big breaking change since a lot of the uses of new List.from are already taking advantage of the type-conversion feature.

@jmesserly
Copy link

[...] the user has to explicitly write the generic parameter since upwards inference can't work.

Is this caused by #25220 ? That one wouldn't be all that hard to fix.

But yeah, regardless it's useful to have some APIs that change the type (e.g. like C# Enumerable.Cast).

@floitschG
Copy link
Contributor

floitschG commented May 3, 2016

In this case the best option would be to infer the type, if none was given, but allow to change it, when written explicitly.

var list = new List.from(<num>[1, 2]);  // => infer "List<num>".
var list2 = new List.from<int>(<num>[1, 2]);  // => Use "int" but don't complain, that List<num> isn't a List<int>.

If this was a very common use case, we would want this to be a language feature:

// Use `other`'s generic type as inference if not given.
new List.from<T>(Iterable<??T> other)

At the moment I don't think this will be warranted.

@nex3
Copy link
Member Author

nex3 commented May 3, 2016

One of the important use cases of new List.from is to allow changing the generic type.

I think this is a little misleading. The vast majority of calls to new List.from and similar APIs were written prior to strong mode, where the generic types were much more flexible than they are now, and consequently casting was much less important. The API was designed in the same context. Now that we're moving to a strong world, it's worth thinking about what the best API is for changing the generic type, rather than just saying "this API happens to do it now so let's stick with it".

Casting is an operation that inherently runs a high risk of runtime failures. We shouldn't build it in to methods that have other purposes; we should require the user to be explicit about it. We can do this today using the Delegating${Collection}.typed constructors in the collection package, or we could add methods like List/*<T>*/ cast/*<T>*/() to generic types. But having constructors silently cast is both dangerous and non-obvious—a very bad combination.

The default toList() that lives on Iterable gives you a List that has the same generic type as the lhs.

There's no equivalent to this for new Map.from() or new Queue.from(), nor for any of the new ${Collection}.unmodifiable() constructors.

Clearly, this would also be a big breaking change since a lot of the uses of new List.from are already taking advantage of the type-conversion feature.

This isn't clear to me. Do you have data about how many uses of new List.from will actually be broken? Will they be broken in spec mode, or only in strong mode where there's already a lot of API churn?

Is this caused by #25220 ? That one wouldn't be all that hard to fix.

This is an API issue, not an inference issue. The constructors are typed as (for example) new List.from(Iterable elements), with no generic type on elements. It doesn't make sense to do inference here because there's no way for the type system to know that the generic type of elements should be the same as the one for the resulting list. That's what I'd like to change.

@jmesserly
Copy link

This is an API issue, not an inference issue. The constructors are typed as (for example) new List.from(Iterable elements), with no generic type on elements. It doesn't make sense to do inference here because there's no way for the type system to know that the generic type of elements should be the same as the one for the resulting list. That's what I'd like to change.

Oops, yeah, that makes sense

@iposva-google iposva-google added the area-core-library SDK core library issues (core, async, ...); use area-vm or area-web for platform specific libraries. label May 6, 2016
@munificent munificent modified the milestone: 1.50 Oct 4, 2016
@floitschG floitschG added core-2 and removed core-m labels Sep 11, 2017
@munificent
Copy link
Member

This is done now. List.from() and friends were left alone to enable you to use those to deliberately change the type of the resulting collection. Separate List.of() etc. methods were added that expect a parameter with the right generic type.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area-core-library SDK core library issues (core, async, ...); use area-vm or area-web for platform specific libraries. core-2
Projects
None yet
Development

No branches or pull requests

6 participants