-
Notifications
You must be signed in to change notification settings - Fork 1.1k
Make inline vals effectively erased #8836
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -7,6 +7,7 @@ import DenotTransformers.SymTransformer | |
import Flags._ | ||
import SymDenotations._ | ||
import Symbols._ | ||
import Types._ | ||
import typer.RefChecks | ||
import MegaPhase.MiniPhase | ||
import ast.tpd | ||
|
@@ -34,19 +35,26 @@ class PruneErasedDefs extends MiniPhase with SymTransformer { thisTransform => | |
|
||
override def transformApply(tree: Apply)(implicit ctx: Context): Tree = | ||
if (tree.fun.tpe.widen.isErasedMethod) | ||
cpy.Apply(tree)(tree.fun, tree.args.map(arg => ref(defn.Predef_undefined))) | ||
cpy.Apply(tree)(tree.fun, tree.args.map(trivialErasedTree)) | ||
else tree | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. In PostTyper, we also have logic to erase arguments. Is there a duplication of code? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. We keep in TASTy the contents of the erased trees. These are erased here. In PostTyper we only remove contents of erased definitions that were generated by inlining code (i.e. needed for typing). I will recheck what happens if we don't remove them in PostTyper, but fur sure the TASTy will increase. |
||
|
||
override def transformValDef(tree: ValDef)(implicit ctx: Context): Tree = | ||
if (tree.symbol.isEffectivelyErased && !tree.rhs.isEmpty) | ||
cpy.ValDef(tree)(rhs = ref(defn.Predef_undefined)) | ||
cpy.ValDef(tree)(rhs = trivialErasedTree(tree)) | ||
else tree | ||
|
||
override def transformDefDef(tree: DefDef)(implicit ctx: Context): Tree = | ||
if (tree.symbol.isEffectivelyErased && !tree.rhs.isEmpty) | ||
cpy.DefDef(tree)(rhs = ref(defn.Predef_undefined)) | ||
cpy.DefDef(tree)(rhs = trivialErasedTree(tree)) | ||
else tree | ||
|
||
private def trivialErasedTree(tree: Tree)(using Context): Tree = | ||
tree.tpe.widenTermRefExpr.dealias match | ||
case ConstantType(c) => Literal(c) | ||
case _ => ref(defn.Predef_undefined) | ||
|
||
} | ||
|
||
object PruneErasedDefs { | ||
val name: String = "pruneErasedDefs" | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,47 @@ | ||
|
||
abstract class A: | ||
def x: Int | ||
val y: Int | ||
|
||
class B extends A: | ||
inline def x: Int = 1 | ||
inline val y = 2 | ||
|
||
class C extends A: | ||
final val x: Int = 3 | ||
final val y = 4 | ||
|
||
class D: | ||
inline def x: Int = 5 | ||
inline val y = 6 | ||
|
||
@main def Test = | ||
val b: B = new B | ||
assert(b.x == 1) | ||
assert(b.y == 2) | ||
|
||
val a: A = b | ||
assert(a.x == 1) | ||
assert(a.y == 2) | ||
|
||
val c: C = new C | ||
assert(c.x == 3) | ||
assert(c.y == 4) | ||
|
||
val a2: A = c | ||
assert(a2.x == 3) | ||
assert(a2.y == 4) | ||
|
||
val d: D = new D | ||
assert(d.x == 5) | ||
assert(d.y == 6) | ||
|
||
|
||
assert(classOf[B].getDeclaredMethods.size == 2) | ||
assert(classOf[B].getDeclaredFields.isEmpty) | ||
|
||
assert(classOf[C].getDeclaredMethods.size == 2) | ||
assert(classOf[C].getDeclaredFields.size == 1) | ||
|
||
assert(classOf[D].getDeclaredMethods.isEmpty) | ||
assert(classOf[D].getFields.isEmpty) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What we need the test
!hasAnnotation(defn.ScalaStaticAnnot)
?There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The
@static
annotation is supposed to guarantee that the field is emitted in the bytecode with the original signature. Hence we cannot remove the definition. There was one bytecode test with afinal val
that failed without this test.