Add Matchable trait

[odersky]

This PR addresses the problem discussed in #10662. In a nutshell the question is how to avoid situations like these:

val imm = IArray(1,2,3) // supposedly immutable...
imm match
   case a: Array[Int] => a(0) = 0  // but that's shown to be lie

IArray is an opaque type that's defined in the standard library as follows:

opaque type IArray[+A] = Array[A]

One could be tempted to just decree that opaque types cannot be used as scrutinees in matches, but that would fall short:

def f[T](x: T) = x match
  case a: Array[Int] => a(0) = 0

f(imm)

So we need a more systematic way to guard type abstractions like the ones provided by opaque types, or abstract types, or type parameters.

That's what this PR provides. It adds a new synthetic trait Matchable that gets erased to Object and changes the hierarchy of toplevel classes and their members to the following:

abstract class Any:
  def asInstanceOf
  def ==
  def !=
  def ##
  def equals
  def hashCode
  def toString

trait Matchable extends Any:
  def isInstanceOf
  def getClass

class AnyVal extends Any, Matchable

class Object extends Any, Matchable

Moreover, in a pattern match against a constructor or type ascription the scrutinee should be of type Matchable. The compiler will insert a cast from Any to Matchable where this is needed. More precisely, given an expression E or type T, where T does not derive from Matchable, and an expected type which is either Matchable or [ ].getClass or [ ].isInstanceOf, we insert the cast E.asInstanceOf[T & Matchable]. With that change, everything(*) compiles as before. However, the cast can be tuned to come with a warning. The current PR enables the warning for -source 3.1 or source 3.1-migration and later. For -source 3.0 we don't want a warning yet since we want to maintain the property that we can cross-compile without warnings between 2.13 and 3.0.

With this change the problematic examples (both versions) are rejected:

-- Warning: i7314.scala:6:12 ---------------------------------------------------
6 |    case a: Array[Int] => 
  |            ^^^^^^^^^^
  |            pattern selector should be an instance of Matchable,
  |            but it has unmatchable type opaques.IArray[Int] instead

(*) currently there's a problem with zio that needs investigation

EDIT: With the speedy help of @adamgfraser we could figure out the zio problems and a fix is available. However, diagnosing that problem makes me retreat a bit in the interest of safety. I think it is safest not to move any methods from Any to Matchable and let Matchable instead be a pure marker trait that controls the ability to pattern match. That's the crucial parametricity guarantee we want to give; the methods in Any themselves are not a problem since they are read-only (except for asInstanceOf, of course, but that's our unsafe escape hatch).

With that latest restricted version, all tests and all community build projects pass without modifications. That's not surprising, since the generated code with Matchable is the same as the generated code without Matchable. There are some additional warnings, which will be turned on after 3.0. There's also the chance that some inferred type will be Matchable instead of Any, but it looks quite far-fetched to construct a problem from that in practice.

[odersky]

test performance please

[dottybot]

performance test scheduled: 2 job(s) in queue, 1 running.

[odersky]

Breakages

Two tests broke.

• 1044.scala now gives "value getClass is not a member of Nothing" for ???.getClass. This is arguably an improvement.
• tasty-interpreter broke since the implemented interpreter cannot handle trees of the form t.$asInstanceOf$[T] where $asInstanceOf$ is the compiler-generated type test. The interpreter should be updated. $asInstanceOf$ is not new, it's just that it did not appear in the test inputs before.

Also, ZIO broke with errors like this one:

Warning: A test is using time, but is not advancing the test clock, which may result in the test hanging. Use TestClock.adjust to manually advance the time.
Fiber failed.
An interrupt was produced by #72329.

Fiber:Id(1607346360849,72331) was supposed to continue to:
  a future continuation at zio.ZIO$.$init$$$anonfun$4(ZIO.scala:3264)
  a future continuation at zio.ZIO.6$$anonfun(ZIO.scala:420)
  a future continuation at zio.ZIO.6$$anonfun(ZIO.scala:420)
  a future continuation at zio.ZQueue$internal$BackPressure.7$$anonfun(ZQueue.scala:455)
  a future continuation at zio.ZIO.$less$times$$anonfun$1(ZIO.scala:1805)

This should be investigated further.

I had a quick look how much would break once we turn on a warning with each conversion from Any to Matchable. In the compiler codebase itself I got 126 warnings, but it looks that there are less than 10 root causes for the warnings, and that the root causes would be easily fixed.

[dottybot]

Performance test finished successfully:

Visit http://dotty-bench.epfl.ch/10670/ to see the changes.

Benchmarks is based on merging with master (e1166a0)

[odersky]

Test performance please

[odersky]

test performance please

[adamgfraser]

@odersky I looked into the ZIO issue and was able to isolate it to this line in Assertion.scala:

def isSubtype[A](assertion: Assertion[A])(implicit C: ClassTag[A]): Assertion[Any] =
  Assertion.assertionRec("isSubtype")(param(C.runtimeClass.getSimpleName))(assertion) { actual =>
    if (C.runtimeClass.isAssignableFrom(actual.getClass())) Some(actual.asInstanceOf[A])
    else None
  }

