SI-9571 Avoid boxing primitives in string concatenation#4944
SI-9571 Avoid boxing primitives in string concatenation#4944lrytz merged 2 commits intoscala:2.12.xfrom
Conversation
|
@melezov thanks a lot for your patch, very nice work! I refined it a bit and added some tests. Review by @soc. For the record, @melezov signed the CLA. |
8a0a410 to
89343e9
Compare
|
Maybe we should rename |
|
Apart from that, LGTM. |
89343e9 to
96283ed
Compare
|
Thanks for the review, @soc - with fresh eyes I managed to get rid of some duplicate code, now things look better. |
96283ed to
bea2860
Compare
| case ct: ClassBType if ct.isSubtypeOf(jlCharSequenceRef).get => jlCharSequenceRef | ||
| case rt: RefBType => ObjectRef | ||
| case pt: PrimitiveBType => pt // Currently this ends up being boxed in erasure | ||
| // Don't match for `ArrayBType(CHAR)`, even though StringBuilder has an such an overload |
There was a problem hiding this comment.
sorry 😓 really minor "has an such an overload" -> "has such an overload"
bea2860 to
cb68d9c
Compare
|
@soc does this LGTY now? |
|
LGTM! |
| case Apply(boxOp, value :: Nil) if currentRun.runDefinitions.isBox(boxOp.symbol) => | ||
| // Eliminate boxing of primitive values. Boxing is introduced by erasure because | ||
| // there's only a single synthetic `+` method "added" to the string class. | ||
| value |
There was a problem hiding this comment.
This will incorrectly transform:
"xx" + {println("side effect"); Int}.box(1)
to
val sb = new StringBuilder(); sb.append("xx"); sb.append(1)
However, this isn't the only place in the backend that has this bug:
scala> {???; Int}.box(1)
res4: Integer = 1
We can treat this as a separate bug: https://github.com/scala/scala-dev/issues/85
SI-9571 Avoid boxing primitives in string concatenation
No description provided.