8000 Remove ancient codegen opt for expanded Scala anon function classes. · scala-js/scala-js@ca39c30 · GitHub
[go: up one dir, main page]
Skip to content

Commit ca39c30

Browse files
sjrdsjrd
committed
Remove ancient codegen opt for expanded Scala anon function classes.
Since the very early days of Scala.js, we have had a code size optimization for lambdas expanded as anonymous classes. We tried very hard to recover the captures from 10000 fields, and the body of the `apply` method, in order to reconstruct a `js.Closure`. That made sense as long as we supported Scala 2.11 (and before that, 2.10). However, since Scala 2.12.x, scalac almost never expands lambdas of `scala.FunctionN`. Instead, they reach the backend as `Function` node. The only situations where scalac still expands lambdas as anonymous classes is if it used in an argument to a super constructor (or delegate `this()` constructor). The code paths for that code size optimization are therefore almost dead code, and do not pull their weight anymore. It was, AFAICT, the only place where we still *tried* to emit something a certain way, with a fallback. This commit removes the optimization and all the related infrastructure.
1 parent fb051e5 commit ca39c30

File tree

2 files changed

+39
-152
lines changed

2 files changed

+39
-152
lines changed

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

Lines changed: 37 additions & 150 deletions
Original file line numberDiff line numberDiff line change
@@ -204,8 +204,7 @@ abstract class GenJSCode[G <: Global with Singleton](val global: G)
204204
}
205205

206206
// For anonymous methods
207-
// These have a default, since we always read them.
208-
private val tryingToGenMethodAsJSFunction = new ScopedVar[Boolean](false)
207+
// It has a default, since we always read it.
209208
private val paramAccessorLocals = new ScopedVar(Map.empty[Symbol, js.ParamDef])
210209

211210
/* Contextual JS class value for some operations of nested JS classes that
@@ -223,11 +222,6 @@ abstract class GenJSCode[G <: Global with Singleton](val global: G)
223222
}
224223
}
225224

226-
private class CancelGenMethodAsJSFunction(message: String)
227-
extends scala.util.control.ControlThrowable {
228-
override def getMessage(): String = message
229-
}
230-
231225
// Rewriting of anonymous function classes ---------------------------------
232226

233227
/** Start nested generation of a class.
@@ -248,7 +242,6 @@ abstract class GenJSCode[G <: Global with Singleton](val global: G)
248242
undefinedDefaultParams := null,
249243
mutableLocalVars := null,
250244
mutatedLocalVars := null,
251-
tryingToGenMethodAsJSFunction := false,
252245
paramAccessorLocals := Map.empty
253246
)(withNewLocalNameScope(body))
254247
}
@@ -387,21 +380,7 @@ abstract class GenJSCode[G <: Global with Singleton](val global: G)
387380
private val generatedStaticForwarderClasses = ListBuffer.empty[(Symbol, js.ClassDef)]
388381

389382
private def consumeLazilyGeneratedAnonClass(sym: Symbol): ClassDef = {
390-
/* If we are trying to generate an method as JSFunction, we cannot
391-
* actually consume the symbol, since we might fail trying and retry.
392-
* We will then see the same tree again and not find the symbol anymore.
393-
*
394-
* If we are sure this is the only generation, we remove the symbol to
395-
* make sure we don't generate the same class twice.
396-
*/
397-
val optDef = {
398-
if (tryingToGenMethodAsJSFunction)
399-
lazilyGeneratedAnonClasses.get(sym)
400-
else
401-
lazilyGeneratedAnonClasses.remove(sym)
402-
}
403-
404-
optDef.getOrElse {
383+
lazilyGeneratedAnonClasses.remove(sym).getOrElse {
405384
abort("Couldn't find tree for lazily generated anonymous class " +
406385
s"${sym.fullName} at ${sym.pos}")
407386
}
@@ -450,25 +429,22 @@ abstract class GenJSCode[G <: Global with Singleton](val global: G)
450429
}
451430
val allClassDefs = collectClassDefs(cunit.body)
452431

