Merge pull request scala#3383 from adriaanm/merge-2.10.x

adriaanm · adriaanm · commit 0beca4b2f039 · 2014-01-18T10:39:14.000-08:00
Merge 2.10.x
diff --git a/src/asm/scala/tools/asm/MethodWriter.java b/src/asm/scala/tools/asm/MethodWriter.java
@@ -1869,7 +1869,12 @@ final int getSize() {
         int size = 8;
         if (code.length > 0) {
             if (code.length > 65536) {
-                throw new RuntimeException("Method code too large!");
+                String nameString = "";
+                int i = 0;
+                // find item that corresponds to the index of our name
+                while (i < cw.items.length && (cw.items[i] == null || cw.items[i].index != name)) i++;
+                if (cw.items[i] != null) nameString = cw.items[i].strVal1 +"'s ";
+                throw new RuntimeException("Method "+ nameString +"code too large!");
             }
             cw.newUTF8("Code");
             size += 18 + code.length + 8 * handlerCount;
diff --git a/src/compiler/scala/tools/nsc/ast/parser/Parsers.scala b/src/compiler/scala/tools/nsc/ast/parser/Parsers.scala
@@ -1030,7 +1030,11 @@ self =>
 
     /** Assumed (provisionally) to be TermNames. */
     def ident(skipIt: Boolean): Name = (
-      if (isIdent) rawIdent().encode
+      if (isIdent) {
+        val name = in.name.encode
+        in.nextToken()
+        name
+      }
       else syntaxErrorOrIncompleteAnd(expectedMsg(IDENTIFIER), skipIt)(nme.ERROR)
     )
 
diff --git a/src/compiler/scala/tools/nsc/ast/parser/Scanners.scala b/src/compiler/scala/tools/nsc/ast/parser/Scanners.scala
@@ -607,10 +607,7 @@ trait Scanners extends ScannersCommon {
       if (ch == '`') {
         nextChar()
         finishNamed(BACKQUOTED_IDENT)
-        if (name.length == 0)
-          syntaxError("empty quoted identifier")
-        else if (name == nme.WILDCARD)
-          syntaxError("wildcard invalid as backquoted identifier")
+        if (name.length == 0) syntaxError("empty quoted identifier")
       }
       else syntaxError("unclosed quoted identifier")
     }
diff --git a/src/compiler/scala/tools/nsc/backend/icode/GenICode.scala b/src/compiler/scala/tools/nsc/backend/icode/GenICode.scala
@@ -793,10 +793,7 @@ abstract class GenICode extends SubComponent  {
                 case _ =>
               }
               ctx1.bb.emit(cm, tree.pos)
-
-              if (sym == ctx1.method.symbol) {
-                ctx1.method.recursive = true
-              }
+              ctx1.method.updateRecursive(sym)
               generatedType =
                 if (sym.isClassConstructor) UNIT
                 else toTypeKind(sym.info.resultType)
diff --git a/src/compiler/scala/tools/nsc/backend/icode/Members.scala b/src/compiler/scala/tools/nsc/backend/icode/Members.scala
@@ -195,6 +195,10 @@ trait Members {
       this
     }
 
+    final def updateRecursive(called: Symbol): Unit = {
+      recursive ||= (called == symbol)
+    }
+
     def addLocal(l: Local): Local = findOrElse(locals)(_ == l) { locals ::= l ; l }
 
     def addParam(p: Local): Unit =
diff --git a/src/compiler/scala/tools/nsc/backend/jvm/GenASM.scala b/src/compiler/scala/tools/nsc/backend/jvm/GenASM.scala
@@ -469,9 +469,9 @@ abstract class GenASM extends SubComponent with BytecodeWriters with GenJVMASM {
         }
         bytecodeWriter.writeClass(label, jclassName, arr, outF)
       } catch {
-        case e: java.lang.RuntimeException if(e.getMessage() == "Class file too large!") =>
-          // TODO check where ASM throws the equivalent of CodeSizeTooBigException
-          log("Skipped class "+jclassName+" because it exceeds JVM limits (it's too big or has methods that are too long).")
+        case e: java.lang.RuntimeException if e != null && (e.getMessage contains "too large!") =>
+          reporter.error(sym.pos,
+            s"Could not write class $jclassName because it exceeds JVM code size limits. ${e.getMessage}")
       }
     }
 
