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

Type ambiguity in iterable.Joined ctors #1600

Open
andreoss opened this issue May 4, 2021 · 8 comments
Open

Type ambiguity in iterable.Joined ctors #1600

andreoss opened this issue May 4, 2021 · 8 comments

Comments

@andreoss
Copy link
Contributor

andreoss commented May 4, 2021

Joined has two constructors with the same erasure.

     Joined(final Iterable<? extends T>... items) // erases ctor below when called with 1 argument
     Joined(final Iterable<Iterable<? extends T>> items) { 

Example:

final Iterable<? extends Iterable<Integer>> xxs = new ListOf<Iterable<Integer>>(new ListOf<>(1, 2, 3));
final Iterable<Integer> joined = new Joined<>(xxs); // does not compile
@0crat 0crat added the 0crat/new label May 4, 2021
@andreoss
Copy link
Contributor Author

@victornoel Proposal is to extract from this class

  1. Prepended 2) Appended and remove vararg ctor

@victornoel
Copy link
Collaborator

@andreoss I experimented a bit and changing public Joined(final Iterable<Iterable<? extends T>> items) to public Joined(final Iterable<? extends Iterable<? extends T>> items) should solve the problem. Would that be enough or do you see other problems?

@andreoss
Copy link
Contributor Author

@victornoel I don't think so. After erasurej Joined(Iterable<X>) and Joined(Iterable<Iterable<X>) will be the same thing.

@victornoel
Copy link
Collaborator

victornoel commented May 15, 2021

@andreoss maybe I misunderstood the subject you are talking about.

This is what I understand:

  • the problem with erasure is when two methods/constructors have the same name and the same arguments after erasure: so if there is no compile error when declaring the constructors, there is no problem with type erasure.
    • so first conclusion for me: we are not talking about type erasure
  • here the problem I understood is that the ambiguity between the two constructors (one taking a vararg of Iterable and one taking an Iterable of Iterable) makes their use (so not their declaration, since it already compile) problematic in some case
  • the example you gave is neither related to type erasure (since it's about constructor usage, not declaration) and is not about the ambiguity (since all the types are explicit in your example, so the ambiguity does not exist)
    • the problem of compilation in your example is only related to the fact the constructor taking an Iterable of Iterable does not accept the Iterable of Iterable you gave it
    • the solution I gave above (adding ? extends) solve this problem nicely

So now, even after applying the fix I propose (test it, your code will compile, at least in Eclipse), we only have one problem left that I can see: the ambiguity.
When I write new Joined<>(xxs) without expliciting the receiving type (Iterable<Integer> joined in your example), do I mean calling the first vararg constructor or the second one?. The eclipse compiler decides it's the second one even though I could have meant to use the first vararg constructor to get Iterable<Iterable<Integer>> joined.

If I call new Joined<>(new ListOf<>(1, 2, 3)) the compiler correctly infers that I meant to use the first vararg constructor btw.

But in practice, I don't see it happening, because you are always assigning a new Joined to something, so the compiler will know.

Am I missing something here? Do you have examples of problematic code (once you applied the fix I propose).

@victornoel
Copy link
Collaborator

@andreoss any feedback on the above?

@andreoss andreoss changed the title Type eresure in iterable.Joined Type ambiguity in iterable.Joined ctors Jun 19, 2021
@andreoss
Copy link
Contributor Author

@victornoel Yes, the problem is compile time only. I wanted to suggest to remove vararg ctor, or move it to another class. But if wildcard solves that, it's fine. WIll make a PR.

@victornoel
Copy link
Collaborator

@0crat assign @andreoss

@victornoel
Copy link
Collaborator

@andreoss by the way, what about this? Was there a PR in the end?

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

3 participants