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

Jump to bottom

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
Closed

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

kevmoo opened this issue Oct 12, 2016 · 9 comments
Labels
area-core-library SDK core library issues (core, async, ...); use area-vm or area-web for platform specific libraries. library-core

Comments

@kevmoo
Copy link
Member

kevmoo commented Oct 12, 2016 •
edited

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
@kevmoo kevmoo added area-core-library SDK core library issues (core, async, ...); use area-vm or area-web for platform specific libraries. library-core analyzer-strong-mode labels Oct 12, 2016
@floitschG
Copy link
Contributor

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
Copy link
Member Author

kevmoo commented Oct 12, 2016

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

@lrhn
Copy link
Member

lrhn 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
Copy link
Member Author

kevmoo 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
Copy link
Contributor

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
Copy link
Member

vsmenon commented Oct 12, 2016

What do you guys think is the right fix?

@floitschG
Copy link
Contributor

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

@lrhn
Copy link
Member

lrhn 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
Copy link
Member

vsmenon commented Oct 12, 2016

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

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

No branches or pull requests
4 participants
@kevmoo @vsmenon @lrhn @floitschG