diff --git a/src/compiler/scala/tools/nsc/symtab/BrowsingLoaders.scala b/src/compiler/scala/tools/nsc/symtab/BrowsingLoaders.scala
@@ -64,8 +64,10 @@ abstract class BrowsingLoaders extends GlobalSymbolLoaders {
           addPackagePrefix(pre)
           packagePrefix += ("." + name)
         case Ident(name) =>
-          if (packagePrefix.length != 0) packagePrefix += "."
-          packagePrefix += name
+          if (name != nme.EMPTY_PACKAGE_NAME) { // mirrors logic in Namers, see createPackageSymbol
+            if (packagePrefix.length != 0) packagePrefix += "."
+            packagePrefix += name
+          }
         case _ =>
           throw new MalformedInput(pkg.pos.point, "illegal tree node in package prefix: "+pkg)
       }
diff --git a/src/compiler/scala/tools/nsc/symtab/classfile/ICodeReader.scala b/src/compiler/scala/tools/nsc/symtab/classfile/ICodeReader.scala
@@ -575,23 +575,28 @@ abstract class ICodeReader extends ClassfileParser {
         case JVM.invokevirtual =>
           val m = pool.getMemberSymbol(u2, static = false); size += 2
           code.emit(CALL_METHOD(m, Dynamic))
+          method.updateRecursive(m)
         case JVM.invokeinterface  =>
           val m = pool.getMemberSymbol(u2, static = false); size += 4
           in.skip(2)
           code.emit(CALL_METHOD(m, Dynamic))
+          // invokeinterface can't be recursive
         case JVM.invokespecial   =>
           val m = pool.getMemberSymbol(u2, static = false); size += 2
           val style = if (m.name == nme.CONSTRUCTOR || m.isPrivate) Static(onInstance = true)
                       else SuperCall(m.owner.name)
           code.emit(CALL_METHOD(m, style))
+          method.updateRecursive(m)
         case JVM.invokestatic    =>
           val m = pool.getMemberSymbol(u2, static = true); size += 2
           if (isBox(m))
             code.emit(BOX(toTypeKind(m.info.paramTypes.head)))
           else if (isUnbox(m))
             code.emit(UNBOX(toTypeKind(m.info.resultType)))
-          else
+          else {
             code.emit(CALL_METHOD(m, Static(onInstance = false)))
+            method.updateRecursive(m)
+          }
         case JVM.invokedynamic  =>
           // TODO, this is just a place holder. A real implementation must parse the class constant entry
           debuglog("Found JVM invokedynamic instructionm, inserting place holder ICode INVOKE_DYNAMIC.")
diff --git a/src/compiler/scala/tools/nsc/typechecker/Typers.scala b/src/compiler/scala/tools/nsc/typechecker/Typers.scala
@@ -3326,6 +3326,28 @@ trait Typers extends Adaptations with Tags with TypersTracking with PatternTyper
               // calls to the default getters. Example:
               //  foo[Int](a)()  ==>  foo[Int](a)(b = foo$qual.foo$default$2[Int](a))
               checkNotMacro()
+
+              // SI-8111 transformNamedApplication eagerly shuffles around the application to preserve
+              //         evaluation order. During this process, it calls `changeOwner` on symbols that
+              //         are transplanted underneath synthetic temporary vals.
+              //
+              //         Here, we keep track of the symbols owned by `context.owner` to enable us to
+              //         rollback, so that we don't end up with "orphaned" symbols.
+              //
+              //         TODO: Find a better way!
+              //
+              //         Note that duplicating trees would not be enough to fix this problem, we would also need to
+              //         clone local symbols in the duplicated tree to truly isolate things (in the spirit of BodyDuplicator),
+              //         or, better yet, disentangle the logic in `transformNamedApplication` so that we could
+              //         determine whether names/defaults is viable *before* transforming trees.
+              def ownerOf(sym: Symbol) = if (sym == null || sym == NoSymbol) NoSymbol else sym.owner
+              val symsOwnedByContextOwner = tree.collect {
+                case t @ (_: DefTree | _: Function) if ownerOf(t.symbol) == context.owner => t.symbol
+              }
+              def rollbackNamesDefaultsOwnerChanges() {
+                symsOwnedByContextOwner foreach (_.owner = context.owner)
+              }
+
               val fun1 = transformNamedApplication(Typer.this, mode, pt)(fun, x => x)
               if (fun1.isErroneous) duplErrTree
               else {
@@ -3354,6 +3376,7 @@ trait Typers extends Adaptations with Tags with TypersTracking with PatternTyper
                   if (!(context.diagnostic contains note)) context.diagnostic = note :: context.diagnostic
                   doTypedApply(tree, if (blockIsEmpty) fun else fun1, allArgs, mode, pt)
                 } else {
+                  rollbackNamesDefaultsOwnerChanges()
                   tryTupleApply orElse duplErrorTree(NotEnoughArgsError(tree, fun, missing))
                 }
               }
