scala.concurrent.impl.Promise.DefaultPromise (upon which the stdlib Future implementation is based) extends scala.concurrent.Future, and implements Promise#future by returning itself. (I assume the choice to have DefaultPromise be its own Future was to reduce memory usage, though I don’t know.)
Unfortunately, this means that the result of Future(1) can be cast to a Promise, which violates the immutability of Future (because anyone can change the result by completing it with something else). As a rather contrived example:
While this example may seem a bit contrived, and perhaps we shouldn’t worry about people explicitly casting things, it could create problems with more “legitimate” uses as well. Consider the following method defined using Dotty’s union types:
def foo[A](promiseOrFuture: Promise[A] | Future[A])(process: A => B): Future[B] =
promiseOrFuture match {
case p: Promise[A] =>
p.completeWith(getNextInput())
foo(p.future)(process)
case f: Future[A] => f map process
}
Suddenly, all of our Futures get treated incorrectly as Promises, leading to incorrect behavior and in this example an infinite loop. In the very best case, this is confusing. I opine however that it is dangerous and damages the immutability of Future.
This ties into a deeper philosophical question: how hard do you have to encapsulate something before it counts as encapsulation? On the JVM the user can always fish things out if they want to, it’s just a matter of how much effort they have to put in:
Return a narrow interface instead of the concrete/wide implementation? They can cast it. This includes pattern matching on open types, as you have done above
Box your private thing in a wrapper with a private field? They can use java.lang.reflect and .setAccessible.
Even if you try harder to hide things, there’s always sun.misc.Unsafe. Or someone may bytecode-instrument your code using AspectJ or similar
The line can be drawn at any point. I have done all of these things at various points in my code, both proprietary and open source.
For myself, I draw the line at the first bullet: if you cast things to access implementation details, all bets are off. Not to say you can’t do it, but as a library maintainer I won’t support you in your efforts, so you better know what you’re doing when you take such measures. If I give you a collection.Seq and you cast it into collection.mutable.Buffer and start mangling it, all bets are off!
I would mostly agree but I think the union type example is more a compelling problem.
If we take your point aren’t unions of traits always unsafe since in principle you could have an instance that extends all of the traits (that don’t have conflicting signatures)?
I’m concerned unions will have many unsafe corners since they invite so much use of reflective (isInstanceOf) matching.
The instant you use “asInstanceOf”, or reflection, generally speaking, you left Object Oriented land. Pattern matching is a case of reflection, as it happens, and it is a very non-object oriented way of doing things. So while what you said is all true, you forego the principle of encapsulation when you do these things.
Java’s CompletableFuture deals with this by ensuring that completion is atomic and only the first result ‘wins’. Further completions do not modify the result.
The problem isn’t completing it twice, the problem is that it was going to be completed in one way, but because it was accessed as a Promise, it was completed differently.
I’m not sure that this one is easily fixable—but would love to review any PRs which do not adversely affect performance. (This “problem” is manifested in implementations since the introduction of SIP-14)
def foo[A](promiseOrFuture: Promise[A] | Future[A]) ...=
promiseOrFuture match {
case p: Promise[A] => ...
...
An interesting interaction with union types, but I can think of a meriad of similar cases to this. Matching like this where values can have more than one type of the union always will allow bugs like this.
You could make a similar mistake with collection.Seq[T] | mutable.Buffer[T] (although they are subtypes) or java.concurrent.Flow.Publisher[T] | java.concurrent.Flow.Subscriber[T] because Processor[T, R] implements both.
I think we can generalize things further as: if you have a union type A | B where you cannot guarantee that A and B are disjoint, you’re doing something wrong.
It’s not an “interaction” with union types. It’s just the union types (and how they are used in the original example). Promise is not doing anything wrong, IMO.
Interesting. What are possible ways to guarantee that, and can the compiler enforce this?
Can I somehow express in my code that I’m assuming two types to be disjoint such that the compiler will warn or error if that assumption may not be true?
No, that’s not possible in general. The Scala type system does not have any feature to guarantee disjointness, in general. It is your job to do so if you wish to use union types. If you can’t guarantee disjointness yourself, as a human, don’t use union types.
An easy example where one can guarantee disjointness is when A and B are both unrelated classes.
Another possibility is if A is unconstrained, but B is a known internal (private) class. If instances of the internal class cannot leak to the outside world, then you have a guarantee that a user-provided A cannot be a B. That is actually a useful scenario, but the compiler cannot prove that without some very advanced escape analysis.
Separately, it is also OK to talk about A | B when you don’t require that they be disjoint, but only if any treatment that you apply to an A is also valid if you apply on a B, and conversely. So there are cases where a non-disjoint union can be correct—which means the compiler can’t outright reject non-disjoint union even if it were smart enough to prove them—, although some extra care is also required in that situation.
where A is defined in some library a4s, version 1, like this:
case class A(string: String)
Then, version 2 of a4s comes out, and now it is:
trait A {
** def string: Unit**
}
case class AImpl(string: String) extends A
object A {
** def apply(string: String): A = AImpl(string)**
}
I replace version 1 with version 2 in my build definition and I’m happy to see that it still compiles and the unit tests still pass.
I don’t know that much about compatibility, but the API has not changed, so I thought it is compatible, is it not? Regardless, how I would notice the change?
I don’t understand what you’re getting at. If the library makes breaking changes, your code might break. Not all breaking changes imply compile errors for broken code. Sometimes (often) they imply behavioral changes. The present issue has nothing to do with it; it doesn’t make things any different than before.