Skip to content

Commit 819c872

Browse files
committed
Constructors are not inherited anymore at the IR level.
In JVM bytecode, constructors are not inherited. Due to historical reasons, the Scala.js IR has supported inherited constructors, and the compiler avoided generating so-called "trivial" constructors, which only call their super constructor. In this commit, we change the specification of the IR so that constructors are not inherited anymore. For backward binary compatibility, the analyzer still tolerates not to find a constructor in the class where it is looked for. If it finds one in an inherited class, the linker synthesizes the missing constructor. As is, this introduces more code in the generated JavaScript files, since no effort is made to avoid emitting useless JavaScript init methods. This will be addressed in a follow up commit at the class emitter level. This commit does not modify the compiler yet (it still avoids trivial constructors in the IR) so that we can test the backward binary compatibility hack.
1 parent 3a3f83a commit 819c872

File tree

6 files changed

+164
-35
lines changed

6 files changed

+164
-35
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
;;

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/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 {

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

Lines changed: 49 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -23,10 +23,16 @@ import org.scalajs.core.tools.javascript.{LongImpl, OutputMode}
2323
import ScalaJSOptimizer._
2424

2525
final class Analyzer(semantics: Semantics, outputMode: OutputMode,
26-
reachOptimizerSymbols: Boolean) extends Analysis {
26+
reachOptimizerSymbols: Boolean, initialLink: Boolean) extends Analysis {
2727
import Analyzer._
2828
import Analysis._
2929

30+
@deprecated("Use the overload with an explicit `initialLink` flag", "0.6.6")
31+
def this(semantics: Semantics, outputMode: OutputMode,
32+
reachOptimizerSymbols: Boolean) = {
33+
this(semantics, outputMode, reachOptimizerSymbols, initialLink = true)
34+
}
35+
3036
@deprecated("Use the overload with an explicit output mode", "0.6.3")
3137
def this(semantics: Semantics, reachOptimizerSymbols: Boolean) =
3238
this(semantics, OutputMode.ECMAScript51Isolated, reachOptimizerSymbols)
@@ -283,16 +289,50 @@ final class Analyzer(semantics: Semantics, outputMode: OutputMode,
283289
(mutable.Map(methodInfos: _*), mutable.Map(staticMethodInfos: _*))
284290
}
285291

292+
def lookupConstructor(ctorName: String): MethodInfo = {
293+
/* As of 0.6.6, constructors are not inherited, and so must be found
294+
* directly in this class. However, to be able to read sjsir files from
295+
* before 0.6.6, we tolerate finding it in a superclass, in which case
296+
* we materialize a new constructor in this class. We only allow this
297+
* during the initial link. In a refiner, this must not happen anymore.
298+
*/
299+
methodInfos.get(ctorName).getOrElse {
300+
if (!initialLink) {
301+
createNonExistentMethod(ctorName)
302+
} else {
303+
val inherited = lookupMethod(ctorName)
304+
if (inherited.owner eq this) {
305+
// Can happen only for non-existent constructors, at this point
306+
assert(inherited.nonExistent)
307+
inherited
308+
} else {
309+
val syntheticInfo = Infos.MethodInfo(
310+
encodedName = ctorName,
311+
methodsCalledStatically = Map(
312+
superClass.encodedName -> List(ctorName)))
313+
val m = new MethodInfo(this, syntheticInfo)
314+
m.syntheticKind = MethodSyntheticKind.InheritedConstructor
315+
methodInfos += ctorName -> m
316+
m
317+
}
318+
}
319+
}
320+
}
321+
286322
def lookupMethod(methodName: String): MethodInfo = {
287323
tryLookupMethod(methodName).getOrElse {
288-
val syntheticData = createMissingMethodInfo(methodName)
289-
val m = new MethodInfo(this, syntheticData)
290-
m.nonExistent = true
291-
methodInfos += methodName -> m
292-
m
324+
createNonExistentMethod(methodName)
293325
}
294326
}
295327

328+
private def createNonExistentMethod(methodName: String): MethodInfo = {
329+
val syntheticData = createMissingMethodInfo(methodName)
330+
val m = new MethodInfo(this, syntheticData)
331+
m.nonExistent = true
332+
methodInfos += methodName -> m
333+
m
334+
}
335+
296336
def tryLookupMethod(methodName: String): Option[MethodInfo] = {
297337
assert(isScalaClass,
298338
s"Cannot call lookupMethod($methodName) on non Scala class $this")
@@ -416,7 +456,7 @@ final class Analyzer(semantics: Semantics, outputMode: OutputMode,
416456
// constructors must always be called statically
417457
assert(statically,
418458
s"Trying to call dynamically the constructor $this.$methodName from $from")
419-
lookupMethod(methodName).reachStatic()
459+
lookupConstructor(methodName).reachStatic()
420460
} else if (statically) {
421461
assert(!isReflProxyName(methodName),
422462
s"Trying to call statically refl proxy $this.$methodName")
@@ -460,6 +500,8 @@ final class Analyzer(semantics: Semantics, outputMode: OutputMode,
460500

461501
var nonExistent: Boolean = false
462502

503+
var syntheticKind: MethodSyntheticKind = MethodSyntheticKind.None
504+
463505
override def toString(): String = s"$owner.$encodedName"
464506

465507
def reachStatic()(implicit from: From): Unit = {

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

Lines changed: 68 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,8 @@
99

1010
package org.scalajs.core.tools.optimizer
1111

12+
import scala.annotation.tailrec
13+
1214
import scala.collection.mutable
1315
import scala.collection.immutable.{Seq, Traversable}
1416

@@ -19,7 +21,8 @@ import org.scalajs.core.tools.io._
1921

2022
import org.scalajs.core.ir
2123
import ir.Infos
22-
import ir.Trees.ClassDef
24+
import ir.Trees._
25+
import ir.Types._
2326
import ir.ClassKind
2427
import ir.Hashers
2528
import ir.Position
@@ -91,7 +94,8 @@ final class Linker(semantics: Semantics, outputMode: OutputMode,
9194
}
9295

9396
val analysis = logTime(logger, "Linker: Compute reachability") {
94-
val analyzer = new Analyzer(semantics, outputMode, reachOptimizerSymbols)
97+
val analyzer = new Analyzer(semantics, outputMode, reachOptimizerSymbols,
98+
initialLink = true)
9599
analyzer.computeReachability(infoInput)
96100
}
97101

@@ -134,7 +138,7 @@ final class Linker(semantics: Semantics, outputMode: OutputMode,
134138
infoByName.get(encodedName).map { info =>
135139
val (tree, version) = getTree(encodedName)
136140
val newVersion = version.map("real" + _) // avoid collision with dummy
137-
linkedClassDef(info, tree, analyzerInfo, newVersion)
141+
linkedClassDef(info, tree, analyzerInfo, newVersion, getTree, analysis)
138142
}.orElse(optDummyParent)
139143
}
140144

@@ -150,7 +154,8 @@ final class Linker(semantics: Semantics, outputMode: OutputMode,
150154
/** Takes a Infos, a ClassDef and DCE infos to construct a stripped down
151155
* LinkedClassDef */
152156
private def linkedClassDef(info: Infos.ClassInfo, classDef: ClassDef,
153-
analyzerInfo: Analysis.ClassInfo, version: Option[String]) = {
157+
analyzerInfo: Analysis.ClassInfo, version: Option[String],
158+
getTree: TreeProvider, analysis: Analysis) = {
154159
import ir.Trees._
155160

156161
val memberInfoByName = Map(info.methods.map(m => m.encodedName -> m): _*)
@@ -173,6 +178,12 @@ final class Linker(semantics: Semantics, outputMode: OutputMode,
173178
new LinkedMember(info, p, None)
174179
}
175180

181+
def linkedSyntheticMethod(m: MethodDef) = {
182+
val info = Infos.generateMethodInfo(m)
183+
val version = m.hash.map(Hashers.hashAsVersion(_, considerPositions))
184+
new LinkedMember(info, m, version)
185+
}
186+
176187
classDef.defs.foreach {
177188
// Static methods
178189
case m: MethodDef if m.static =>
@@ -212,6 +223,22 @@ final class Linker(semantics: Semantics, outputMode: OutputMode,
212223
sys.error(s"Illegal tree in ClassDef of class ${tree.getClass}")
213224
}
214225

226+
// Synthetic members
227+
for {
228+
m <- analyzerInfo.methodInfos.valuesIterator
229+
if m.isReachable
230+
} {
231+
m.syntheticKind match {
232+
case MethodSyntheticKind.None =>
233+
// nothing to do
234+
235+
case MethodSyntheticKind.InheritedConstructor =>
236+
val syntheticMDef = synthesizeInheritedConstructor(
237+
analyzerInfo, m, getTree, analysis)(classDef.pos)
238+
memberMethods += linkedSyntheticMethod(syntheticMDef)
239+
}
240+
}
241+
215242
val classExportInfo =
216243
memberInfoByName.get(Definitions.ExportedConstructorsName)
217244

@@ -243,6 +270,43 @@ final class Linker(semantics: Semantics, outputMode: OutputMode,
243270
version)
244271
}
245272

273+
private def synthesizeInheritedConstructor(
274+
classInfo: Analysis.ClassInfo, methodInfo: Analysis.MethodInfo,
275+
getTree: TreeProvider, analysis: Analysis)(
276+
implicit pos: Position): MethodDef = {
277+
val encodedName = methodInfo.encodedName
278+
279+
@tailrec
280+
def findInheritedMethodDef(ancestorInfo: Analysis.ClassInfo): MethodDef = {
281+
val inherited = ancestorInfo.methodInfos(methodInfo.encodedName)
282+
if (inherited.syntheticKind == MethodSyntheticKind.None) {
283+
val (classDef, _) = getTree(ancestorInfo.encodedName)
284+
classDef.defs.collectFirst {
285+
case mDef: MethodDef if mDef.name.name == encodedName => mDef
286+
}.getOrElse {
287+
throw new AssertionError(
288+
s"Cannot find $encodedName in ${ancestorInfo.encodedName}")
289+
}
290+
} else {
291+
findInheritedMethodDef(ancestorInfo.superClass)
292+
}
293+
}
294+
295+
val inheritedMDef = findInheritedMethodDef(classInfo.superClass)
296+
297+
val origName = inheritedMDef.name.asInstanceOf[Ident].originalName
298+
val ctorIdent = Ident(encodedName, origName)
299+
val params = inheritedMDef.args.map(_.copy()) // for the new pos
300+
val currentClassType = ClassType(classInfo.encodedName)
301+
val superClassType = ClassType(classInfo.superClass.encodedName)
302+
MethodDef(static = false, ctorIdent,
303+
params, NoType,
304+
ApplyStatically(This()(currentClassType),
305+
superClassType, ctorIdent, params.map(_.ref))(NoType))(
306+
OptimizerHints.empty,
307+
inheritedMDef.hash) // over-approximation
308+
}
309+
246310
private def startRun(): Unit = {
247311
statsReused = 0
248312
statsInvalidated = 0

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

Lines changed: 8 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -16,11 +16,17 @@ final class Refiner(semantics: Semantics, outputMode: OutputMode) {
1616
def refine(unit: LinkingUnit, logger: Logger): LinkingUnit = {
1717
val analysis = logTime(logger, "Refiner: Compute reachability") {
1818
val analyzer = new Analyzer(semantics, outputMode,
19-
reachOptimizerSymbols = false)
19+
reachOptimizerSymbols = false, initialLink = false)
2020
analyzer.computeReachability(unit.infos.values.toList)
2121
}
2222

23-
// Note: We ignore all errors of the analysis
23+
/* There really should not be linking errors at this point. If there are,
24+
* it is most likely a bug in the optimizer. We should crash here, but we
25+
* used to silently ignore any errors before 0.6.6. So currently we only
26+
* warn, not to break compatibility.
27+
* TODO Issue errors when we can break backward compatibility.
28+
*/
29+
analysis.errors.foreach(Analysis.logError(_, logger, Level.Warn))
2430

2531
logTime(logger, "Refiner: Assemble LinkedClasses") {
2632
val linkedClassesByName =

0 commit comments

Comments
 (0)