Give constructor methods a result type of NoType.

Previously, constructor methods explicitly returned `this`, and
advertised it in the result type of their signature. This was an
implementation detail of the encoding in JS, for which we want
init methods to return `this` so that we can chain them with `new`.

This commit fixes the specification of init methods with NoType as
result type. They do not explicitly return `this` at the IR level
anymore. It is the responsibility of the desugaring to JavaScript
to introduce the `return this`.

This commit does not change the compiler yet, so that we can test
the backward binary compatibility hack.

Author: sjrd

ir/src/main/scala/org/scalajs/core/ir/Serializers.scala

@@ -16,6 +16,7 @@ import java.net.URI
 
 import scala.collection.mutable
 
+import Definitions.isConstructorName
 import Position._
 import Trees._
 import Types._
@@ -587,6 +588,8 @@ object Serializers {
 
   private final class Deserializer(stream: InputStream, sourceVersion: String) {
     private[this] val useHacks060 = sourceVersion == "0.6.0"
+    private[this] val useHacks065 = true
+      //Set("0.6.0", "0.6.3", "0.6.4", "0.6.5").contains(sourceVersion)
 
     private[this] val input = new DataInputStream(stream)
 
@@ -638,7 +641,13 @@ object Serializers {
         case TagStoreModule     => StoreModule(readClassType(), readTree())
         case TagSelect          => Select(readTree(), readIdent())(readType())
         case TagApply           => Apply(readTree(), readIdent(), readTrees())(readType())
-        case TagApplyStatically => ApplyStatically(readTree(), readClassType(), readIdent(), readTrees())(readType())
+        case TagApplyStatically =>
+          val result1 =
+            ApplyStatically(readTree(), readClassType(), readIdent(), readTrees())(readType())
+          if (useHacks065 && result1.tpe != NoType && isConstructorName(result1.method.name))
+            result1.copy()(NoType)
+          else
+            result1
         case TagApplyStatic     => ApplyStatic(readClassType(), readIdent(), readTrees())(readType())
         case TagUnaryOp         => UnaryOp(readByte(), readTree())
         case TagBinaryOp        => BinaryOp(readByte(), readTree(), readTree())
@@ -720,14 +729,22 @@ object Serializers {
           // read and discard the length
           val len = readInt()
           assert(len >= 0)
-          val result = MethodDef(readBoolean(), readPropertyName(),
+          val result1 = MethodDef(readBoolean(), readPropertyName(),
               readParamDefs(), readType(), readTree())(
               new OptimizerHints(readInt()), optHash)
-          if (foundArguments) {
+          val result2 = if (foundArguments) {
             foundArguments = false
-            new RewriteArgumentsTransformer().transformMethodDef(result)
+            new RewriteArgumentsTransformer().transformMethodDef(result1)
           } else {
-            result
+            result1
+          }
+          if (useHacks065 && result2.resultType != NoType &&
+              isConstructorName(result2.name.name)) {
+            result2.copy(resultType = NoType, body = result2.body)(
+                result2.optimizerHints, result2.hash)(
+                result2.pos)
+          } else {
+            result2
           }
         case TagPropertyDef =>
           PropertyDef(readPropertyName(), readTree(),

test-suite/js/src/test/resources/SourceMapTestTemplate.scala

@@ -53,13 +53,23 @@ object SourceMapTest extends JasmineTest {
             val trace2 = trace1.dropWhile(
               normFileName(_).endsWith("/java/lang/Throwables.scala"))
 
-            val exSte :: throwSte :: _ = trace2
-
-            expect(normFileName(exSte)).toContain("/SourceMapTest.scala")
-            // line where `case class TestException is written` above
-            expect(exSte.getLineNumber).toBe(15)
+            val topSte = trace2.head
+            expect(normFileName(topSte)).toContain("/SourceMapTest.scala")
+
+            val throwSte = if (topSte.getLineNumber == 15) {
+              // line where `case class TestException is written` above
+              val throwSte = trace2.tail.head
+              expect(normFileName(throwSte)).toContain("/SourceMapTest.scala")
+              throwSte
+            } else {
+              /* In fullOpt, it may happen that the constructor of
+               * TestException is inlined, in which case there is no trace of
+               * it anymore. The first stack element in SourceMapTest.scala is
+               * therefore the one we're interested in.
+               */
+              topSte
+            }
 
-            expect(normFileName(throwSte)).toContain("/SourceMapTest.scala")
             expect(throwSte.getLineNumber).toBe(lineNo)
         }
       }

tools/shared/src/main/scala/org/scalajs/core/tools/javascript/ScalaJSClassEmitter.scala

@@ -320,9 +320,21 @@ final class ScalaJSClassEmitter private (
   def genMethod(className: String, method: MethodDef): js.Tree = {
     implicit val pos = method.pos
 
-    val methodFun = desugarToFunction(this, className,
+    val methodFun0 = desugarToFunction(this, className,
         method.args, method.body, method.resultType == NoType)
 
+    val methodFun = if (Definitions.isConstructorName(method.name.name)) {
+      // init methods have to return `this` so that we can chain them to `new`
+      js.Function(methodFun0.args, {
+        implicit val pos = methodFun0.body.pos
+        js.Block(
+            methodFun0.body,
+            js.Return(js.This()))
+      })(methodFun0.pos)
+    } else {
+      methodFun0
+    }
+
     outputMode match {
       case OutputMode.ECMAScript6StrongMode =>
         js.MethodDef(static = method.static, genPropertyName(method.name),

tools/shared/src/main/scala/org/scalajs/core/tools/optimizer/GenIncOptimizer.scala

@@ -569,8 +569,8 @@ abstract class GenIncOptimizer(semantics: Semantics, outputMode: OutputMode,
     /** UPDATE PASS ONLY. */
     private def isElidableModuleConstructor(impl: MethodImpl): Boolean = {
       def isTriviallySideEffectFree(tree: Tree): Boolean = tree match {
-        case _:VarRef | _:Literal | _:This => true
-        case _                             => false
+        case _:VarRef | _:Literal | _:This | _:Skip => true
+        case _                                      => false
       }
       def isElidableStat(tree: Tree): Boolean = tree match {
         case Block(stats) =>

tools/shared/src/main/scala/org/scalajs/core/tools/optimizer/IRChecker.scala

@@ -186,11 +186,8 @@ class IRChecker(unit: LinkingUnit, logger: Logger) {
     }
 
     val isConstructor = isConstructorName(name)
-    val resultTypeForSig =
-      if (isConstructor) NoType
-      else resultType
 
-    val advertizedSig = (params.map(_.ptpe), resultTypeForSig)
+    val advertizedSig = (params.map(_.ptpe), resultType)
     val sigFromName = inferMethodType(name, static)
     if (advertizedSig != sigFromName) {
       reportError(
@@ -527,15 +524,15 @@ class IRChecker(unit: LinkingUnit, logger: Logger) {
     implicit val ctx = ErrorContext(tree)
 
     def checkApplyGeneric(methodName: String, methodFullName: String,
-        args: List[Tree], isStatic: Boolean): Unit = {
+        args: List[Tree], tpe: Type, isStatic: Boolean): Unit = {
       val (methodParams, resultType) = inferMethodType(methodName, isStatic)
       if (args.size != methodParams.size)
         reportError(s"Arity mismatch: ${methodParams.size} expected but "+
             s"${args.size} found")
       for ((actual, formal) <- args zip methodParams) {
         typecheckExpect(actual, env, formal)
       }
-      if (!isConstructorName(methodName) && tree.tpe != resultType)
+      if (tpe != resultType)
         reportError(s"Call to $methodFullName of type $resultType "+
             s"typed as ${tree.tpe}")
     }
@@ -607,7 +604,8 @@ class IRChecker(unit: LinkingUnit, logger: Logger) {
         val clazz = lookupClass(cls)
         if (!clazz.kind.isClass)
           reportError(s"new $cls which is not a class")
-        checkApplyGeneric(ctor.name, s"$cls.$ctor", args, isStatic = false)
+        checkApplyGeneric(ctor.name, s"$cls.$ctor", args, NoType,
+            isStatic = false)
 
       case LoadModule(cls) =>
         if (!cls.className.endsWith("$"))
@@ -640,16 +638,18 @@ class IRChecker(unit: LinkingUnit, logger: Logger) {
 
       case Apply(receiver, Ident(method, _), args) =>
         val receiverType = typecheckExpr(receiver, env)
-        checkApplyGeneric(method, s"$receiverType.$method", args,
+        checkApplyGeneric(method, s"$receiverType.$method", args, tree.tpe,
             isStatic = false)
 
       case ApplyStatically(receiver, cls, Ident(method, _), args) =>
         typecheckExpect(receiver, env, cls)
-        checkApplyGeneric(method, s"$cls.$method", args, isStatic = false)
+        checkApplyGeneric(method, s"$cls.$method", args, tree.tpe,
+            isStatic = false)
 
       case ApplyStatic(cls, Ident(method, _), args) =>
         val clazz = lookupClass(cls)
-        checkApplyGeneric(method, s"$cls.$method", args, isStatic = true)
+        checkApplyGeneric(method, s"$cls.$method", args, tree.tpe,
+            isStatic = true)
 
       case UnaryOp(op, lhs) =>
         import UnaryOp._

tools/shared/src/main/scala/org/scalajs/core/tools/optimizer/OptimizerCore.scala

@@ -1759,8 +1759,12 @@ private[optimizer] abstract class OptimizerCore(
     if (scope.implsBeingInlined.contains(targetID))
       cancelFun()
 
-    val MethodDef(_, _, formals, _, BlockOrAlone(stats, This())) =
-      getMethodBody(target)
+    val targetMethodDef = getMethodBody(target)
+    val formals = targetMethodDef.args
+    val stats = targetMethodDef.body match {
+      case Block(stats) => stats
+      case singleStat   => List(singleStat)
+    }
 
     val argsBindings = for {
       (ParamDef(Ident(name, originalName), tpe, mutable, _), arg) <- formals zip args
@@ -2170,7 +2174,7 @@ private[optimizer] abstract class OptimizerCore(
     case FloatLiteral(value) =>
       foldToStringForString_+(DoubleLiteral(value.toDouble))
 
-    case DoubleLiteral(value) => 
+    case DoubleLiteral(value) =>
       jsNumberToString(value).fold(tree)(StringLiteral(_))
 
     case LongLiteral(value)    => StringLiteral(value.toString)
@@ -2183,9 +2187,9 @@ private[optimizer] abstract class OptimizerCore(
 
   /* Following the ECMAScript 6 specification */
   private def jsNumberToString(value: Double): Option[String] = {
-    if (1.0.toString == "1") { 
+    if (1.0.toString == "1") {
       // We are in a JS environment, so the host .toString() is the correct one.
-      Some(value.toString) 
+      Some(value.toString)
     } else {
       value match {
         case _ if value.isNaN       => Some("NaN")
@@ -2195,7 +2199,7 @@ private[optimizer] abstract class OptimizerCore(
         case _ if value.isValidInt  => Some(value.toInt.toString)
         case _                      => None
       }
-    }  
+    }
   }
 
   private def foldBinaryOp(op: BinaryOp.Code, lhs: Tree, rhs: Tree)(
@@ -2218,13 +2222,13 @@ private[optimizer] abstract class OptimizerCore(
         val rhs1 = foldToStringForString_+(rhs)
         @inline def stringDefault = BinaryOp(String_+, lhs1, rhs1)
         (lhs1, rhs1) match {
-          case (StringLiteral(s1), StringLiteral(s2)) => 
+          case (StringLiteral(s1), StringLiteral(s2)) =>
             StringLiteral(s1 + s2)
           case (_, StringLiteral("")) =>
             foldBinaryOp(op, rhs1, lhs1)
           case (StringLiteral(""), _) if rhs1.tpe == StringType =>
             rhs1
-          case (_, BinaryOp(String_+, rl, rr)) => 
+          case (_, BinaryOp(String_+, rl, rr)) =>
             foldBinaryOp(String_+, BinaryOp(String_+, lhs1, rl), rr)
           case (BinaryOp(String_+, ll, StringLiteral(lr)), StringLiteral(r)) =>
             BinaryOp(String_+, ll, StringLiteral(lr + r))