The call to actual.getClass() fails with java.lang.ClassCastException: class java.lang.Error cannot be cast to class scala.Function0.

This can be resolved by reimplementing the method as:

def isSubtype[A](assertion: Assertion[A])(implicit C: ClassTag[A]): Assertion[Any] =
  Assertion.assertionRec("isSubtype")(param(className(C)))(assertion)(C.unapply(_))

def className[A](C: ClassTag[A]): String =
  try {
    C.runtimeClass.getSimpleName
  } catch {
    // See https://github.com/scala/bug/issues/2034.
    case t: InternalError if t.getMessage == "Malformed class name" =>
      C.runtimeClass.getName
  }

With this change all ZIO tests in the community build pass on your commit before you reverted the getClass changes.

Note that this change was actually already implement in ZIO in February in zio/zio#2971 so we have a bit of an issue that the ZIO version in the community build is relatively old. Not sure what the best process is to update it at this point and ensure it stays current in the future.

In any case, there does appear to be some difference in semantics with this PR right now. Not sure if it is possible or advisable to address it but regardless I don't think ZIO should be a blocker for moving forward with this PR.

[griggt]

@adamgfraser I've opened #10696 to update ZIO in the community build.

[dottybot]

performance test scheduled: 1 job(s) in queue, 0 running.

[dottybot]

Performance test finished successfully:

Visit http://dotty-bench.epfl.ch/10670/ to see the changes.

Benchmarks is based on merging with master (c042be9)

[odersky]

@adamgfraser Thank you for digging into this. What must have happened is that getClass was taken from the wrong value (a Function0 instead of java.lang.Error. The reason is that getClass is now resolved later, after some adaptations, and those adaptations converted something to a Function0.

I believe it is overall safer not to move getClass and isInstanceOf to Matchable. The don't not cause particular damage to parametricity, since both are read-only, just like equals, hashCode, and toString. That's overall safer since it avoids any discrepancy between member lookup and casts.

[jdegoes]

A holy grail of the FP community is to statically prevent violation of parametric reasoning, e.g. rule out implementations like the following bogus definition of the identity function:

  def identity[A](a: A): A = 
    if a == 1 then 2.asInstanceOf[A]
    else if a.isInstanceOf[Int] then a.hashCode.asInstanceOf[A]
    else a match
      case "foo" => "bar".asInstanceOf[A]
      case x: Boolean => !x.asInstanceOf[A]
      case x => x

(This does not compile without casts, but with improved flow-typing, presumably could!)

Toward this aim, I'd be quite excited about any proposal that moves us even incrementally in this direction; or which allows Any to truly mean, "the absence of any structure".

Unfortunately, some Scala developers have become unreasonably scared of "Any", because of fear of how a value of that type would be used (indeed, Scala 2.x aggressively warns on the use of Any even in places where it should not, IMO!). I consider Any (by this or by another name—a parametric top of the hierarchy) to be one of the beautiful selling points of Scala, the dual of Nothing and the type of values for which we know nothing, and in a perfect world, this type would come with the guarantee that we can do absolutely nothing with values of this type.

[odersky]

The current scheme is now the bare minimum needed to move forward: An empty marker trait Matchable that can be used in the future to warn when matching against a scrutinee that's not supposed to be matchable. We need the trait in any future extension where we will turn the warnings on, so that we can include it in a bound or cast to it.

The plan is that over time we will move methods like getClass and isInstanceOf to Matchable and turn the warnings on. All this can come after 3.0. But Matchable needs to be in now, so that we can start the migration.

Since the current PR is risk-free and useful I propose we merge it, even at that late state in the game.

[LPTK]

@odersky is there any plans at all, perhaps in the far future, to also deprecate or remove methods like equals on Any?
If that is the case, we may want to pick a more general trait name than Matchable.

[sjrd]

I believe this PR needs the following additional tests:

• Make sure that a class/object cannot directly extend Matchable (without also extending AnyRef or AnyVal or a subclass)
• Either reject the a trait directly extend Matchable, or make sure that if we allow it, it behaves like a universal trait. But then we also need to make sure that a class extending that trait also extends AnyRef or AnyVal.
• Make sure that we refuse to synthesize a TypeTest[T, U] for when T is not a subtype of Matchable (or emit the same warning than if we try to do t.isInstanceOf[U], at any rate).
• Make sure that synthesizing a TypeTest[T, U] when T <: Matchable does not emit any warning.

[jdegoes]

The plan is that over time we will move methods like getClass and isInstanceOf to Matchable and turn the warnings on. All this can come after 3.0. But Matchable needs to be in now, so that we can start the migration.

Sounds great.

Since the current PR is risk-free and useful I propose we merge it, even at that late state in the game.

Double thumbs up from me. 👍 👍

[nicolasstucki]

Otherwise, the code looks good

[nicolasstucki]

There is a bug with https://github.com/lampepfl/dotty/pull/10670/files#diff-8893c00063ce469a750bdbdbca40265550e0901a39a8a45e2f83aa6d51230aa0R1161. a389a11 reproduces it.