Skip to content

Commit 2b1e4ef

Browse files
committed
SD-48 limit the lenght of inlined local variable names
When inlining local variables, the names are prefixed with the callee method name. In long chains of inlining, these names can grow indefinitely. This commits introduces a limit.
1 parent 3c43a7b commit 2b1e4ef

File tree

4 files changed

+68
-10
lines changed

4 files changed

+68
-10
lines changed

src/compiler/scala/tools/nsc/backend/jvm/BTypes.scala

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1164,4 +1164,7 @@ object BTypes {
11641164
// no static way (without symbol table instance) to get to nme.ScalaATTR / ScalaSignatureATTR
11651165
val ScalaAttributeName = "Scala"
11661166
val ScalaSigAttributeName = "ScalaSig"
1167+
1168+
// when inlining, local variable names of the callee are prefixed with the name of the callee method
1169+
val InlinedLocalVariablePrefixMaxLenght = 128
11671170
}

src/compiler/scala/tools/nsc/backend/jvm/opt/BytecodeUtils.scala

Lines changed: 27 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -324,15 +324,33 @@ object BytecodeUtils {
324324
* Clone the local variable descriptors of `methodNode` and map their `start` and `end` labels
325325
* according to the `labelMap`.
326326
*/
327-
def cloneLocalVariableNodes(methodNode: MethodNode, labelMap: Map[LabelNode, LabelNode], prefix: String, shift: Int): List[LocalVariableNode] = {
328-
methodNode.localVariables.iterator().asScala.map(localVariable => new LocalVariableNode(
329-
prefix + localVariable.name,
330-
localVariable.desc,
331-
localVariable.signature,
332-
labelMap(localVariable.start),
333-
labelMap(localVariable.end),
334-
localVariable.index + shift
335-
)).toList
327+
def cloneLocalVariableNodes(methodNode: MethodNode, labelMap: Map[LabelNode, LabelNode], calleeMethodName: String, shift: Int): List[LocalVariableNode] = {
328+
methodNode.localVariables.iterator().asScala.map(localVariable => {
329+
val name =
330+
if (calleeMethodName.length + localVariable.name.length < BTypes.InlinedLocalVariablePrefixMaxLenght) {
331+
calleeMethodName + "_" + localVariable.name
332+
} else {
333+
val parts = localVariable.name.split("_").toVector
334+
val (methNames, varName) = (calleeMethodName +: parts.init, parts.last)
335+
// keep at least 5 characters per method name
336+
val maxNumMethNames = BTypes.InlinedLocalVariablePrefixMaxLenght / 5
337+
val usedMethNames =
338+
if (methNames.length < maxNumMethNames) methNames
339+
else {
340+
val half = maxNumMethNames / 2
341+
methNames.take(half) ++ methNames.takeRight(half)
342+
}
343+
val charsPerMethod = BTypes.InlinedLocalVariablePrefixMaxLenght / usedMethNames.length
344+
usedMethNames.foldLeft("")((res, methName) => res + methName.take(charsPerMethod) + "_") + varName
345+
}
346+
new LocalVariableNode(
347+
name,
348+
localVariable.desc,
349+
localVariable.signature,
350+
labelMap(localVariable.start),
351+
labelMap(localVariable.end),
352+
localVariable.index + shift)
353+
}).toList
336354
}
337355

338356
/**

src/compiler/scala/tools/nsc/backend/jvm/opt/Inliner.scala

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -382,7 +382,7 @@ class Inliner[BT <: BTypes](val btypes: BT) {
382382
callsiteMethod.instructions.insert(callsiteInstruction, clonedInstructions)
383383
callsiteMethod.instructions.remove(callsiteInstruction)
384384

385-
callsiteMethod.localVariables.addAll(cloneLocalVariableNodes(callee, labelsMap, callee.name + "_", localVarShift).asJava)
385+
callsiteMethod.localVariables.addAll(cloneLocalVariableNodes(callee, labelsMap, callee.name, localVarShift).asJava)
386386
// prepend the handlers of the callee. the order of handlers matters: when an exception is thrown
387387
// at some instruction, the first handler guarding that instruction and having a matching exception
388388
// type is executed. prepending the callee's handlers makes sure to test those handlers first if

test/junit/scala/tools/nsc/backend/jvm/opt/InlinerTest.scala

Lines changed: 37 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1587,4 +1587,41 @@ class InlinerTest extends BytecodeTesting {
15871587
val List(c, t) = compile(code)
15881588
assertNoIndy(getMethod(c, "t1"))
15891589
}
1590+
1591+
@Test
1592+
def limitInlinedLocalVariableNames(): Unit = {
1593+
val code =
1594+
"""class C {
1595+
| def f(x: Int): Int = x
1596+
| @inline final def methodWithVeryVeryLongNameAlmostLikeAGermanWordOrAFrenchSentence(param: Int) =
1597+
| f(param)
1598+
| @inline final def anotherMethodWithVeryVeryLongNameAlmostLikeAGermanWordOrAFrenchSentence(param: Int) =
1599+
| methodWithVeryVeryLongNameAlmostLikeAGermanWordOrAFrenchSentence(f(param))
1600+
| @inline final def oneMoreMethodWithVeryVeryLongNameAlmostLikeAGermanWordOrAFrenchSentence(param: Int) =
1601+
| anotherMethodWithVeryVeryLongNameAlmostLikeAGermanWordOrAFrenchSentence(f(param))
1602+
| @inline final def yetAnotherMethodWithVeryVeryLongNameAlmostLikeAGermanWordOrAFrenchSentence(param: Int) =
1603+
| oneMoreMethodWithVeryVeryLongNameAlmostLikeAGermanWordOrAFrenchSentence(f(param))
1604+
| @inline final def oneLastMethodWithVeryVeryLongNameAlmostLikeAGermanWordOrAFrenchSentence(param: Int) =
1605+
| yetAnotherMethodWithVeryVeryLongNameAlmostLikeAGermanWordOrAFrenchSentence(f(param))
1606+
| def t(p: Int) =
1607+
| oneLastMethodWithVeryVeryLongNameAlmostLikeAGermanWordOrAFrenchSentence(f(p)) +
1608+
| oneLastMethodWithVeryVeryLongNameAlmostLikeAGermanWordOrAFrenchSentence(f(p))
1609+
|}
1610+
""".stripMargin
1611+
1612+
val List(c) = compile(code)
1613+
assertEquals(getAsmMethod(c, "t").localVariables.asScala.toList.map(l => (l.name, l.index)).sortBy(_._2),List(
1614+
("this",0),
1615+
("p",1),
1616+
("oneLastMethodWithVeryVeryLongNameAlmostLikeAGermanWordOrAFrenchSentence_param",2),
1617+
("oneLastMethodWithVeryVeryLongNameAlmostLikeAGermanWordOrAFrenchS_yetAnotherMethodWithVeryVeryLongNameAlmostLikeAGermanWordOrAFren_param",3),
1618+
("oneLastMethodWithVeryVeryLongNameAlmostLik_yetAnotherMethodWithVeryVeryLongNameAlmost_oneMoreMethodWithVeryVeryLongNameAlmostLik_param",4),
1619+
("oneLastMethodWithVeryVeryLongNam_yetAnotherMethodWithVeryVeryLong_oneMoreMethodWithVeryVeryLongNam_anotherMethodWithVeryVeryLongNam_param",5),
1620+
("oneLastMethodWithVeryVery_yetAnotherMethodWithVeryV_oneMoreMethodWithVeryVery_anotherMethodWithVeryVery_methodWithVeryVeryLongNam_param",6),
1621+
("oneLastMethodWithVeryVeryLongNameAlmostLikeAGermanWordOrAFrenchSentence_param",7),
1622+
("oneLastMethodWithVeryVeryLongNameAlmostLikeAGermanWordOrAFrenchS_yetAnotherMethodWithVeryVeryLongNameAlmostLikeAGermanWordOrAFren_param",8),
1623+
("oneLastMethodWithVeryVeryLongNameAlmostLik_yetAnotherMethodWithVeryVeryLongNameAlmost_oneMoreMethodWithVeryVeryLongNameAlmostLik_param",9),
1624+
("oneLastMethodWithVeryVeryLongNam_yetAnotherMethodWithVeryVeryLong_oneMoreMethodWithVeryVeryLongNam_anotherMethodWithVeryVeryLongNam_param",10),
1625+
("oneLastMethodWithVeryVery_yetAnotherMethodWithVeryV_oneMoreMethodWithVeryVery_anotherMethodWithVeryVery_methodWithVeryVeryLongNam_param",11)))
1626+
}
15901627
}

0 commit comments

Comments
 (0)