Skip to content
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

isInstanceOf and type matching give inconsistent results #1698

Closed
scabug opened this issue Feb 9, 2009 · 7 comments
Closed

isInstanceOf and type matching give inconsistent results #1698

scabug opened this issue Feb 9, 2009 · 7 comments
Assignees

Comments

@scabug
Copy link

scabug commented Feb 9, 2009

Please see #1683 for background. In summary:

class Outer {
  class Inner { }
  
  def go() = {
    val o1 = new Inner
    val other = new Outer
    val o2 = (new other.Inner).asInstanceOf[Any]  // simulating equals method
    
    println("o2.isInstanceOf[Inner] == " + o2.isInstanceOf[Inner])
    println("o2 match { case _: Inner ... == " + (o2 match { case _: Inner => true ; case _ => false }))

    // at present, for the second line to print true it must be expressed as Outer#Inner
    // println("o2 match { case _: Inner ... == " + (o2 match { case _: Outer#Inner => true ; case _ => false }))
  }
}

object Test 
{
  def main(args: Array[String]): Unit = new Outer() go
}

At present this prints true/false. It needs to print either false/false or true/true. As indicated in #1683 I believe true/true is the better choice.

@scabug
Copy link
Author

scabug commented Feb 9, 2009

Imported From: https://issues.scala-lang.org/browse/SI-1698?orig=1
Reporter: @paulp

@scabug
Copy link
Author

scabug commented Feb 10, 2009

@lrytz said:
decision: special-case the pattern matcher to behave like isInstanceOf

@scabug
Copy link
Author

scabug commented Feb 27, 2009

@odersky said:
The change that would be required for the spec is in the section on designators. It currently reads:

Specifically, the unqualified type name $$t$$ where $$t$$ is bound in some class, object, or package $$C$$ is taken as a shorthand for C.this.type # t.

I propose to change this as follows:

Specifically, the unqualified type name $$t$$ where $$t$$ is bound in some class, object, or package $$C$$ is taken as a shorthand for C.this.type # t, except if the type $$t$$ occurs as a part
of a typed pattern (\sref{sec:typed-patterns}). In the latter case,
$$t$$ is taken as a shorthand for just C # t.

@scabug
Copy link
Author

scabug commented Oct 3, 2009

@odersky said:
The spec has been updated soem time ago with the wording of my last comment. I am not 100% sure it's the right solution, though. Paul, what do you think?

@scabug
Copy link
Author

scabug commented Oct 10, 2009

@odersky said:
Actually, I am changing my opinion on this. It would be weird if

case Foo()

and

case _: Foo

did different things. But

case Foo()

is certainly

case this.Foo

and I see no need nor way to change this. So I think pattern matching stays like it is. As for asInstanceOf I tend to think it should also stay as it is (i.e. inconsistent with pattern matching), and that for two reasons:

  1. least surprise for java programmers, and one less trap in porting Java programs.
  2. backwards compatibility with current practice. It would be very awkward to
    change the behavior of isInstanceOf without warning from one version to the next.
    (I realize that we migth do a similar thing with equality, but that's no reason
    for generalizing it).

Compared to these I find the correspondence between isInstanceOf and pattern matching
less important.

I reassign to me so that the spec gets updated. But comments are of course welcome.

@scabug
Copy link
Author

scabug commented Oct 13, 2009

@paulp said:
Sorry I haven't commented on this. I have independently come to the same conclusion that pattern matching should stay as it is, with the proviso (I'm accumulating a lot of this proviso) that we really, really need a "-Wstrict" type compilation option which can warn you about this and a couple dozen other situations where there's such a high chance that the code is not doing what you think. (I'm not asking anyone else to do that, I'll do it -- but in most cases implementing a warning is comparable in difficulty to changing the behavior so it's somewhat time consuming.)

It's even sort of convenient to have isInstanceOf doing a less strict check, so I won't argue that either.

I have to say 90% of my scala debugging time (for bugs in my own code anyway) ends up coming down to code which looks like it does one thing and actually does something else. Shadowing being by far the biggest culprit here.

@scabug
Copy link
Author

scabug commented Mar 24, 2010

@odersky said:
(In r21255) Closes #1698 by reverting to the old spec. Review by extempore.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants