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

Set.difference arguments disagree in SDK vs dev_compiler (Set<E> vs Set<Object>) #27573

Closed
kevmoo opened this issue Oct 12, 2016 · 9 comments

Comments

@kevmoo
Copy link
Member

commented Oct 12, 2016

Hit this trying to compile pkg/collection w/ DDC.

Help?

CC @vsmenon @leafpetersen

The dev_compiler version was updated on March 5, 2015 (to Set, which is good)
SDK version hasn't been changed since Mar 13, 2013

@floitschG

This comment has been minimized.

Copy link
Contributor

commented Oct 12, 2016

The core library's version looks fine to me. I don't know why DDC has Set<E>.

Somehow related: why does api.dartlang.org not show Set<Object>? https://api.dartlang.org/stable/1.20.0/dart-core/Set/difference.html

@kevmoo

This comment has been minimized.

Copy link
Member Author

commented Oct 12, 2016

No! The SDK has Set<E> – DDC has (the correct version, IMHO) Set<Object>

@lrhn

This comment has been minimized.

Copy link
Member

commented Oct 12, 2016

It probably should have been Set<Object> for symmetry with intersection and similarity with removeAll.
Changing it now will be a breaking change for strong mode - that makes all existing implementations have covariant parameter overrides which isn't strong-mode sound.

@kevmoo

This comment has been minimized.

Copy link
Member Author

commented Oct 12, 2016

Changing it now will be a breaking change for strong mode

Ironically, DDC (at least for its copy of the SDK) is already shipping the break 😄

@floitschG

This comment has been minimized.

Copy link
Contributor

commented Oct 12, 2016

Aah. I looked at the wrong place.
The SetMixin has Set<Object>. (This is probably the reason that DDC has it fixed).
This is just a mistake in the interface.

@vsmenon

This comment has been minimized.

Copy link
Member

commented Oct 12, 2016

What do you guys think is the right fix?

@floitschG

This comment has been minimized.

Copy link
Contributor

commented Oct 12, 2016

It should be safe to change Set in the core library.

@lrhn

This comment has been minimized.

Copy link
Member

commented Oct 12, 2016

Agree, if all the existing implementations have the other behavior anyway, and they do if the use SetBase or SetMixin, then we should just fix the interface.

@vsmenon

This comment has been minimized.

Copy link
Member

commented Oct 12, 2016

Sounds like package:collection will need to be fixed as well from @kevmoo 's original message.

@floitschG floitschG closed this in 6beb1fd Oct 20, 2016

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
4 participants
You can’t perform that action at this time.