-
Notifications
You must be signed in to change notification settings - Fork 1.1k
Unsoundness in handling of variance in pattern matching #3856
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
Comments
Now Scala 2.x avoids this by not allowing the variance in a subtype to be looser than in the base type: sealed trait Optional[A]
case class No[+A]() extends Optional[A]
but I'm going to argue that this should be resolved differently than in Scala 2.x. Namely that
The argument for 2. is monotonicity of type-checking: If a program typechecks with some type information, then it should still typecheck with more type information. This is violated in both Scala 2.x and Dotty: trait Foo[+A]
class Bar[A] extends Foo[A]
def g[F[+_]](fi: F[Int]): F[Any] = fi
g(new Bar[Int]: Foo[Int]) // OK
g(new Bar[Int]) // does not typecheck, despite having strictly more
// information about the type of the argument My argument for 1. is that it would be consistent with how type aliases already work. Consider an abstract type trait OptionalModule {
type Optional[A]
} In the implementation, invariant object OptionalImpl extends OptionalModule {
type Optional[A] = Option[A] // OK
} So But in Scala 2.x, invariant interface to covariant implementation cannot be achieved directly by subclassing: sealed trait Optional[A]
case class Yes[+A](a: A) extends Optional[A] // error: covariant type A occurs in invariant position |
I don't see how we can allow what you're proposing, allowing the variance to be looser in the subtype always leads to unsoundness: trait Foo[A](var x: A)
class Bar[+A](_x: A) extends Foo[A](_x)
object Test {
def main(args: Array[String]): Unit = {
val bars: Bar[String] = new Bar("foo")
val bara: Bar[Any] = bars
bara.x = 1
println(bars.x.length) // ClassCastException
}
} |
Closing as a duplicate of #2973, feel free to comment there if there's something I'm missing :) |
If the base class is declared as invariant, but actually uses the type parameter in contravariant position, the subclasses would be allowed to loosen it only to contravariant, not covariant. The compiler (and binaries) would have to keep track of actual variance of a type parameter, in addition to just the declared one. In your example
the type parameter |
So you're proposing to replace the concept of variance (which is one of the concept of Scala that confuses newcomers the most) with two concepts of declared variance and actual variance, something like that would require a very well motivated and detailed proposal. |
You can conclude that I already mentioned another motivation before, namely consistency: If an abstract invariant
I think at this stage it is premature to argue based on what impact it would have on newcomers and we should stick to technical arguments. But anyway, I subjectively think the net impact on newcomers would be positive (more consistent language, monotonic type inference). The "actual" variance would show up to the user only if they mix interface and implementation (such as in your |
@TomasMikula I agree with @smarter here, though
remains an option. I see basically two arguments: a technical one about type inference ("monotonicity"), and one about language aesthetics and "consistency".
I'd say this is Liskov's substitution principle and is a very good heuristic, but there are multiple cases where type inference isn't able to recover the appropriate typing derivation, while adding upcasts fixes the issue. That's because type inference can give better result with more information about the desired typing derivation. Just looking at DOT, if On this point:
== On consistency, your arguments is interesting, but I'm not convinced base traits and type aliases should be related, and that's relevant to most of your points. In terms of ML modules (which is the relevant and principled design), type aliases/members are the interface, and base traits are part of the implementation. In Scala you have extra freedom to try other designs, but that alone doesn't mean we can support all of them well. If you use type aliases/members for the interface and traits as part of the implementation, you currently end up with some boilerplate. That's annoying, but in many common cases the implementation can be expressed with less boilerplate using |
Thanks for pointing out the relationship to Liskov's substitution principle. What I mean is more like a substitution principle for types and type constructors rather than values. (Allow me to handwave the argument somewhat.) Consider a subtype relation on type constructors. Then, given the definitions as above trait Foo[+A]
class Bar[A] extends Foo[A] we might ask whether def h[F[+_]]: Unit = ()
h[Foo] // OK
h[Bar] // ERROR Since the substitution principle is violated, we might conclude that, at least internally to Scala, |
Ah, that's indeed an important distinction, thanks for clarifying I was confused. I'm afraid this argument is not convincing, and the handwaving you're doing might be confusing you (even though in general I'm happy to concede handwaving if helpful). It's not true that if For an example with just upper bounds (so that its core translates more easily to the standard F<: calculus), let's take
And before you start pointing out that LSP says something much different — it basically reminds us that if x \in A and A \subset B then x \in B, and once you have a set-theoretic model (or something close enough) that's true whether you add a rule to the syntax or not. I'm indeed studying such models formally at the moment and this was quite helpful, since I could define the model and observe the rules it satisfied. |
My original example is a software engineering benefit, namely avoiding the need to upcast in certain situations. Recall the definition of method def g[F[+_]](fi: F[Int]): F[Any] = fi To make You guessed my response to the counter-example with bounds and type (constructor) in contravariant position. |
Corrections: the example with
I looked this up because I was sure I'd never seen the substitution lemma you mention, but I concede your intuition was right. (I might not have time to look this up next time, but I will have to restudy a few systems with subtyping). Still, this example, while counterintuitive, is not necessarily a violation of narrowing, because while trait Foo[+A]
class Bar[A] extends Foo[A] Maybe I should look at how higher-order subtyping systems handle this. === Regarding the other half of your proposal:
absent a good SIP, I'd still leave the more restrictive rule in place, because of @smarter's argument about usability. In any case, I'd treat them as separate, and handle the above part first. |
👍 |
Btw, 👍 to considering type aliases/members as interface and base traits as part of the implementation. However, if I want exhaustive pattern matching, a sealed trait is currently the only option. |
I imagined that the pattern match would be written against the implementation — see traits trait Intf {
type Rep[T]
}
trait Impl extends Intf {
type Rep[T] = Exp[T]
trait Exp[T] // FIXED
case class IntExp extends Exp[Int] // FIXED
} Unless if your interface describes your entire ADT (which you can), you still don't get exhaustivity warning. |
In my musings about hypothetical Scala there is no such restriction.
For starters, I would take if I could just tell the compiler to trust me that an appointed set of methods deconstructs an abstract type exhaustively. Anyway, that's another discussion. |
Interestingly enough, sealed trait Optional[A]
object Optional {
private[this] case class No[+A]() extends Optional[A]
} already works in Scala. (I only just noticed this in scala/bug#10822 (comment).) This issue is then about being able to omit that |
Actually, here's a simple counter argument to the part of my proposal that in trait Foo[+A]
class Bar[A] extends Foo[A]
It breaks down with trait Function[-A, +B]
class Identity[X] extends Function[X, X] The proposal would require |
Test.boom
producesTested with
dotty-0.5.0-RC1
.The text was updated successfully, but these errors were encountered: