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

Compiler generates wrong code from some code using extractor #1697

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

Compiler generates wrong code from some code using extractor #1697

scabug opened this issue Feb 8, 2009 · 7 comments
Assignees
Labels
Milestone

Comments

@scabug
Copy link

scabug commented Feb 8, 2009

I wrote following code and compiled it(scala 2.7.3.final).

case class Num(n : scala.Int) extends Term
case class Add(x : Term,y : Term) extends Term
object Value {
  def unapply(term : Term) : Boolean =
    term match {
      case Num(_) => true
      case Add(_,_) => false
    }
}

object App {
  def main(args: Array[String]) {
    val term = Add(Num(1), Add(Num(2),Num(3)))
    term match {
      case Add(Num(x),Num(y)) =>
        println("Add(Num, Num)")
      case Add(Value(),y) => // <- not match!
        println("Add(Value, ?)")
      case _ =>
        println("?")
    }
  }
}

I expects that "Add(Value, ?)" is displayed , but "?" is displayed.
Diassembling the compiled code, It seems that the compiler generates the code that skips pattern "Add(Value(),y)" when pattern "Num(y)" fails.

@scabug
Copy link
Author

scabug commented Feb 8, 2009

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

@scabug
Copy link
Author

scabug commented Sep 23, 2009

Justin Wick (justinwick) said:
This issue appears to be solved in scalac as of scala-2.8.0.r18746-b20090923021937. fsc refuses to compile the code, however. Is that a bug?

Also, I motion that someone create a script to make simple combinations of standalone code and expected output into a ParTest test.

@scabug
Copy link
Author

scabug commented Sep 23, 2009

@paulp said:
Replying to [comment:6 JustinWick]:

This issue appears to be solved in scalac as of scala-2.8.0.r18746-b20090923021937. fsc refuses to compile the code, however. Is that a bug?

I don't know what refusal it is giving you but fsc works for me. The exact case given above I fixed in r18184 but the general issue is not fixed so I left this open.

Also, I motion that someone create a script to make simple combinations of standalone code and expected output into a ParTest test.

I second the motion! All those in favor of assert(Someone == JustinWick)? The motion is carried unanimously!

@scabug
Copy link
Author

scabug commented Nov 11, 2010

@ingoem said:
Some of our students trapped into this issue. I am not sure it is the very same underlying issue, so here is another test case:

abstract class Term
case object Var extends Term
case object Abs extends Term

object Ex {
  def unapply(t: Term): Option[Term] = {
    println("unapply " + t)
    t match {
      case Abs => Some(t)
      case _ => None
    }
  }
}

object Test extends Application {
  (Abs, Var) match {
    case (Abs, Ex(v)) => println("Hm") // Comment this line out to get "Success"
    case (Ex(v), t) => println("Success")
    case (a, b) if Value.unapply(a) != None => println("Fail")
  }
}

@scabug
Copy link
Author

scabug commented Aug 8, 2011

@sschaef said:
I got also problems with extractors - but I used unapplySeq instead of unapply. Comment out the first case to get the expected output.

object MyListTest extends App {
  val xs = MyList(1 to 5: _*)
  xs match {
//    case MyList(head) => println("no") // works fine
    case head :: MyNil => println("no") // produces a MatchError
    case MyList(head, tail @ _*) => println("yay")
  }
}

object MyList {
  def apply[A](a: A*): MyList[A] =
    (a :\ empty[A]) { _ :: _ }
    
  def empty[A]: MyList[A] = MyNil
  
  def unapplySeq[A](a: MyList[A]): Option[Seq[A]] = {
    var xs: List[A] = Nil
    var ys = a
    while (!ys.isEmpty) {
      xs = ys.head :: xs
      ys = ys.tail
    }
    Some(xs.reverse)
  }
}

abstract class MyList[+A] {
  def head: A
  def tail: MyList[A]
  def isEmpty: Boolean
  
  def :: [B >: A](b: B): MyList[B] =
    new ::(b, this)
}

case class :: [A](head: A, tail: MyList[A]) extends MyList[A] {
  def isEmpty = false
}

case object MyNil extends MyList[Nothing] {
  def head = throw new UnsupportedOperationException("nil head")
  def tail = throw new UnsupportedOperationException("nil tail")
  def isEmpty = true
}

@scabug
Copy link
Author

scabug commented Nov 8, 2011

Commit Message Bot (anonymous) said:
(moors in r25966) smarter bridges to unapplies

wraps the call to a bridged synthetic unapply(Seq) in a defensive if-test:

if (x.isInstanceOf[expectedType]) real.unapply(x.asInstanceOf[expectedType]) else None

NOTE: the test is WRONG, but it has to be due to #1697/#2337 -- once those are fixed, this one should generate the expected output

@scabug
Copy link
Author

scabug commented May 4, 2012

@paulp said:
virtpatmat saves day, test in 8bc8b83f0b .

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

No branches or pull requests

2 participants