453-
/* There are three types of anonymous classes we want to generate
454-
* only once we need them so we can inline them at construction site:
432+
/* There are two types of anonymous classes we want to generate only
433+
* once we need them, so we can inline them at construction site:
455434
*
456-
* - anonymous class that are JS types, which includes:
457-
* - lambdas for js.FunctionN and js.ThisFunctionN (SAMs). (We may
458-
* not generate actual Scala classes for these).
459-
* - anonymous (non-lambda) JS classes. These classes may not have
460-
* their own prototype. Therefore, their constructor *must* be
461-
* inlined.
462-
* - lambdas for scala.FunctionN. This is only an optimization and may
463-
* fail. In the case of failure, we fall back to generating a
464-
* fully-fledged Scala class.
435+
* - Lambdas for `js.Function` SAMs, including `js.FunctionN`,
436+
* `js.ThisFunctionN` and custom JS function SAMs. We must generate
437+
* JS functions for these, instead of actual classes.
438+
* - Anonymous (non-lambda) JS classes. These classes may not have
439+
* their own prototype. Therefore, their constructor *must* be
440+
* inlined.
465441
*
466442
* Since for all these, we don't know how they inter-depend, we just
467443
* store them in a map at this point.
468444
*/
469445
val (lazyAnons, fullClassDefs) = allClassDefs.partition { cd =>
470446
val sym = cd.symbol
471-
isAnonymousJSClass(sym) || isJSFunctionDef(sym) || sym.isAnonymousFunction
447+
isAnonymousJSClass(sym) || isJSFunctionDef(sym)
472448
}
473449

