8000 Merge pull request #3323 from sjrd/no-return-without-label · scala-js/scala-js@d592b99 · GitHub
[go: up one dir, main page]
Skip to content

Commit d592b99

Browse files
gzm0gzm0
authored
Merge pull request #3323 from sjrd/no-return-without-label
Make the label of `Return` nodes mandatory.
2 parents 72a6809 + c61d78d commit d592b99

File tree

10 files changed

+103
-82
lines changed

10 files changed

+103
-82
lines changed

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

Lines changed: 19 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -1716,7 +1716,7 @@ abstract class GenJSCode extends plugins.PluginComponent
17161716

17171717
val bodyIsStat = resultIRType == jstpe.NoType
17181718

1719-
def genBody() = tree match {
1719+
def genBodyWithinReturnableScope(): js.Tree = tree match {
17201720
case Block(
17211721
(thisDef @ ValDef(_, nme.THIS, _, initialThis)) :: otherStats,
17221722
rhs) =>
@@ -1793,6 +1793,12 @@ abstract class GenJSCode extends plugins.PluginComponent
17931793
else genExpr(tree)
17941794
}
17951795

1796+
def genBody(): js.Tree = {
1797+
withNewReturnableScope(resultIRType) {
1798+
genBodyWithinReturnableScope()
1799+
}
1800+
}
1801+
17961802
if (!isNonNativeJSClass(currentClassSym) ||
17971803
isRawJSFunctionDef(currentClassSym)) {
17981804
val body = {
@@ -1958,7 +1964,7 @@ abstract class GenJSCode extends plugins.PluginComponent
19581964
js.Return(toIRType(expr.tpe) match {
19591965
case jstpe.NoType => js.Block(genStat(expr), js.Undefined())
19601966
case _ => genExpr(expr)
1961-
}, None)
1967+
}, getEnclosingReturnLabel())
19621968

19631969
case t: Try =>
19641970
genTry(t, isStat)
@@ -2267,9 +2273,9 @@ abstract class GenJSCode extends plugins.PluginComponent
22672273
js.While(js.BooleanLiteral(true), {
22682274
js.Labeled(labelIdent, jstpe.NoType, {
22692275
if (bodyType == jstpe.NoType)
2270-
js.Block(genStat(rhs), js.Return(js.Undefined(), Some(blockLabelIdent)))
2276+
js.Block(genStat(rhs), js.Return(js.Undefined(), blockLabelIdent))
22712277
else
2272-
js.Return(genExpr(rhs), Some(blockLabelIdent))
2278+
js.Return(genExpr(rhs), blockLabelIdent)
22732279
})
22742280
})
22752281
})
@@ -2634,7 +2640,7 @@ abstract class GenJSCode extends plugins.PluginComponent
26342640
* labeled block surrounding the match.
26352641
*/
26362642
countsOfReturnsToMatchEnd(sym) += 1
2637-
js.Return(genExpr(args.head), Some(encodeLabelSym(sym)))
2643+
js.Return(genExpr(args.head), encodeLabelSym(sym))
26382644
} else {
26392645
/* No other label apply should ever happen. If it does, then we
26402646
* have missed a pattern of LabelDef/LabelApply and some new
@@ -2715,7 +2721,7 @@ abstract class GenJSCode extends plugins.PluginComponent
27152721
}
27162722

27172723
// The actual jump (return(labelDefIdent) undefined;)
2718-
val jump = js.Return(js.Undefined(), Some(encodeLabelSym(sym)))
2724+
val jump = js.Return(js.Undefined(), encodeLabelSym(sym))
27192725

27202726
quadruplets match {
27212727
case Nil => jump
@@ -3078,7 +3084,7 @@ abstract class GenJSCode extends plugins.PluginComponent
30783084
def genJumpToElseClause(implicit pos: ir.Position): js.Tree = {
30793085
if (optElseClauseLabel.isEmpty)
30803086
optElseClauseLabel = Some(freshLocalIdent("default"))
3081-
js.Return(js.Undefined(), optElseClauseLabel)
3087+
js.Return(js.Undefined(), optElseClauseLabel.get)
30823088
}
30833089

30843090
for (caze @ CaseDef(pat, guard, body) <- cases) {
@@ -3160,10 +3166,9 @@ abstract class GenJSCode extends plugins.PluginComponent
31603166
val matchResultLabel = freshLocalIdent("matchResult")
31613167
val patchedClauses = for ((alts, body) <- clauses) yield {
31623168
implicit val pos = body.pos
3163-
val lab = Some(matchResultLabel)
31643169
val newBody =
3165-
if (isStat) js.Block(body, js.Return(js.Undefined(), lab))
3166-
else js.Return(body, lab)
3170+
if (isStat) js.Block(body, js.Return(js.Undefined(), matchResultLabel))
3171+
else js.Return(body, matchResultLabel)
31673172
(alts, newBody)
31683173
}
31693174
js.Labeled(matchResultLabel, resultType, js.Block(List(
@@ -3220,11 +3225,12 @@ abstract class GenJSCode extends plugins.PluginComponent
32203225
// Peculiar shape generated by `return x match {...}` - #2928
32213226
case Return(retExpr: LabelDef) if isCaseLabelDef(retExpr) =>
32223227
val result = translateMatch(retExpr)
3228+
val label = getEnclosingReturnLabel()
32233229
if (result.tpe == jstpe.NoType) {
32243230
// Could not actually reproduce this, but better be safe than sorry
3225-
js.Block(result, js.Return(js.Undefined(), None))
3231+
js.Block(result, js.Return(js.Undefined(), label))
32263232
} else {
3227-
js.Return(result, None)
3233+
js.Return(result, label)
32283234
}
32293235

32303236
case _ =>
@@ -3354,7 +3360,7 @@ abstract class GenJSCode extends plugins.PluginComponent
33543360

33553361
if (revAlts.size == returnCount - 1) {
33563362
def tryDropReturn(body: js.Tree): Option[js.Tree] = body match {
3357-
case jse.BlockOrAlone(prep, js.Return(result, Some(`label`))) =>
3363+
case jse.BlockOrAlone(prep, js.Return(result, `label`)) =>
33583364
Some(js.Block(prep :+ result)(body.pos))
33593365

33603366
case _ =>

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

Lines changed: 27 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -12,7 +12,7 @@ import scala.tools.nsc._
1212
import org.scalajs.ir
1313
import ir.{Trees => js, Types => jstpe}
1414

15-
import util.ScopedVar
15+
import util.{ScopedVar, VarBox}
1616
import ScopedVar.withScopedVars
1717

1818
/** Encoding of symbol names for JavaScript
@@ -59,13 +59,15 @@ trait JSEncoding extends SubComponent { self: GenJSCode =>
5959
// Fresh local name generator ----------------------------------------------
6060

6161
private val usedLocalNames = new ScopedVar[mutable.Set[String]]
62+
private val returnLabelName = new ScopedVar[VarBox[Option[String]]]
6263
private val localSymbolNames = new ScopedVar[mutable.Map[Symbol, String]]
6364
private val isReserved =
6465
Set("arguments", "eval", ScalaJSEnvironmentName)
6566

6667
def withNewLocalNameScope[A](body: => A): A = {
6768
withScopedVars(
6869
usedLocalNames := mutable.Set.empty,
70+
returnLabelName := null,
6971
localSymbolNames := mutable.Map.empty
7072
)(body)
7173
}
@@ -77,6 +79,21 @@ trait JSEncoding extends SubComponent { self: GenJSCode =>
7779
usedLocalNames += name
7880
}
7981

82+
def withNewReturnableScope(tpe: jstpe.Type)(body: => js.Tree)(
83+
implicit pos: ir.Position): js.Tree = {
84+
withScopedVars(
85+
returnLabelName := new VarBox(None)
86+
) {
87+
val inner = body
88+
returnLabelName.get.value match {
89+
case None =>
90+
inner
91+
case Some(labelName) =>
92+
js.Labeled(js.Ident(labelName), tpe, inner)
93+
}
94+
}
95+
}
96+
8097
private def freshName(base: String = "x"): String = {
8198
var suffix = 1
8299
var longName = base
@@ -97,6 +114,15 @@ trait JSEncoding extends SubComponent { self: GenJSCode =>
97114
private def localSymbolName(sym: Symbol): String =
98115
localSymbolNames.getOrElseUpdate(sym, freshName(sym.name.toString))
99116

117+
def getEnclosingReturnLabel()(implicit pos: ir.Position): js.Ident = {
118+
val box = returnLabelName.get
119+
if (box == null)
120+
throw new IllegalStateException(s"No enclosing returnable scope at $pos")
121+
if (box.value.isEmpty)
122+
box.value = Some(freshName("_return"))
123+
js.Ident(box.value.get)
124+
}
125+
100126
// Encoding methods ----------------------------------------------------------
101127

102128
def encodeLabelSym(sym: Symbol)(implicit pos: Position): js.Ident = {

‎ir/src/main/scala/org/scalajs/ir/Hashers.scala

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -120,7 +120,7 @@ object Hashers {
120120
case Return(expr, label) =>
121121
mixTag(TagReturn)
122122
mixTree(expr)
123-
mixOptIdent(label)
123+
mixIdent(label)
124124

125125
case If(cond, thenp, elsep) =>
126126
mixTag(TagIf)

‎ir/src/main/scala/org/scalajs/ir/Printers.scala

Lines changed: 4 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -164,15 +164,10 @@ object Printers {
164164
print(rhs)
165165

166166
case Return(expr, label) =>
167-
if (label.isEmpty) {
168-
print("return ")
169-
print(expr)
170-
} else {
171-
print("return(")
172-
print(label.get)
173-
print(") ")
174-
print(expr)
175-
}
167+
print("return@")
168+
print(label)
169+
print(" ")
170+
print(expr)
176171

177172
case If(cond, BooleanLiteral(true), elsep) =>
178173
print(cond)

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

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -155,7 +155,7 @@ object Serializers {
155155

156156
case Return(expr, label) =>
157157
writeByte(TagReturn)
158-
writeTree(expr); writeOptIdent(label)
158+
writeTree(expr); writeIdent(label)
159159

160160
case If(cond, thenp, elsep) =>
161161
writeByte(TagIf)
@@ -839,7 +839,7 @@ object Serializers {
839839
case TagBlock => Block(readTrees())
840840
case TagLabeled => Labeled(readIdent(), readType(), readTree())
841841
case TagAssign => Assign(readTree(), readTree())
842-
case TagReturn => Return(readTree(), readOptIdent())
842+
case TagReturn => Return(readTree(), readIdent())
843843
case TagIf => If(readTree(), readTree(), readTree())(readType())
844844
case TagWhile => While(readTree(), readTree())
845845
case TagDoWhile => DoWhile(readTree(), readTree())

‎ir/src/main/scala/org/scalajs/ir/Trees.scala

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -177,7 +177,7 @@ object Trees {
177177
val tpe = NoType // cannot be in expression position
178178
}
179179

180-
case class Return(expr: Tree, label: Option[Ident])(
180+
case class Return(expr: Tree, label: Ident)(
181181
implicit val pos: Position) extends Tree {
182182
val tpe = NothingType
183183
}

‎ir/src/test/scala/org/scalajs/ir/PrintersTest.scala

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -147,8 +147,7 @@ class PrintersTest {
147147
}
148148

149149
@Test def printReturn(): Unit = {
150-
assertPrintEquals("return 5", Return(i(5), None))
151-
assertPrintEquals("return(lab) 5", Return(i(5), Some("lab")))
150+
assertPrintEquals("return@lab 5", Return(i(5), "lab"))
152151
}
153152

154153
@Test def printIf(): Unit = {

‎linker/shared/src/main/scala/org/scalajs/linker/backend/emitter/FunctionEmitter.scala

Lines changed: 15 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -462,10 +462,6 @@ private[emitter] class FunctionEmitter(jsGen: JSGen) {
462462

463463
val env = env0.withParams(params)
464464

465-
val withReturn =
466-
if (isStat) body
467-
else Return(body, None)
468-
469465
val translateRestParam =
470466
if (esFeatures.useECMAScript2015) false
471467
else params.nonEmpty && params.last.rest
@@ -477,13 +473,17 @@ private[emitter] class FunctionEmitter(jsGen: JSGen) {
477473
val newParams =
478474
(if (translateRestParam) params.init else params).map(transformParamDef)
479475

480-
val newBody = transformStat(withReturn, Set.empty)(env) match {
476+
val newBody =
477+
if (isStat) transformStat(body, Set.empty)(env)
478+
else pushLhsInto(Lhs.ReturnFromFunction, body, Set.empty)(env)
479+
480+
val cleanedNewBody = newBody match {
481481
case js.Block(stats :+ js.Return(js.Undefined())) => js.Block(stats)
482482
case other => other
483483
}
484484

485485
js.Function(arrow && useArrowFunctions, newParams,
486-
js.Block(extractRestParam, newBody))
486+
js.Block(extractRestParam, cleanedNewBody))
487487
}
488488

489489
private def makeExtractRestParam(params: List[ParamDef])(
@@ -1455,9 +1455,9 @@ private[emitter] class FunctionEmitter(jsGen: JSGen) {
14551455
doVarDef(name, tpe, mutable, rhs)
14561456
case Lhs.Assign(lhs) =>
14571457
doAssign(lhs, rhs)
1458-
case Lhs.Return(None) =>
1458+
case Lhs.ReturnFromFunction =>
14591459
js.Return(transformExpr(rhs, env.expectedReturnType))
1460-
case Lhs.Return(Some(l)) =>
1460+
case Lhs.Return(l) =>
14611461
doReturnToLabel(l)
14621462
}
14631463

@@ -1485,10 +1485,10 @@ private[emitter] class FunctionEmitter(jsGen: JSGen) {
14851485
js.Block(varDef, assign)
14861486
}
14871487

1488-
case Lhs.Return(None) =>
1488+
case Lhs.ReturnFromFunction =>
14891489
throw new AssertionError("Cannot return a record value.")
14901490

1491-
case Lhs.Return(Some(l)) =>
1491+
case Lhs.Return(l) =>
14921492
doReturnToLabel(l)
14931493
}
14941494

@@ -2792,7 +2792,11 @@ private object FunctionEmitter {
27922792
case class Assign(lhs: Tree) extends Lhs
27932793
case class VarDef(name: Ident, tpe: Type, mutable: Boolean) extends Lhs
27942794

2795-
case class Return(label: Option[Ident]) extends Lhs {
2795+
case object ReturnFromFunction extends Lhs {
2796+
override def hasNothingType: Boolean = true
2797+
}
2798+
2799+
case class Return(label: Ident) extends Lhs {
27962800
override def hasNothingType: Boolean = true
27972801
}
27982802

‎linker/shared/src/main/scala/org/scalajs/linker/checker/IRChecker.scala

Lines changed: 4 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -710,7 +710,7 @@ private final class IRChecker(unit: LinkingUnit,
710710
typecheckExpect(body, env.withLabeledReturnType(label.name, tpe), tpe)
711711

712712
case Return(expr, label) =>
713-
env.returnTypes.get(label.map(_.name)).fold[Unit] {
713+
env.returnTypes.get(label.name).fold[Unit] {
714714
reportError(s"Cannot return to label $label.")
715715
typecheckExpr(expr, env)
716716
} { returnType =>
@@ -1277,7 +1277,7 @@ private final class IRChecker(unit: LinkingUnit,
12771277
/** Local variables in scope (including through closures). */
12781278
val locals: Map[String, LocalDef],
12791279
/** Return types by label. */
1280-
val returnTypes: Map[Option[String], Type],
1280+
val returnTypes: Map[String, Type],
12811281
/** Whether we're in a constructor of the class */
12821282
val inConstructor: Boolean
12831283
) {
@@ -1291,13 +1291,9 @@ private final class IRChecker(unit: LinkingUnit,
12911291
this.inConstructor)
12921292
}
12931293

