Skip to content

Inconsistent behavior of String literal type between pattern matching and isInstanceOf #6996

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

Closed
taisukeoe opened this issue Aug 6, 2019 · 8 comments · Fixed by #7027 or #12799
Closed
Assignees
Milestone

Comments

@taisukeoe
Copy link
Contributor

taisukeoe commented Aug 6, 2019

minimized code

Tried with dotr in Dotty compiler version 0.17.0-RC1 .

def isAType(arg: String): Unit = arg match {    
  case _ : "a" => println("This is `a` type")
  case _ => println("not `a`")
}

isAType("a")
//This is `a` type

isAType(new String("a"))
//This is `a` type

new String("a").isInstanceOf["a"]
//val res0: Boolean = false

expectation

new String("a") should not match with case _ : "a" as it doesn't with isInstanceOf.

@smarter
Copy link
Member

smarter commented Aug 12, 2019

The fact that pattern matching and isInstanceOf don't agree is definitely a bug, but (new Integer(1)).isInstanceOf[1] returns true in both scalac and dotty (because it erases to 1.equals(new Integer(1))), so I would argue that x.isInstanceOf["a"] should similarly erase to "a".equals(x). In fact, SIP-23 already says:

Pattern matching against typed patterns (see 8.1.2 Typed Patterns) where the TypePat is a literal type is translated as a match against the subsuming non-singleton type followed by an equality test with the value corresponding to the literal type.

/cc @milessabin @sjrd, do you agree ?

@sjrd
Copy link
Member

sjrd commented Aug 12, 2019

Yes, I believe that x.isInstanceOf["a"] should compile down to "a".equals(x).

@abgruszecki
Copy link
Contributor

Hm, now that you mention it, I think we did talk about it already w/ @sjrd. My sole argument as to why this should not be the case is that all AnyRef singleton types work based on object identity, and making String work based on equality instead is inconsistent.

Something to consider - how should the following compile?

val x: "a" = "a"
val y: Object = x

foo.isInstanceOf[x.type]
foo.isInstanceOf[y.type]

@abgruszecki abgruszecki self-assigned this Aug 13, 2019
@sjrd
Copy link
Member

sjrd commented Aug 13, 2019

That is a concern, indeed. But it is not limited to strings. You can also construct something with Ints:

val x: 1 = 1
val y: Integer = x

foo.isInstanceOf[x.type]
foo.isInstanceOf[y.type]

@abgruszecki
Copy link
Contributor

abgruszecki commented Aug 13, 2019

Ok, I just checked and apparently there's ample precedent for interesting behaviour when upcasting/boxing AnyVals:

scala> val a: Any = 1
val a: Any = 1

scala> 1.isInstanceOf[a.type]
val res0: Boolean = true

scala> a.isInstanceOf[1]
val res1: Boolean = true

scala> (new Integer(1)).isInstanceOf[a.type]
val res2: Boolean = true

scala> val i: Integer = new Integer(1)
val i: Integer = 1

scala> val j: Integer = new Integer(1)
val j: Integer = 1

scala> i.isInstanceOf[a.type]
val res3: Boolean = true

scala> j.isInstanceOf[a.type]
val res4: Boolean = true

scala> i.isInstanceOf[j.type]
val res5: Boolean = false

scala> i eq j
val res6: Boolean = false

scala> i.isInstanceOf[1]                                                                                                                                                                                                                                                                  
val res7: Boolean = true

scala> 1.isInstanceOf[i.type]
val res8: Boolean = false

scala> val o: Object = i
val o: Object = 1

scala> o.isInstanceOf[a.type]
val res9: Boolean = true

scala> a.isInstanceOf[o.type]
val res10: Boolean = false

Note in particular that upcasting an Int to Any preserves isInstanceOf behaviour, but boxing it in Integer/Object does not.

@smarter
Copy link
Member

smarter commented Aug 13, 2019

Note in particular that upcasting an Int to Any preserves isInstanceOf behaviour, but boxing it in Integer/Object does not.

upcasting ends up calling Integer.valueOf which caches small integers (< 1024), try a big one.

@smarter
Copy link
Member

smarter commented Sep 23, 2019

My sole argument as to why this should not be the case is that all AnyRef singleton types work based on object identity, and making String work based on equality instead is inconsistent.

Maybe we need to reconsider the concept of literal singleton types, if we stop insisting that they're singletons, then I think that using == is completely natural.

@odersky odersky reopened this Dec 12, 2019
@odersky
Copy link
Contributor

odersky commented Dec 12, 2019

Seems this needs more deliberations.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment