Skip to content

Commit ac65737

Browse files
sjrdsjrd
committed
Merge pull request scala-js#2130 from sjrd/redesign-constructors
Redesign constructors
2 parents 98e88d1 + 0cc6ebe commit ac65737

File tree

15 files changed

+286
-107
lines changed

15 files changed

+286
-107
lines changed

‎ci/checksizes.sh

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -34,20 +34,20 @@ REVERSI_OPT_GZ_SIZE=$(stat '-c%s' "$REVERSI_OPT.gz")
3434

3535
case $FULLVER in
3636
2.10.2)
37-
REVERSI_PREOPT_EXPECTEDSIZE=533000
38-
REVERSI_OPT_EXPECTEDSIZE=120000
37+
REVERSI_PREOPT_EXPECTEDSIZE=537000
38+
REVERSI_OPT_EXPECTEDSIZE=121000
3939
REVERSI_PREOPT_GZ_EXPECTEDSIZE=72000
4040
REVERSI_OPT_GZ_EXPECTEDSIZE=32000
4141
;;
4242
2.11.7)
43-
REVERSI_PREOPT_EXPECTEDSIZE=535000
44-
REVERSI_OPT_EXPECTEDSIZE=122000
43+
REVERSI_PREOPT_EXPECTEDSIZE=540000
44+
REVERSI_OPT_EXPECTEDSIZE=124000
4545
REVERSI_PREOPT_GZ_EXPECTEDSIZE=73000
4646
REVERSI_OPT_GZ_EXPECTEDSIZE=33000
4747
;;
4848
2.12.0-M3)
49-
REVERSI_PREOPT_EXPECTEDSIZE=531000
50-
REVERSI_OPT_EXPECTEDSIZE=122000
49+
REVERSI_PREOPT_EXPECTEDSIZE=535000
50+
REVERSI_OPT_EXPECTEDSIZE=124000
5151
REVERSI_PREOPT_GZ_EXPECTEDSIZE=73000
5252
REVERSI_OPT_GZ_EXPECTEDSIZE=33000
5353
;;

‎compiler/src/main/scala/org/scalajs/core/compiler/GenJSCode.scala

Lines changed: 7 additions & 33 deletions
Original file line numberDiff line numberDiff line change
@@ -744,8 +744,6 @@ abstract class GenJSCode extends plugins.PluginComponent
744744
OptimizerHints.empty, None))
745745
} else if (isRawJSCtorDefaultParam(sym)) {
746746
None
747-
} else if (isTrivialConstructor(sym, params, rhs)) {
748-
None
749747
} else if (sym.isClassConstructor && isHijackedBoxedClass(sym.owner)) {
750748
None
751749
} else {
@@ -786,8 +784,8 @@ abstract class GenJSCode extends plugins.PluginComponent
786784
optimizerHints, None)
787785
} else if (sym.isClassConstructor) {
788786
js.MethodDef(static = false, methodName,
789-
jsParams, currentClassType,
790-
js.Block(genStat(rhs), genThis()))(optimizerHints, None)
787+
jsParams, jstpe.NoType,
788+
genStat(rhs))(optimizerHints, None)
791789
} else {
792790
val resultIRType = toIRType(sym.tpe.resultType)
793791
genMethodDef(static = sym.owner.isImplClass, methodName,
@@ -825,33 +823,6 @@ abstract class GenJSCode extends plugins.PluginComponent
825823
"scala.runtime.ScalaRunTime.arrayElementClass"
826824
)
827825

828-
private def isTrivialConstructor(sym: Symbol, params: List[Symbol],
829-
rhs: Tree): Boolean = {
830-
if (!sym.isClassConstructor || isScalaJSDefinedJSClass(sym.owner)) {
831-
false
832-
} else {
833-
rhs match {
834-
// Shape of a constructor that only calls super
835-
case Block(List(Apply(fun @ Select(_: Super, _), args)), Literal(_)) =>
836-
val callee = fun.symbol
837-
implicit val dummyPos = NoPosition
838-
839-
// Does the callee have the same signature as sym
840-
if (encodeMethodSym(sym) == encodeMethodSym(callee)) {
841-
// Test whether args are trivial forwarders
842-
assert(args.size == params.size, "Argument count mismatch")
843-
params.zip(args) forall { case (param, arg) =>
844-
arg.symbol == param
845-
}
846-
} else {
847-
false
848-
}
849-
850-
case _ => false
851-
}
852-
}
853-
}
854-
855826
/** Patches the mutable flags of selected locals in a [[js.MethodDef]].
856827
*
857828
* @param patches Map from local name to new value of the mutable flags.
@@ -1704,7 +1675,7 @@ abstract class GenJSCode extends plugins.PluginComponent
17041675
isModuleInitialized = true
17051676
val thisType = jstpe.ClassType(encodeClassFullName(currentClassSym))
17061677
val initModule = js.StoreModule(thisType, js.This()(thisType))
1707-
js.Block(superCall, initModule, js.This()(thisType))
1678+
js.Block(superCall, initModule)
17081679
} else {
17091680
superCall
17101681
}
@@ -1888,8 +1859,11 @@ abstract class GenJSCode extends plugins.PluginComponent
18881859
arguments: List[js.Tree])(implicit pos: Position): js.Tree = {
18891860
val className = encodeClassFullName(method.owner)
18901861
val methodIdent = encodeMethodSym(method)
1862+
val resultType =
1863+
if (method.isClassConstructor) jstpe.NoType
1864+
else toIRType(method.tpe.resultType)
18911865
js.ApplyStatically(receiver, jstpe.ClassType(className),
1892-
methodIdent, arguments)(toIRType(method.tpe.resultType))
1866+
methodIdent, arguments)(resultType)
18931867
}
18941868

18951869
def genTraitImplApply(method: Symbol, arguments: List[js.Tree])(

‎ir/src/main/scala/org/scalajs/core/ir/ScalaJSVersions.scala

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -21,7 +21,7 @@ object ScalaJSVersions {
2121
* - a prior release version (i.e. "0.5.0", *not* "0.5.0-SNAPSHOT")
2222
* - `current`
2323
*/
24-
val binaryEmitted: String = "0.6.5"
24+
val binaryEmitted: String = current
2525

2626
/** Versions whose binary files we can support (used by deserializer) */
2727
val binarySupported: Set[String] =

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

Lines changed: 22 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -16,6 +16,7 @@ import java.net.URI
1616

1717
import scala.collection.mutable
1818

19+
import Definitions.isConstructorName
1920
import Position._
2021
import Trees._
2122
import Types._
@@ -587,6 +588,8 @@ object Serializers {
587588

588589
private final class Deserializer(stream: InputStream, sourceVersion: String) {
589590
private[this] val useHacks060 = sourceVersion == "0.6.0"
591+
private[this] val useHacks065 =
592+
Set("0.6.0", "0.6.3", "0.6.4", "0.6.5").contains(sourceVersion)
590593

591594
private[this] val input = new DataInputStream(stream)
592595

@@ -638,7 +641,13 @@ object Serializers {
638641
case TagStoreModule => StoreModule(readClassType(), readTree())
639642
case TagSelect => Select(readTree(), readIdent())(readType())
640643
case TagApply => Apply(readTree(), readIdent(), readTrees())(readType())
641-
case TagApplyStatically => ApplyStatically(readTree(), readClassType(), readIdent(), readTrees())(readType())
644+
case TagApplyStatically =>
645+
val result1 =
646+
ApplyStatically(readTree(), readClassType(), readIdent(), readTrees())(readType())
647+
if (useHacks065 && result1.tpe != NoType && isConstructorName(result1.method.name))
648+
result1.copy()(NoType)
649+
else
650+
result1
642651
case TagApplyStatic => ApplyStatic(readClassType(), readIdent(), readTrees())(readType())
643652
case TagUnaryOp => UnaryOp(readByte(), readTree())
644653
case TagBinaryOp => BinaryOp(readByte(), readTree(), readTree())
@@ -720,14 +729,22 @@ object Serializers {
720729
// read and discard the length
721730
val len = readInt()
722731
assert(len >= 0)
723-
val result = MethodDef(readBoolean(), readPropertyName(),
732+
val result1 = MethodDef(readBoolean(), readPropertyName(),
724733
readParamDefs(), readType(), readTree())(
725734
new OptimizerHints(readInt()), optHash)
726-
if (foundArguments) {
735+
val result2 = if (foundArguments) {
727736
foundArguments = false
728-
new RewriteArgumentsTransformer().transformMethodDef(result)
737+
new RewriteArgumentsTransformer().transformMethodDef(result1)
729738
} else {
730-
result
739+
result1
740+
}
741+
if (useHacks065 && result2.resultType != NoType &&
742+
isConstructorName(result2.name.name)) {
743+
result2.copy(resultType = NoType, body = result2.body)(
744+
result2.optimizerHints, result2.hash)(
745+
result2.pos)
746+
} else {
747+
result2
731748
}
732749
case TagPropertyDef =>
733750
PropertyDef(readPropertyName(), readTree(),

‎project/JavaLangObject.scala

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -36,8 +36,8 @@ object JavaLangObject {
3636
static = false,
3737
Ident("init___", Some("<init>")),
3838
Nil,
39-
AnyType,
40-
This()(ThisType))(OptimizerHints.empty, None),
39+
NoType,
40+
Skip())(OptimizerHints.empty, None),
4141

4242
/* def hashCode(): Int = System.identityHashCode(this) */
4343
MethodDef(

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

Lines changed: 22 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -44,19 +44,32 @@ object SourceMapTest extends JasmineTest {
4444
sys.error("No exception thrown")
4545
} catch {
4646
case e @ TestException(lineNo) =>
47+
def normFileName(e: StackTraceElement): String =
48+
e.getFileName.replace('\\', '/')
49+
4750
val trace0 = e.getStackTrace.toList
4851
val trace1 = trace0.dropWhile(
49-
_.getFileName.endsWith("/scala/scalajs/runtime/StackTrace.scala"))
52+
normFileName(_).endsWith("/scala/scalajs/runtime/StackTrace.scala"))
5053
val trace2 = trace1.dropWhile(
51-
_.getFileName.endsWith("/java/lang/Throwables.scala"))
52-
53-
val exSte :: throwSte :: _ = trace2
54-
55-
expect(exSte.getFileName).toContain("/SourceMapTest.scala")
56-
// line where `case class TestException is written` above
57-
expect(exSte.getLineNumber).toBe(15)
54+
normFileName(_).endsWith("/java/lang/Throwables.scala"))
55+
56+
val topSte = trace2.head
57+
expect(normFileName(topSte)).toContain("/SourceMapTest.scala")
58+
59+
val throwSte = if (topSte.getLineNumber == 15) {
60+
// line where `case class TestException is written` above
61+
val throwSte = trace2.tail.head
62+
expect(normFileName(throwSte)).toContain("/SourceMapTest.scala")
63+
throwSte
64+
} else {
65+
/* In fullOpt, it may happen that the constructor of
66+
* TestException is inlined, in which case there is no trace of
67+
* it anymore. The first stack element in SourceMapTest.scala is
68+
* therefore the one we're interested in.
69+
*/
70+
topSte
71+
}
5872

59-
expect(throwSte.getFileName).toContain("/SourceMapTest.scala")
6073
expect(throwSte.getLineNumber).toBe(lineNo)
6174
}
6275
}

‎test-suite/js/src/test/scala/org/scalajs/testsuite/library/StackTraceTest.scala

Lines changed: 24 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -69,26 +69,34 @@ object StackTraceTest extends JasmineTest {
6969
when("nodejs").
7070
unlessAny("fullopt-stage", "strong-mode").
7171
it("decode class name and method name") {
72-
verifyClassMethodNames("Foo" -> "f") {
73-
new Foo().f(25)
74-
}
72+
val Error = js.constructorOf[js.Error]
73+
val oldStackTraceLimit = Error.stackTraceLimit
74+
Error.stackTraceLimit = 20
7575

76-
verifyClassMethodNames("Foo" -> "f", "Bar" -> "g") {
77-
new Bar().g(7)
78-
}
76+
try {
77+
verifyClassMethodNames("Foo" -> "f") {
78+
new Foo().f(25)
79+
}
7980

80-
verifyClassMethodNames("Foo" -> "f", "FooTrait$class" -> "h") {
81-
new Foo().h(78)
82-
}
81+
verifyClassMethodNames("Foo" -> "f", "Bar" -> "g") {
82+
new Bar().g(7)
83+
}
8384

84-
verifyClassMethodNames("Foo" -> "f", "FooTrait$class" -> "h",
85-
"Baz" -> "<init>") {
86-
new Baz()
87-
}
85+
verifyClassMethodNames("Foo" -> "f", "FooTrait$class" -> "h") {
86+
new Foo().h(78)
87+
}
8888

89-
verifyClassMethodNames("Foo" -> "f", "Bar" -> "g",
90-
"Foobar$" -> "<clinit>", "Foobar$" -> "<init>") {
91-
Foobar.z
89+
verifyClassMethodNames("Foo" -> "f", "FooTrait$class" -> "h",
90+
"Baz" -> "<init>") {
91+
new Baz()
92+
}
93+
94+
verifyClassMethodNames("Foo" -> "f", "Bar" -> "g",
95+
"Foobar$" -> "<clinit>", "Foobar$" -> "<init>") {
96+
Foobar.z
97+
}
98+
} finally {
99+
Error.stackTraceLimit = oldStackTraceLimit
92100
}
93101
}
94102

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

Lines changed: 13 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -320,9 +320,21 @@ final class ScalaJSClassEmitter private (
320320
def genMethod(className: String, method: MethodDef): js.Tree = {
321321
implicit val pos = method.pos
322322

323-
val methodFun = desugarToFunction(this, className,
323+
val methodFun0 = desugarToFunction(this, className,
324324
method.args, method.body, method.resultType == NoType)
325325

326+
val methodFun = if (Definitions.isConstructorName(method.name.name)) {
327+
// init methods have to return `this` so that we can chain them to `new`
328+
js.Function(methodFun0.args, {
329+
implicit val pos = methodFun0.body.pos
330+
js.Block(
331+
methodFun0.body,
332+
js.Return(js.This()))
333+
})(methodFun0.pos)
334+
} else {
335+
methodFun0
336+
}
337+
326338
outputMode match {
327339
case OutputMode.ECMAScript6StrongMode =>
328340
js.MethodDef(static = method.static, genPropertyName(method.name),

‎tools/shared/src/main/scala/org/scalajs/core/tools/optimizer/Analysis.scala

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -66,6 +66,15 @@ object Analysis {
6666
def calledFrom: Seq[From]
6767
def instantiatedSubclasses: Seq[ClassInfo]
6868
def nonExistent: Boolean
69+
def syntheticKind: MethodSyntheticKind
70+
}
71+
72+
sealed trait MethodSyntheticKind
73+
74+
object MethodSyntheticKind {
75+
final case object None extends MethodSyntheticKind
76+
// TODO Get rid of InheritedConstructor when we can break binary compat
77+
final case object InheritedConstructor extends MethodSyntheticKind
6978
}
7079

7180
sealed trait Error {

0 commit comments

Comments
 (0)