bug icon indicating copy to clipboard operation
bug copied to clipboard

Pattern match claims `x.type` is erased

Open som-snytt opened this issue 3 years ago • 6 comments

Reproduction steps

Scala version: 2.13.8

scala 2.13.8> class C[A] { val a: A = null.asInstanceOf[A] ; def f(x: A) = x match { case _: a.type => 42 } }
                                                                                              ^
              warning: abstract type pattern A (the underlying of C.this.a.type) is unchecked since it is eliminated by erasure
class C

scala 2.13.8>

scala 2.13.8> :javap -p -v C#f
  public int f(A);
    descriptor: (Ljava/lang/Object;)I
    flags: ACC_PUBLIC
    Code:
      stack=3, locals=4, args_size=2
         0: aload_1
         1: astore_3
         2: aload_0
         3: invokevirtual #29                 // Method a:()Ljava/lang/Object;
         6: aload_3
         7: invokestatic  #35                 // Method scala/runtime/BoxesRunTime.equals:(Ljava/lang/Object;Ljava/lang/Object;)Z
[snip]

scala 2.13.8> class C[A <: AnyRef] { val a: A = null.asInstanceOf[A] ; def f(x: A) = x match { case _: a.type => 42 } }
                                                                                                        ^
              warning: abstract type pattern A (the underlying of C.this.a.type) is unchecked since it is eliminated by erasure
class C

scala 2.13.8> :javap -p -v C#f
  public int f(A);
    descriptor: (Ljava/lang/Object;)I
    flags: ACC_PUBLIC
    Code:
      stack=3, locals=4, args_size=2
         0: aload_1
         1: astore_3
         2: aload_3
         3: aload_0
         4: invokevirtual #29                 // Method a:()Ljava/lang/Object;
         7: if_acmpne     16

Same code but different messages in 2.12:

scala 2.12.15> class C[A] { val a: A = null.asInstanceOf[A] ; def f(x: A) = x match { case _: a.type => 42 } }
<console>:11: error: type mismatch;
 found   : C.this.a.type (with underlying type A)
 required: AnyRef
Note that A is unbounded, which means AnyRef is not a known parent.
Such types can participate in value classes, but instances
cannot appear in singleton types or in reference comparisons.
               class C[A] { val a: A = null.asInstanceOf[A] ; def f(x: A) = x match { case _: a.type => 42 } }
                                                                                              ^

scala 2.12.15> class C[A <: AnyRef] { val a: A = null.asInstanceOf[A] ; def f(x: A) = x match { case _: a.type => 42 } }
<console>:11: warning: abstract type pattern A (the underlying of C.this.a.type) is unchecked since it is eliminated by erasure
               class C[A <: AnyRef] { val a: A = null.asInstanceOf[A] ; def f(x: A) = x match { case _: a.type => 42 } }
                                                                                                         ^
defined class C

Problem

Spec says "A singleton type p.type. This type pattern matches only the value denoted by the path p (the eq method is used to compare the matched value to p)."

If the warning were justified, then the 2.12 messaging about the unconstrained type is more helpful.

I don't think the equals check for the singleton pattern is correct, however. If there is a problem with the "pathiness" of my value, then it should disallow singleton pattern and require a backticked stable identifier pattern.

Apologies in advance for asking a question on a ticket. At least a messaging change is warranted.

Forgot to ask WWDD? Scala 3 offers no messaging in either case! You got boxes? No problem!

som-snytt avatar Apr 03 '22 20:04 som-snytt

Dale points out this closely related ticket: https://github.com/lampepfl/dotty/issues/9359, and see also https://github.com/lampepfl/dotty/issues/8430

SethTisue avatar Apr 19 '22 16:04 SethTisue

The relevant spec Som quotes is at https://www.scala-lang.org/files/archive/spec/2.13/08-pattern-matching.html#type-patterns :

  • ...
  • A singleton type 𝑝.type. This type pattern matches only the value denoted by the path 𝑝 (the eq method is used to compare the matched value to 𝑝).
  • ...

Types which are not of one of the forms described above are also accepted as type patterns. However, such type patterns will be translated to their erasure. The Scala compiler will issue an "unchecked" warning for these patterns to flag the possible loss of type-safety

The discussion on 9359 and 8430 goes pretty deep and I won't attempt to summarize here.

But, hmm, is there anything to say about this in the context of Scala 2 specifically?

The spec wording above specifically says that eq is used, not equals. So both compilers are out of spec here, but it's an open question whether the compilers should be changed or the spec should be changed. I'm agreeing with Som's:

I don't think the equals check for the singleton pattern is correct

But given that that's the behavior currently, the erasure warning, while the wording isn't ideal, in fact it does warn you of an actual issue, namely that equals is being emitted and that isn't actually sound, as demonstrated by the code in https://github.com/lampepfl/dotty/issues/9359

🤷 , I don't know what I think about all this. Here in Scala 2 the status quo seems not-terrible to me... but it sure as hell ain't ideal either... which leaves us where? I'd be reluctant to change any Scala 2 behaviors until this is considered resolved in Scala 3 — then we could decide what kind of a backport might or might not be doable.

SethTisue avatar Apr 19 '22 16:04 SethTisue

class C {
  override def equals(x: Any) = true
}
def cmp[A](x: A, y: Any) =
  x match {
    case _: y.type => true
  }
cmp(new C,new C)  // should be false

To refresh my memory, cmp asks for singleton comparison, but the match uses BoxesRunTime.equals which does eq and falls back to equals, which is nice for boxed primitives but not ref types.

It ought to use a hypothetical BoxesRunTime.eq which does eq and falls back to equals only for boxed value types.

som-snytt avatar May 19 '23 02:05 som-snytt

I don't remember the context of my last observation, but the match is just a stupid eq and basta!

I'm embarrassed I forgot about this bug when I recommended the idiom which must be worked around:

          val fallback: AnyRef = NotApplied
          pf.applyOrElse(grab(in), NotApplied) match {
            case _: fallback.type => completeStage()
            case elem: Out @unchecked => push(out, elem)
          }

where NotApplied is Any => Any. See the Pekko link.

som-snytt avatar Jan 23 '24 05:01 som-snytt

I tried :

          pf.applyOrElse(grab(in), NotApplied:Any/AnyRef/Any => Any) match {
            case _: NotApplied.type => completeStage()
            case elem: Out @unchecked => push(out, elem)
          }

Which works too.

He-Pin avatar Jan 23 '24 05:01 He-Pin

Wait, I'm rescinding my :eyes: reaction. NotApplied: Any => Any does not work for me.

Well, when I am awake again, I'll be interested in learning why it is pushing back in this example.

som-snytt avatar Jan 23 '24 05:01 som-snytt