diff --git a/test/files/neg/t6426.check b/test/files/neg/t6426.check
diff --git a/test/files/neg/t6426.scala b/test/files/neg/t6426.scala
diff --git a/test/files/neg/t6563.check b/test/files/neg/t6563.check
@@ -0,0 +1,4 @@
+t6563.scala:4: error: not found: value e
+    e("f")
+    ^
+one error found
diff --git a/test/files/neg/t6563.scala b/test/files/neg/t6563.scala
@@ -0,0 +1,8 @@
+class A{
+  def b(c: => Unit){}
+  b{
+    e("f")
+    new G()(){}
+ }
+}
+class G(h:String="i")()
diff --git a/test/files/pos/t8062.flags b/test/files/pos/t8062.flags
@@ -0,0 +1 @@
+-optimize
diff --git a/test/files/pos/t8062/A_1.scala b/test/files/pos/t8062/A_1.scala
@@ -0,0 +1,5 @@
+package warmup
+
+object Warmup {
+  def filter[A](p: Any => Boolean): Any = filter[Any](p)
+}
diff --git a/test/files/pos/t8062/B_2.scala b/test/files/pos/t8062/B_2.scala
@@ -0,0 +1,3 @@
+object Test {
+  warmup.Warmup.filter[Any](x => false)
+}
diff --git a/test/files/pos/t8111.scala b/test/files/pos/t8111.scala
@@ -0,0 +1,24 @@
+trait T {
+
+  def crashy(ma: Any) {
+    // okay
+    val f1 = (u: Unit) => ma
+    foo(f1)()
+    foo((u: Unit) => ma)
+    foo(0, (u: Any) => ma) apply ()
+
+    // crash due to side effects on the onwer of the symbol in the 
+    // qualifier or arguments of the application during an abandoned
+    // names/defaults transform. The code type checkes because of
+    // autp-tupling which promotes and empty parmater list to `(): Unit`
+    foo((u: Any) => ma)()
+    
+    {{(u: Any) => ma}; this}.foo(0)()
+    
+    foo({def foo = ma; 0})()
+    
+    {def foo = ma; this}.foo(0)()
+  }
+
+  def foo(f: Any): Any => Any
+}
diff --git a/test/files/presentation/t8085.check b/test/files/presentation/t8085.check
@@ -0,0 +1,3 @@
+reload: NodeScalaSuite.scala
+open package module: package object nodescala
+Test OK
diff --git a/test/files/presentation/t8085.flags b/test/files/presentation/t8085.flags
@@ -0,0 +1 @@
+-sourcepath src
diff --git a/test/files/presentation/t8085/Test.scala b/test/files/presentation/t8085/Test.scala
@@ -0,0 +1,27 @@
+import scala.tools.nsc.interactive.tests.InteractiveTest
+import scala.reflect.internal.util.SourceFile
+import scala.tools.nsc.interactive.Response
+
+object Test extends InteractiveTest {
+
+  override def execute(): Unit = {
+    val src = loadSourceAndWaitUntilTypechecked("NodeScalaSuite.scala")
+    checkErrors(src)
+  }
+
+  private def loadSourceAndWaitUntilTypechecked(sourceName: String): SourceFile = {
+    val sourceFile = sourceFiles.find(_.file.name == sourceName).head
+    askReload(List(sourceFile)).get
+    askLoadedTyped(sourceFile).get
+    sourceFile
+  }
+
+  private def checkErrors(source: SourceFile): Unit = compiler.getUnitOf(source) match {
+    case Some(unit) =>
+      val problems = unit.problems.toList
+      if(problems.isEmpty) reporter.println("Test OK")
+      else problems.foreach(problem => reporter.println(problem.msg))
+
+    case None => reporter.println("No compilation unit found for " + source.file.name)
+  }
+}
diff --git a/test/files/presentation/t8085/src/nodescala/Foo.scala b/test/files/presentation/t8085/src/nodescala/Foo.scala
@@ -0,0 +1,3 @@
+package nodescala
+
+class Foo
diff --git a/test/files/presentation/t8085/src/nodescala/NodeScalaSuite.scala b/test/files/presentation/t8085/src/nodescala/NodeScalaSuite.scala
@@ -0,0 +1,10 @@
+package nodescala
+
+class NodeScalaSuite {
+  "".rich
+
+  // This is here only to prove that the presentation compiler is instantiated with the
+  // correct `sourcepath` value (if it wasn't, you would see a `not found: type Foo` in
+  // the test's output
+  println(new Foo())
+}
diff --git a/test/files/presentation/t8085/src/nodescala/package.scala b/test/files/presentation/t8085/src/nodescala/package.scala
@@ -0,0 +1,7 @@
+import scala.Some // <-- if you move the import *inside* the package object, then it all works fine!!
+
+package object nodescala {
+  implicit class StringOps(val f: String) {
+    def rich = 0
+  }
+}
diff --git a/test/files/presentation/t8085b.check b/test/files/presentation/t8085b.check
@@ -0,0 +1,3 @@
+reload: NodeScalaSuite.scala
+open package module: package object nodescala
+Test OK
diff --git a/test/files/presentation/t8085b.flags b/test/files/presentation/t8085b.flags
@@ -0,0 +1 @@
+-sourcepath src
diff --git a/test/files/presentation/t8085b/Test.scala b/test/files/presentation/t8085b/Test.scala
@@ -0,0 +1,27 @@
+import scala.tools.nsc.interactive.tests.InteractiveTest
+import scala.reflect.internal.util.SourceFile
+import scala.tools.nsc.interactive.Response
+
+object Test extends InteractiveTest {
+
+  override def execute(): Unit = {
+    val src = loadSourceAndWaitUntilTypechecked("NodeScalaSuite.scala")
+    checkErrors(src)
+  }
+
+  private def loadSourceAndWaitUntilTypechecked(sourceName: String): SourceFile = {
+    val sourceFile = sourceFiles.find(_.file.name == sourceName).head
+    askReload(List(sourceFile)).get
+    askLoadedTyped(sourceFile).get
+    sourceFile
+  }
+
+  private def checkErrors(source: SourceFile): Unit = compiler.getUnitOf(source) match {
+    case Some(unit) =>
+      val problems = unit.problems.toList
+      if(problems.isEmpty) reporter.println("Test OK")
+      else problems.foreach(problem => reporter.println(problem.msg))
+
+    case None => reporter.println("No compilation unit found for " + source.file.name)
+  }
+}
diff --git a/test/files/presentation/t8085b/src/p1/nodescala/Foo.scala b/test/files/presentation/t8085b/src/p1/nodescala/Foo.scala
@@ -0,0 +1,4 @@
+package p1
+package nodescala
+
+class Foo
diff --git a/test/files/presentation/t8085b/src/p1/nodescala/NodeScalaSuite.scala b/test/files/presentation/t8085b/src/p1/nodescala/NodeScalaSuite.scala
@@ -0,0 +1,11 @@
+package p1
+package nodescala
+
+class NodeScalaSuite {
+  "".rich
+
+  // This is here only to prove that the presentation compiler is instantiated with the
+  // correct `sourcepath` value (if it wasn't, you would see a `not found: type Foo` in
+  // the test's output
+  println(new Foo())
+}
diff --git a/test/files/presentation/t8085b/src/p1/nodescala/package.scala b/test/files/presentation/t8085b/src/p1/nodescala/package.scala
@@ -0,0 +1,9 @@
+import scala.Some // <-- if you move the import *inside* the package object, then it all works fine!!
+
+package p1 {
+  package object nodescala {
+    implicit class StringOps(val f: String) {
+      def rich = 0
+    }
+  }
+}
diff --git a/test/files/run/large_code.check b/test/files/run/large_code.check
@@ -0,0 +1,3 @@
+newSource1.scala:1: error: Could not write class BigEnoughToFail because it exceeds JVM code size limits. Method tooLong's code too large!
+class BigEnoughToFail {
+      ^
diff --git a/test/files/run/large_code.scala b/test/files/run/large_code.scala
@@ -0,0 +1,24 @@
+import scala.tools.partest._
+import java.io.{Console => _, _}
+
+// a cold run of partest takes about 15s for this test on my laptop
+object Test extends DirectTest {
+  override def extraSettings: String = "-usejavacp -d " + testOutput.path
+
+  // test that we hit the code size limit and error out gracefully
+  // 5958 is the magic number (2^16/11 -- each `a(1,2,3,4,5,6)` is 11 bytes of bytecode)
+  override def code
+    = s"""
+      |class BigEnoughToFail {
+      |  def a(a: Int, b: Int, c: Int, d: Int, e: Int, f: Int): Unit = {}
+      |  def tooLong: Unit = {
+      |    ${(1 to 5958) map (_ => "a(1,2,3,4,5,6)") mkString(";")}
+      |  }
+      |}""".stripMargin.trim
+
+  override def show(): Unit = {
+    Console.withErr(System.out) {
+      compile()
+    }
+  }
+}

Original file line number	Diff line number	Diff line change
`@@ -607,10 +607,7 @@ trait Scanners extends ScannersCommon {`
`607`	`607`	if (ch == '`') {
`608`	`608`	`nextChar()`
`609`	`609`	`finishNamed(BACKQUOTED_IDENT)`
`610`		`- if (name.length == 0)`
`611`		`- syntaxError("empty quoted identifier")`
`612`		`- else if (name == nme.WILDCARD)`
`613`		`- syntaxError("wildcard invalid as backquoted identifier")`
	`610`	`+ if (name.length == 0) syntaxError("empty quoted identifier")`
`614`	`611`	`}`
`615`	`612`	`else syntaxError("unclosed quoted identifier")`
`616`	`613`	`}`
Original file line number	Diff line number	Diff line change
`@@ -195,6 +195,10 @@ trait Members {`
`195`	`195`	`this`
`196`	`196`	`}`
`197`	`197`
	`198`	`+ final def updateRecursive(called: Symbol): Unit = {`
	`199`	`+ recursive \|\|= (called == symbol)`
	`200`	`+ }`
	`201`	`+`
`198`	`202`	`def addLocal(l: Local): Local = findOrElse(locals)(_ == l) { locals ::= l ; l }`
`199`	`203`
`200`	`204`	`def addParam(p: Local): Unit =`
Original file line number	Diff line number	Diff line change
`@@ -469,9 +469,9 @@ abstract class GenASM extends SubComponent with BytecodeWriters with GenJVMASM {`
`469`	`469`	`}`
`470`	`470`	`bytecodeWriter.writeClass(label, jclassName, arr, outF)`
`471`	`471`	`} catch {`
`472`		`- case e: java.lang.RuntimeException if(e.getMessage() == "Class file too large!") =>`
`473`		`- // TODO check where ASM throws the equivalent of CodeSizeTooBigException`
`474`		`- log("Skipped class "+jclassName+" because it exceeds JVM limits (it's too big or has methods that are too long).")`
	`472`	`+ case e: java.lang.RuntimeException if e != null && (e.getMessage contains "too large!") =>`
	`473`	`+ reporter.error(sym.pos,`
	`474`	`+ s"Could not write class $jclassName because it exceeds JVM code size limits. ${e.getMessage}")`
`475`	`475`	`}`
`476`	`476`	`}`
`477`	`477`