474450
lazilyGeneratedAnonClasses ++= lazyAnons.map(cd => cd.symbol -> cd)
@@ -2824,9 +2800,10 @@ abstract class GenJSCode[G <: Global with Singleton](val global: G)
28242800
*/
28252801
private def genThis()(implicit pos: Position): js.Tree = {
28262802
thisLocalVarName.fold[js.Tree] {
2827-
if (tryingToGenMethodAsJSFunction) {
2828-
throw new CancelGenMethodAsJSFunction(
2829-
"Trying to generate `this` inside the body")
2803+
if (isJSFunctionDef(currentClassSym)) {
2804+
abort(
2805+
"Unexpected `this` reference inside the body of a JS function class: " +
2806+
currentClassSym.fullName)
28302807
}
28312808
js.This()(currentThisType)
28322809
} { thisLocalName =>
@@ -3359,10 +3336,10 @@ abstract class GenJSCode[G <: Global with Singleton](val global: G)
33593336
}
33603337

33613338
/** Gen JS code for a constructor call (new).
3339+
*
33623340
* Further refined into:
3363-
* * new String(...)
33643341
* * new of a hijacked boxed class
3365-
* * new of an anonymous function class that was recorded as JS function
3342+
* * new of a JS function class
33663343
* * new of a JS class
33673344
* * new Array
33683345
* * regular new
@@ -3382,13 +3359,6 @@ abstract class GenJSCode[G <: Global with Singleton](val global: G)
33823359
} else if (isJSFunctionDef(clsSym)) {
33833360
val classDef = consumeLazilyGeneratedAnonClass(clsSym)
33843361
genJSFunction(classDef, args.map(genExpr))
3385-
} else if (clsSym.isAnonymousFunction) {
3386-
val classDef = consumeLazilyGeneratedAnonClass(clsSym)
3387-
tryGenAnonFunctionClass(classDef, args.map(genExpr)).getOrElse {
3388-
// Cannot optimize anonymous function class. Generate full class.
3389-
generatedClasses += nestedGenerateClass(clsSym)(genClass(classDef)) -> clsSym.pos
3390-
genNew(clsSym, ctor, genActualArgs(ctor, args))
3391-
}
33923362
} else if (isJSType(clsSym)) {
33933363
genPrimitiveJSNew(tree)
33943364
} else {
@@ -6057,69 +6027,6 @@ abstract class GenJSCode[G <: Global with Singleton](val global: G)
60576027

60586028
// Synthesizers for JS functions -------------------------------------------
60596029

6060-
/** Try and generate JS code for an anonymous function class.
6061-
*
6062-
* Returns Some(<js code>) if the class could be rewritten that way, None
6063-
* otherwise.
6064-
*
6065-
* We make the following assumptions on the form of such classes:
6066-
* - It is an anonymous function
6067-
* - Includes being anonymous, final, and having exactly one constructor
6068-
* - It is not a PartialFunction
6069-
* - It has no field other than param accessors
6070-
* - It has exactly one constructor
6071-
* - It has exactly one non-bridge method apply if it is not specialized,
6072-
* or a method apply$...$sp and a forwarder apply if it is specialized.
6073-
* - As a precaution: it is synthetic
6074-
*
6075-
* From a class looking like this:
6076-
*
6077-
* final class <anon>(outer, capture1, ..., captureM) extends AbstractionFunctionN[...] {
6078-
* def apply(param1, ..., paramN) = {
6079-
* <body>
6080-
* }
6081-
* }
6082-
* new <anon>(o, c1, ..., cM)
6083-
*
6084-
* we generate a function:
6085-
*
6086-
* arrow-lambda<o = outer, c1 = capture1, ..., cM = captureM>(param1, ..., paramN) {
6087-
* <body>
6088-
* }
6089-
*
6090-
* so that, at instantiation point, we can write:
6091-
*
6092-
* new AnonFunctionN(function)
6093-
*
6094-
* the latter tree is returned in case of success.
6095-
*
6096-
* Trickier things apply when the function is specialized.
6097-
*/
6098-
private def tryGenAnonFunctionClass(cd: ClassDef,
6099-
capturedArgs: List[js.Tree]): Option[js.Tree] = {
6100-
// scalastyle:off return
6101-
implicit val pos = cd.pos
6102-
val sym = cd.symbol
6103-
assert(sym.isAnonymousFunction,
6104-
s"tryGenAndRecordAnonFunctionClass called with non-anonymous function $cd")
6105-
6106-
if (!sym.superClass.fullName.startsWith("scala.runtime.AbstractFunction")) {
6107-
/* This is an anonymous class for a non-LMF capable SAM in 2.12.
6108-
* We must not rewrite it, as it would then not inherit from the
6109-
* appropriate parent class and/or interface.
6110-
*/
6111-
None
6112-
} else {
6113-
nestedGenerateClass(sym) {
6114-
val (functionBase, arity) =
6115-
tryGenAnonFunctionClassGeneric(cd, capturedArgs)(_ => return None)
6116-
6117-
Some(genJSFunctionToScala(functionBase, arity))
6118-
}
6119-
}
6120-
// scalastyle:on return
6121-
}
6122-
61236030
/** Gen a conversion from a JavaScript function into a Scala function. */
61246031
private def genJSFunctionToScala(jsFunction: js.Tree, arity: Int)(
E377 61256032
implicit pos: Position): js.Tree = {
@@ -6136,11 +6043,9 @@ abstract class GenJSCode[G <: Global with Singleton](val global: G)
61366043
* functions are not classes, we deconstruct the ClassDef, then
61376044
* reconstruct it to be a genuine Closure.
61386045
*
6139-
* Compared to `tryGenAnonFunctionClass()`, this function must
6140-
* always succeed, because we really cannot afford keeping them as
6141-
* anonymous classes. The good news is that it can do so, because the
6142-
* body of SAM lambdas is hoisted in the enclosing class. Hence, the
6143-
* apply() method is just a forwarder to calling that hoisted method.
6046+
* We can always do so, because the body of SAM lambdas is hoisted in the
6047+
* enclosing class. Hence, the apply() method is just a forwarder to
6048+
* calling that hoisted method.
61446049
*
61456050
* From a class looking like this:
61466051
*
@@ -6163,26 +6068,18 @@ abstract class GenJSCode[G <: Global with Singleton](val global: G)
61636068
s"genAndRecordJSFunctionClass called with non-JS function $cd")
61646069

61656070
nestedGenerateClass(sym) {
6166-
val (function, _) = tryGenAnonFunctionClassGeneric(cd, captures)(msg =>
6167-
abort(s"Could not generate function for JS function: $msg"))
6168-
6169-
function
6071+
genJSFunctionInner(cd, captures)
61706072
}
61716073
}
61726074

6173-
/** Code common to tryGenAndRecordAnonFunctionClass and
6174-
* genAndRecordJSFunctionClass.
6175-
*/
6176-
private def tryGenAnonFunctionClassGeneric(cd: ClassDef,
6177-
initialCapturedArgs: List[js.Tree])(
6178-
fail: (=> String) => Nothing): (js.Tree, Int) = {
6075+
/** The code of `genJSFunction` that is inside the `nestedGenerateClass` wrapper. */
6076+
private def genJSFunctionInner(cd: ClassDef,
6077+
initialCapturedArgs: List[js.Tree]): js.Closure = {
61796078
implicit val pos = cd.pos
61806079
val sym = cd.symbol
61816080

6182-
// First checks
6183-
6184-
if (sym.isSubClass(PartialFunctionClass))
6185-
fail(s"Cannot rewrite PartialFunction $cd")
6081+
def fail(reason: String): Nothing =
6082+
abort(s"Could not generate function for JS function: $reason")
61866083

61876084
// First step: find the apply method def, and collect param accessors
61886085

@@ -6210,10 +6107,12 @@ abstract class GenJSCode[G <: Global with Singleton](val global: G)
62106107
if (!ddsym.isPrimaryConstructor)
62116108
fail(s"Non-primary constructor $ddsym in anon function $cd")
62126109
} else {
6213-
val name = dd.name.toString
6214-
if (name == "apply" || (ddsym.isSpecialized && name.startsWith("apply$"))) {
6215-
if ((applyDef eq null) || ddsym.isSpecialized)
6110+
if (dd.name == nme.apply) {
6111+
if (!ddsym.isBridge) {
6112+
if (applyDef ne null)
6113+
fail(s"Found duplicate apply method $ddsym in $cd")
62166114
applyDef = dd
6115+
}
62176116
} else if (ddsym.hasAnnotation(JSOptionalAnnotation)) {
62186117
// Ignore (this is useful for default parameters in custom JS function types)
62196118
} else {
@@ -6253,24 +6152,15 @@ abstract class GenJSCode[G <: Global with Singleton](val global: G)
62536152
// Third step: emit the body of the apply method def
62546153

62556154
val applyMethod = withScopedVars(
6256-
paramAccessorLocals := (paramAccessors zip ctorParamDefs).toMap,
6257-
tryingToGenMethodAsJSFunction := true
6155+
paramAccessorLocals := (paramAccessors zip ctorParamDefs).toMap
62586156
) {
6259-
try {
6260-
genMethodWithCurrentLocalNameScope(applyDef)
6261-
} catch {
6262-
case e: CancelGenMethodAsJSFunction =>
6263-
fail(e.getMessage)
6264-
}
6157+
genMethodWithCurrentLocalNameScope(applyDef)
62656158
}
62666159

62676160
// Fourth step: patch the body to unbox parameters and box result
62686161

6269-
val hasRepeatedParam = {
6270-
sym.superClass == JSFunctionClass && // Scala functions are known not to have repeated params
6271-
enteringUncurry {
6272-
applyDef.symbol.paramss.flatten.lastOption.exists(isRepeated(_))
6273-
}
6162+
val hasRepeatedParam = enteringUncurry {
6163+
applyDef.symbol.paramss.flatten.lastOption.exists(isRepeated(_))
62746164
}
62756165

62766166
val js.MethodDef(_, _, _, params, _, body) = applyMethod
@@ -6303,8 +6193,7 @@ abstract class GenJSCode[G <: Global with Singleton](val global: G)
63036193
val ok = patchedParams.nonEmpty
63046194
if (!ok) {
63056195
reporter.error(pos,
6306-
"The SAM or apply method for a js.ThisFunction must have a " +
6307-
"leading non-varargs parameter")
6196+
"The apply method for a js.ThisFunction must have a leading non-varargs parameter")
63086197
}
63096198
ok
63106199
}
@@ -6335,9 +6224,7 @@ abstract class GenJSCode[G <: Global with Singleton](val global: G)
63356224
}
63366225
}
63376226

6338-
val arity = params.size
6339-
6340-
(closure, arity)
6227+
closure
63416228
}
63426229
}
63436230

‎compiler/src/test/scala/org/scalajs/nscplugin/test/JSSAMTest.scala

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -126,10 +126,10 @@ class JSSAMTest extends DirectTest with TestHelpers {
126126
}
127127
""" hasErrors
128128
"""
129-
|newSource1.scala:14: error: The SAM or apply method for a js.ThisFunction must have a leading non-varargs parameter
129+
|newSource1.scala:14: error: The apply method for a js.ThisFunction must have a leading non-varargs parameter
130130
| val badThisFunction1: BadThisFunction1 = () => 42
131131
| ^
132-
|newSource1.scala:15: error: The SAM or apply method for a js.ThisFunction must have a leading non-varargs parameter
132+
|newSource1.scala:15: error: The apply method for a js.ThisFunction must have a leading non-varargs parameter
133133
| val badThisFunction2: BadThisFunction2 = args => args.size
134134
| ^
135135
"""

0 commit comments

Comments
 (0)
0