Fix #3518: Do not generate calls to BoxesRunTime for Unit box/unbox #3522
Conversation
nicolasstucki
force-pushed
the
fix-#3518
branch
from
7de8e7c to
212b630
Compare
| val Apply(fun, args) = tree | ||
| if (fun.symbol == defn.cbnArg) | ||
| typedUnadapted(args.head, pt) | ||
| else if ((tree.symbol.name == nme.box || tree.symbol.name == nme.unbox) && tree.symbol.owner.asClass.companionClass == defn.UnitClass) ref(defn.BoxedUnit_UNIT) |
There was a problem hiding this comment.
That drops the side effects of args.head, unless I'm mistaken.
nicolasstucki
force-pushed
the
fix-#3518
branch
from
b1d38f6 to
c437512
Compare
| override def typedApply(tree: untpd.Apply, pt: Type)(implicit ctx: Context): Tree = { | ||
| val Apply(fun, args) = tree | ||
| if (fun.symbol == defn.cbnArg) | ||
| if (fun.symbol == defn.cbnArg || (tree.symbol.name == nme.box || tree.symbol.name == nme.unbox) && tree.symbol.owner.asClass.companionClass == defn.UnitClass) |
There was a problem hiding this comment.
I would factor out the newly added tests into an inner function, to make it clear what is tested: def isUnitBoxOrUnbox(fun: Symbol)
smarter
left a comment
smarter
left a comment
There was a problem hiding this comment.
I don't understand why these methods need to be erased. They have a legitimate implementation in https://github.com/scala/scala/blob/2.13.x/src/library/scala/Unit.scala, why is the backend exploding on them in the first place?
nicolasstucki
force-pushed
the
fix-#3518
branch
from
4ca7d8d to
749d9b4
Compare
nicolasstucki
dismissed
smarter’s stale review
Reimplemented fix. Now fix is in backend.
|
Fixes are in backend lampepfl/scala#30 |
nicolasstucki
force-pushed
the
fix-#3518
branch
from
749d9b4 to
40a25fd
Compare
nicolasstucki
changed the title
|
scalac also ignores |
|
@smarter now it does the same as in |
|
|
||
| def isBox(sym: Symbol): Boolean = Erasure.Boxing.isBox(sym) | ||
| def isUnbox(sym: Symbol): Boolean = Erasure.Boxing.isUnbox(sym) | ||
| def isBox(sym: Symbol): Boolean = Erasure.Boxing.isBox(sym) && !isInUnit(sym) |
There was a problem hiding this comment.
Why not change the definition of Erasure.Boxing.isBox directly?
There was a problem hiding this comment.
Erasure does transformations on Unit box inbox based on Boxing.isBox. If I add that condition directly there erasure starts failing.
| def isBox(sym: Symbol): Boolean = Erasure.Boxing.isBox(sym) && !isInUnit(sym) | ||
| def isUnbox(sym: Symbol): Boolean = Erasure.Boxing.isUnbox(sym) && !isInUnit(sym) | ||
|
|
||
| private def isInUnit(sym: Symbol) = sym.denot.owner.denot.linkedClass == defn.UnitClass |
There was a problem hiding this comment.
I would add a defn.UnitModule instead.
nicolasstucki
force-pushed
the
fix-#3518
branch
from
40a25fd to
85fb578
Compare
5 participants
No description provided.