Open raboof opened 5 years ago
asScalaIterable(Collections.newArrayList()).toList()
, of course. (They're used to the verbosity.)
We could slap @varargs
on List.apply
and friends, but then we'd have to fix #10683. (I did a bit of work on that some moons ago; it's not trivial but may be Java-interop-specific to be considered for 2.13).
That would also give me List.apply("foo", "bar")
, which I've been missing since forever.
I'm not familiar with changes in for comprehensions in Dotty so I'll leave that to @adriaanm. But if they require this method then the change is probably inevitable.
You can always get the "static" parts in Java via MODULE$
: https://github.com/scala/scala/blob/2.13.x/test/files/pos/t1642/JavaCallingScalaHashMap.java
I think the change hasn't happened yet (or I didn't notice) but it's in pursuit of lampepfl/dotty#2573 as I understand it. scala/scala-dev#607 mentions that we could wait for 2.14, but in scala/scala#7929 @adriaanm expressed a desire to make no backwards-incompatible changes in 2.14.
So depending on Dotty's willingness to run off of 2.14.x, (or merge lampepfl/dotty#5813), maybe this could just be backed out. (Unless the for
-comprehension improvement is already merged?)
Scheduling for RC2 in case we want to revert the empty()
change.
Or add a static List.of
(like https://docs.oracle.com/javase/9/docs/api/java/util/List.html#of-E...-) for the express purpose of Java interop ?
Also to clarify, dotty doesn't care about empty
being there or not currently, but I'd love it if https://github.com/lampepfl/dotty/issues/2573 got solved in one way or another.
of
! I had forgotten about that in Java 9.
I like it, but does that mean we'd have two ways of creating the same List
? Might be confusing.
evil suggestion: make it private[scala]
so that it's public from Java's POV but not Scala's :).
I think it's quite likely an improved for-comprehension desugaring would avoid filter
/withFilter
and use empty
instead, so I'd prefer to keep the method in place. We could provide some convenience methods for Java to create Scala collections outside of the stdlib.
provide some convenience methods for Java to create Scala collections outside of the stdlib
Or in scala.jdk.javaapi.CollectionConverters
We're considering removing the empty
method because it's weird to have on the instance side of things (it should only be on the companion, but then the for-comprehension desugaring couldn't get at it without us having a way to get to the companion from the instance, which is also weird).
After trying to remove empty
I think we should keep it. While it's a bit of a weird method to have on collection instances, we already have it for Map
and Set
in 2.12. There is no good reason why these should be special cases. We should either have it on all collection kinds or none of them.
The version in 2.13.x before https://github.com/scala/scala/pull/7929/commits/a864ae04c48b9be94b00345668e8ecaf85fc24da was not ideal, either. empty
only returned an IterableCC
or MapCC
in the generic case but you want it to be of type C
. This was probably just an oversight as it is actually easier to implement with C
.
Fixing those types and deprecating the existing methods would be an option but they are used pervasively in the collection implementations. We need at least a protected version of this method so we'd have to add it under a new name and deprecate empty
so the name can potentially be freed up in 2.14, unless we discover that we need it for a new for
comprehension desugaring.
Maybe we are just missing the right abstraction. We have different polymorphic factories in collection instances but there is no monomorphic Factory[A, C]
. The new *FactoryDefaults
traits have to implement several methods that could all be provided byFactory
. But it could be difficult to add this extra layer of indirection without negatively affecting performance.
Ok, I’m glad you did the due diligence. Let’s leave this ticket open for some alternate fix.
But it could be difficult to add this extra layer of indirection without negatively affecting performance.
Do you mean having to go via .iterableFactory
, or something else?
Yes, technically we could spec the desugaring to use a sequence of two method calls (e.g., too.companion.empty when you need to signal failure). Since the desugaring should be more general than our collections, it seems a bit too ad-hoc, though. It should be grounded in some more abstract theory, imo.
On Tue, 14 May 2019 at 17:46, Dale Wijnand notifications@github.com wrote:
But it could be difficult to add this extra layer of indirection without negatively affecting performance.
Do you mean having to go via .iterableFactory, or something else?
— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/scala/bug/issues/11509?email_source=notifications&email_token=AAAWHS37F5D5OFXK4TN7YULPVLNEHA5CNFSM4HJJR552YY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGODVL5GUQ#issuecomment-492294994, or mute the thread https://github.com/notifications/unsubscribe-auth/AAAWHSZQSN5RF4S2P7JVQ6TPVLNEHANCNFSM4HJJR55Q .
Could you spec it as .empty
but add that to the collections with an extension method? Agreed that .companion.empty
is kludgy.
That seems cleaner, yes. The larger question, though, is: what should the spec be in the first place :-) I think we could consider working on this in 2.14 if there's demand. Will keep that in mind when I open the roadmap ticket for 2.14, as soon as 2.13.0 final is tagged :)
If we remove it now, and Dotty wants to play with .empty
as part of the spec, they can put an extension method in scala.collection
, right? Especially when they can add it as a top-level definition.
True, but there are other reasons to keep it: https://github.com/scala/bug/issues/11509#issuecomment-490538196
Related: https://github.com/scala/scala-dev/issues/573
Another hack we could do is adding an @staticForwarderName("empties")
annotation, so people can provide a different name in case of a conflict.
OK, the specific factory doesn't really work. newSpecificBuilder
and fromSpecific
have to use A @uncheckedVariance
. Both methods are protected
so it's not too bad, but you wouldn't want them accessible from the outside. The same goes for the factory: protected[this] def specificFactory: Factory[A, C]
should work but it doesn't help (even if Factory
had an empty
method), you still have to add a forwarder for empty
. You could expose it to users as a Factory[_ <: A, C]
but that's not useful, either, because you couldn't call anything other than empty
on it.
As a side-note, why do some collections have a package-privateemptyInstance
method ? https://github.com/scala/scala/blob/06392a55749f34ece097863c50a2af3fd6b3a88b/src/library/scala/collection/immutable/Set.scala#L131
As a side-note, why do some collections have a package-private
emptyInstance
method ? scala/scala:src/library/scala/collection/immutable/Set.scala@06392a5
#L131
Probably and oversight when the old code was ported from 2.12. These methods are not used for anything.
Moving to 2.14 as we can't do anything (binary incompatible) compatible in the 2.13 series.
https://github.com/scala/scala/commit/a864ae04c48b9be94b00345668e8ecaf85fc24da (added between 2.13.0-M5 and 2.13.0-RC1) introduces a
def empty()
onscala.collection.Iterable
.Many (immutable) implementations of
Iterable
, such asscala.collection.immutable.List
, have a companion object with aval empty
. On 2.13.0-M5 and earlier, Java users could use theList.empty()
static forwarder to get the emptyList
instance. From 2.13.0-RC1, this static forwarder no longer exists, because theList
class itself now has a non-staticempty()
method inherited fromIterable
.According to the commit message the change was intended to "facilitate the switch to dotty's for comprehensions" - I don't know anything about that, so it might well be that the change is inevitable or worth it for that reason - I just wanted to flag this consequence that might not have been intentional.
What would be the new way for a Java user to get easy access to an empty instance of such an immutable collection?