1294-
def withReturnType(returnType: Type): Env =
1295-
new Env(this.thisTpe, this.locals,
1296-
returnTypes + (None -> returnType), this.inConstructor)
1297-
12981294
def withLabeledReturnType(label: String, returnType: Type): Env =
12991295
new Env(this.thisTpe, this.locals,
1300-
returnTypes + (Some(label) -> returnType), this.inConstructor)
1296+
returnTypes + (label -> returnType), this.inConstructor)
13011297

13021298
def withInConstructor(inConstructor: Boolean): Env =
13031299
new Env(this.thisTpe, this.locals, this.returnTypes, inConstructor)
@@ -1313,9 +1309,7 @@ private final class IRChecker(unit: LinkingUnit,
13131309
val paramLocalDefs =
13141310
for (p @ ParamDef(ident, tpe, mutable, _) <- allParams)
13151311
yield ident.name -> LocalDef(ident.name, tpe, mutable)(p.pos)
1316-
new Env(thisType, paramLocalDefs.toMap,
1317-
Map(None -> (if (resultType == NoType) AnyType else resultType)),
1318-
isConstructor)
1312+
new Env(thisType, paramLocalDefs.toMap, Map.empt 4622 y, isConstructor)
13191313
}
13201314
}
13211315

0 commit comments

Comments
 (0)
0