Skip to content

Bad bytecode generated for a partial function. #2396

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
valdisxp1 opened this issue May 8, 2017 · 7 comments
Closed

Bad bytecode generated for a partial function. #2396

valdisxp1 opened this issue May 8, 2017 · 7 comments

Comments

@valdisxp1
Copy link

When running the example below, it fails with java.lang.VerifyError Full stack trace

class Main {
  val buzz = Some(Bees.Bee("buzz")).collect {
    case Bees.Bee(value) => value
  }
}

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

object Bees {
  case class Bee(value: String)
}

Reproducible in fresh lampepfl/dotty.g8
OpenJDK 8 on Ubuntu

$ java -version
openjdk version "1.8.0_121"
OpenJDK Runtime Environment (build 1.8.0_121-8u121-b13-0ubuntu1.16.04.2-b13)
OpenJDK 64-Bit Server VM (build 25.121-b13, mixed mode)
@DarkDimius
Copy link
Contributor

Reproduced. Looking into it.

@nicolasstucki
Copy link
Contributor

A simpler version of this issue is:

class Bees {
  import Test._

  def f: PartialFunction[Bee, Unit] = {
    case Test.Bee(_) => ""
//     case Bee(_) => ""  // This one works
  }

  f(new Bee("buzz"))
}

object Test {
  case class Bee(value: String)
  def main(args: Array[String]): Unit = new Bees
}

Note that the issue seems to come from the prefix Test in case Test.Bee(_) .

@nicolasstucki
Copy link
Contributor

nicolasstucki commented May 16, 2017

A simpler case that exhibits a similar issue is:

class Bees {
  def f: PartialFunction[Bee, Unit] = {  case Bee(_) => ""  }

  f(new Bee("buzz"))

  case class Bee(value: String)
  // object Bee // With this it works
}

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

fails with

Caused by: java.lang.NullPointerException
	at Bees.Bees$f$$f$$anonfun$1$1(i2396.scala:6)
	at Bees$$anonfun$1.apply(i2396.scala:6)
	at Bees$$anonfun$1.apply(i2396.scala:6)
	at Bees.<init>(i2396.scala:9)
	at Test$.main(i2396.scala:17)
	at Test.main(i2396.scala)

where the bytecode has

public class Bees {
    public final Bees.Bee$ Bee$lzy1;

    public Bees() {
        this.f().apply(new Bees.Bee(this, "buzz"));
        this.Bee$lzy1 = new Bees.Bee$(this); // should be lazy
    }

    public final Bees.Bee$ Bee() { return this.Bee$lzy1; }  // should be initialized in here if needed
   ...
}

Issue #2454

@DarkDimius
Copy link
Contributor

@nicolasstucki, the issue you've triggered seems like a different issue.

@nicolasstucki
Copy link
Contributor

I will create another issue for it.

@DarkDimius
Copy link
Contributor

This is a bug in ElimStaticThis, that was creating references to packages.
Backend was generating bytecode that passed a package on stack...

See <empty> in the printount of Xprint below.

<static> def Bees$f$$isDefinedAt$1(x$1: Test.Test$Bee): Boolean =
      {
        case val selector2: Test.Test$Bee = x$1: Test.Test$Bee @unchecked
        {
          def case2(): Boolean =
            if
              {
                <empty>.Test
                Test.Bee.unapply(selector2).ne(null)
              }
             then
              {
                case val x2: Test.Test$Bee =
                  {
                    <empty>.Test
                    Test.Bee.unapply(selector2)
                  }
                {
                  case val p2: String = x2._1()
                  true
                }
              }
             else
              {
                def case3(): Boolean =
                  {
                    val _: Test.Test$Bee = selector2
                    false
                  }
                case3()
              }
          case2()
        }
      }
  }

@DarkDimius
Copy link
Contributor

Fixes available for both problems.

@DarkDimius DarkDimius self-assigned this May 17, 2017
nicolasstucki added a commit that referenced this issue May 17, 2017
Fix #2396: don't create refernces to packages in ElimStaticThis
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

4 participants