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

name-based pattern matching is broken #8989

Closed
scabug opened this issue Nov 17, 2014 · 7 comments
Closed

name-based pattern matching is broken #8989

scabug opened this issue Nov 17, 2014 · 7 comments
Assignees
Milestone

Comments

@scabug
Copy link

scabug commented Nov 17, 2014

I guess this feature never left LAMP discussion room as I cannot find any announcement or reference to it, only code that should implement it. So marking as Minor :-)
{code}class A extends Product1[Int] {
def _1 = 1
def isEmpty = false // used by scalac
def isDefined = !isEmpty // used by dotty
def canEqual(a: Any) = true
}

object d{
def unapply(a: Any) = new A
val p: Any = ???
val f = p match {case d(1) => true; case _ => false}
}{code}

NBmatching.scala:11: error: type mismatch;
 found   : Int(1)
 required: <notype>
  val f = p match {case d(1) => true; case _ => false}
                          ^
one error found

This is valid code for dotty and actually shares implementation with matching based on case classes(which are Products in dotty)

@scabug
Copy link
Author

scabug commented Nov 17, 2014

Imported From: https://issues.scala-lang.org/browse/SI-8989?orig=1
Reporter: @DarkDimius
Affected Versions: 2.11.4
See #7897

@scabug
Copy link
Author

scabug commented Nov 17, 2014

@adriaanm said:
It was "documented" by scala/scala#2848 (I never got around to updating the spec).
The get method is missing on the extractor's result.

@scabug
Copy link
Author

scabug commented Nov 17, 2014

@adriaanm said:
Looks like it doesn't work for Product1. Here's an example that works:

class A extends Product2[Int, String] {
  def _1: Int = 1
  def _2: String = "1"
  def isEmpty: Boolean = false
  def get: A = this
  def canEqual(that: Any) = true
}
 
object A {
  def unapply(a: A) = a
}

(new A) match {case A(1, "1") => true; case _ => false}

@scabug
Copy link
Author

scabug commented Nov 17, 2014

@adriaanm said (edited on Nov 17, 2014 2:41:14 PM UTC):
To emulate a unary case class, this works:

class A {
  def isEmpty: Boolean = false
  def get: Int = 1
}
 
object A {
  def unapply(a: A) = a
}

(new A) match {case A(1) => true; case _ => false}

@scabug
Copy link
Author

scabug commented Nov 20, 2014

@DarkDimius said:
I have a problem with name-based pattern matching actually still generating tuples.
given such definitions

type If
implicit val IfTag: ClassTag[If]
trait DeconstructorCommon[T >: Null <: Tree] {
    var field: T = null
    def get: this.type = this
    def isEmpty: Boolean = field eq null
    def isDefined = !isEmpty
    def unapply(s: T): this.type ={
      field = s
      this
    }
  }
trait IfDeconstructor extends DeconstructorCommon[If]{
    def _1: Tree
    def _2: Tree
    def _3: Tree
  }

the pattern
{code} val If(condp, thenp, elsep) = tree{code}
gets compiled into bytecode equivalent to

            Tuple3 tuple3;
            Object object = tree;
            Option option = this.scala$tools$nsc$backend$jvm$BCodeBodyBuilder$PlainBodyBuilder$$$outer().int().IfTag().unapply(object);
            if (option.isEmpty()) throw new MatchError(object);
            Object object2 = option.get();
            BackendInterface.IfDeconstructor ifDeconstructor = (BackendInterface.IfDeconstructor)this.scala$tools$nsc$backend$jvm$BCodeBodyBuilder$PlainBodyBuilder$$$outer().int().If().unapply(object2);
            if (ifDeconstructor.isEmpty()) {
                throw new MatchError(object);
            }
            Object condp = ((BackendInterface.IfDeconstructor)ifDeconstructor.get())._1();
            Object thenp = ((BackendInterface.IfDeconstructor)ifDeconstructor.get())._2();
            Object elsep = ((BackendInterface.IfDeconstructor)ifDeconstructor.get())._3();
            Tuple3 tuple32 = tuple3 = new Tuple3(condp, thenp, elsep);
            Object condp2 = tuple32._1();
            Object thenp2 = tuple32._2();
            Object elsep2 = tuple32._3();

ie tuples Options are not created indeed, but tuples are

@scabug
Copy link
Author

scabug commented Nov 25, 2014

@retronym said:
This problem arises from the desugaring of val patterns.

 cat sandbox/client.scala
trait Client extends Test {
  def test() {
    val If(condp, thenp, elsep) = (??? : Tree)
  }
}
2.11.x /code/scala2 qscalac sandbox/test.scala && qscalac -Xprint:parser sandbox/client.scala
[[syntax trees at end of                    parser]] // client.scala
package <empty> {
  abstract trait Client extends Test {
    def $init$() = {
      ()
    };
    def test(): scala.Unit = {
      <synthetic> <artifact> private[this] val x$1 = ($qmark$qmark$qmark: Tree): @scala.unchecked match {
        case If((condp @ _), (thenp @ _), (elsep @ _)) => scala.Tuple3(condp, thenp, elsep)
      };
      val condp = x$1._1;
      val thenp = x$1._2;
      val elsep = x$1._3;
      ()
    }
  }
}

A similar problem is evident in #8468 (case classes with > 22 fields.)

It would be nice if we could desugar them to vars:

  var x$1: ? = _
  var x$2: ? = _
  var x$3: ? = _
  scrut match {
    case Pattern(a, b, c) =>
      x$1 = a
      x$2 = b
      x$3 = c
    case => throw ...
  }
  val condp = x$1
  val elsep = x$2
  val thenp = x$3

But we would then need to give those symbols a magic type that took on the type of the RHS of the first assignment. Which sounds like too much magic. We'd also have a problem creating unwanted fields if this was part of a class.

So maybe we'd need to resurrect @adriaanm patch to optimize away the TupleN creation / destruction after the fact.

@scabug
Copy link
Author

scabug commented Sep 29, 2015

@retronym said:
The error message is improved in scala/scala#4720

We still incur the overhead of tuples for val patterns.

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

No branches or pull requests

2 participants