Skip to content

Pure expressions are never lifted from function calls, may be copied by default getters logic without creating new symbols, leading to duplicate methods after lambdalift #2939

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
Glavo opened this issue Aug 1, 2017 · 3 comments

Comments

@Glavo
Copy link
Contributor

Glavo commented Aug 1, 2017

For this class, Dotty does not seem to work properly:

import scala.collection.mutable._

class Tag(val name: String, val buffer: Buffer[Tag] = ArrayBuffer()) {
	def space(n: Int = 0): String = {
	    s"${" " * n}<$name>\n" +
	        (if(buffer.isEmpty) "" else buffer.map(_.space(n + 4)).mkString("", "\n", "\n")) +
	    s"${" " * n}</$name>"
	}
	
	def apply[U](f: implicit Tag => U)(implicit tag: Tag = null): this.type = {
		f(this)
		if(tag != null) tag.buffer += this
		this
	}
	
	override def toString(): String = space(0)
}

object Tag {
	implicit def toTag(symbol: Symbol): Tag = new Tag(symbol.name)

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

run by dotr(0.2.0-RC1-bin-SNAPSHOT-git-54d7089):

$ dotc Tag.scala
$ dotr Tag
Exception in thread "main" java.lang.ClassFormatError: Duplicate method name&signature in class file Tag$
	at java.lang.ClassLoader.defineClass1(Native Method)
	at java.lang.ClassLoader.defineClass(ClassLoader.java:763)
	at java.security.SecureClassLoader.defineClass(SecureClassLoader.java:142)
	at java.net.URLClassLoader.defineClass(URLClassLoader.java:467)
	at java.net.URLClassLoader.access$100(URLClassLoader.java:73)
	at java.net.URLClassLoader$1.run(URLClassLoader.java:368)
	at java.net.URLClassLoader$1.run(URLClassLoader.java:362)
	at java.security.AccessController.doPrivileged(Native Method)
	at java.net.URLClassLoader.findClass(URLClassLoader.java:361)
	at java.lang.ClassLoader.loadClass(ClassLoader.java:424)
	at sun.misc.Launcher$AppClassLoader.loadClass(Launcher.java:335)
	at java.lang.ClassLoader.loadClass(ClassLoader.java:357)
	at Tag.main(HTML.scala)

Or compiled with SBT and then used in the repl (dottyVersion="0.3.0-bin-20170729-df4b3dd-NIGHTLY" )

$ sbt console
[info] Loading global plugins from /home/glavo/.sbt/0.13/plugins
[info] Loading project definition from /home/glavo/IdeaProjects/Test/test/project
[info] Set current project to dotty-simple (in build file:/home/glavo/IdeaProjects/Test/test/)
[info] Compiling 1 Scala source to /home/glavo/IdeaProjects/Test/test/target/scala-0.3/classes...
Starting dotty interpreter...
Welcome to Scala.next (pre-alpha, git-hash: df4b3dd)  (OpenJDK 64-Bit Server VM, Java 1.8.0_131).
Type in expressions to have them evaluated.
Type :help for more information.
scala> import Tag._ 
import Tag._
scala> 'html {} 
java.lang.ClassFormatError: Duplicate method name&signature in class file 
	at java.lang.ClassLoader.defineClass1(Native Method)
	at java.lang.ClassLoader.defineClass(ClassLoader.java:763)
	at java.lang.ClassLoader.defineClass(ClassLoader.java:642)
	at dotty.tools.dotc.repl.AbstractFileClassLoader.findClass(AbstractFileClassLoader.scala:29)
	at java.lang.ClassLoader.loadClass(ClassLoa...
@smarter
Copy link
Member

smarter commented Aug 1, 2017

Minimization:

object Test {
  def foo(x: Int => Int)(y: Int = 0) = {}

  def main(args: Array[String]): Unit = {
    foo(x => x)()
  }
}

After typer we get:

package <empty> {
  final lazy module val Test: Test$ = new Test$()
  final module class Test$() extends Object() { this: Test.type => 
    def foo(x: Function1[Int, Int])(y: Int): Unit = 
      {
        ()
      }
    def foo$default$2(x: Function1[Int, Int]): Int = 0
    def main(args: Array[String]): Unit = 
      {
        Test.foo(
          {
            def $anonfun(x: Int): Int = x
            closure($anonfun)
          }
        )(
          Test.foo$default$2(
            {
              def $anonfun(x: Int): Int = x
              closure($anonfun)
            }
          )
        )
      }
  }
}

Which is very fishy: The call to foo$default$2 should take as argument a reference to the first argument, not a copy of it, this is why we end up with two methods with the same name ($anonfun in typer, which becomes main$$anonfun$1 in the backend). This happens because
pure expressions are not lifted from function calls (https://github.com/lampepfl/dotty/blob/d916343daa86c98e739dc3b685f95410565072d9/compiler/src/dotty/tools/dotc/typer/EtaExpansion.scala#L26) and closures are pure expressions.

@smarter smarter changed the title Duplicate method name&signature in class file Pure expressions are never lifted from function calls, leading to duplicate methods in the bytecode when default getters are used Aug 1, 2017
@smarter
Copy link
Member

smarter commented Aug 1, 2017

this is why we end up with two methods with the same name

This wouldn't actually be a problem if the two methods had different symbols, but since we just copy the tree this isn't the case (https://github.com/lampepfl/dotty/blob/d916343daa86c98e739dc3b685f95410565072d9/compiler/src/dotty/tools/dotc/typer/Applications.scala#L298), and that's why lambdalift gets confused and creates two methods called main$$anonfun$1.

@smarter smarter changed the title Pure expressions are never lifted from function calls, leading to duplicate methods in the bytecode when default getters are used Pure expressions are never lifted from function calls, may be copied by default getters logic without owner changes, leading to duplicate methods after lambdalift Aug 1, 2017
@smarter smarter changed the title Pure expressions are never lifted from function calls, may be copied by default getters logic without owner changes, leading to duplicate methods after lambdalift Pure expressions are never lifted from function calls, may be copied by default getters logic without creating new symbols, leading to duplicate methods after lambdalift Aug 1, 2017
@smarter
Copy link
Member

smarter commented Aug 1, 2017

A possible solution would be to always create fresh symbols, see https://github.com/dotty-staging/dotty/commits/fresh-symbols for how this could be done, but this isn't great for performance. Always lifting pure expressions seems like a simpler solution. We would also need to lift by-name arguments which are currently never lifted.

odersky added a commit to dotty-staging/dotty that referenced this issue Jan 15, 2018
Previously a method argument could be duplicated because it was passed
to the method as well as to its default argument methods. This was fatal
if the argument contained local definitions or was a closure.

We now unconditionally lift such arguments if the method has default
parameters.
odersky added a commit to dotty-staging/dotty that referenced this issue Jan 15, 2018
Previously a method argument could be duplicated because it was passed
to the method as well as to its default argument methods. This was fatal
if the argument contained local definitions or was a closure.

We now unconditionally lift such arguments if the method has default
parameters.
odersky added a commit to dotty-staging/dotty that referenced this issue Jan 21, 2018
Previously a method argument could be duplicated because it was passed
to the method as well as to its default argument methods. This was fatal
if the argument contained local definitions or was a closure.

We now unconditionally lift such arguments if the method has default
parameters.
@odersky odersky closed this as completed in 6e52484 Feb 2, 2018
odersky added a commit that referenced this issue Feb 2, 2018
Fix #2939: Avoid duplicating symbols in default arguments
odersky added a commit to dotty-staging/dotty that referenced this issue Feb 2, 2018
Previously a method argument could be duplicated because it was passed
to the method as well as to its default argument methods. This was fatal
if the argument contained local definitions or was a closure.

We now unconditionally lift such arguments if the method has default
parameters.
smarter pushed a commit to dotty-staging/dotty that referenced this issue Feb 10, 2018
Previously a method argument could be duplicated because it was passed
to the method as well as to its default argument methods. This was fatal
if the argument contained local definitions or was a closure.

We now unconditionally lift such arguments if the method has default
parameters.
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