8000 Multiple fixes to @static by DarkDimius · Pull Request #1226 · scala/scala3 · GitHub
[go: up one dir, main page]

Skip to content

Multiple fixes to @static #1226

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

Merged
merged 25 commits into from
Jun 22, 2016
Merged
Changes from 1 commit
Commits
Show all changes
25 commits
Select commit Hold shift + click to select a range
2e5ab54
Fix #1220. Dont die when having incorect static methods
DarkDimius Apr 18, 2016
7408f14
Test #1220
DarkDimius Apr 18, 2016
bbd48d3
SymDenotations: Allow entering Static symbols.
DarkDimius Apr 18, 2016
806a029
Constructors: do not lift static val initialisation into constructors.
DarkDimius Apr 18, 2016
db6a07d
Getters: do not generate getters for static vals
DarkDimius Apr 18, 2016
4ce8ab0
Allow creating static initialisers.
DarkDimius Apr 18, 2016
27846bb
MoveStatic: Move static methods & fields into companion class
DarkDimius Apr 18, 2016
63eb88d
Drop support for @static lazy vals.
DarkDimius Apr 18, 2016
ce2b964
Fix type in SymDenotations.
DarkDimius Apr 18, 2016
7cacd0f
Fix #1224: static members do not override\implement parent symbols.
DarkDimius Apr 18, 2016
1932b17
Test #1224.
DarkDimius Apr 18, 2016
f650b1e
LazyVals: do not share offsets between companions.
DarkDimius Apr 20, 2016
61fe99b
MoveStatics: fix two bugs.
DarkDimius Apr 20, 2016
d9702d2
Fix Ycheck: allow assigning fields in static constructors.
DarkDimius Apr 20, 2016
e1fcb4c
LazyVals: support debug mode.
DarkDimius Apr 20, 2016
990e962
SymDenotations: fix comment.
DarkDimius Apr 20, 2016
f2cfac5
MoveStatics: survive absence of companions.
DarkDimius Apr 20, 2016 8000
fa6deee
DottyBackendInterface: fix a bug in methodSymbols.
DarkDimius Apr 20, 2016
5f73175
Add tests that were used to reproduce issues with LazyVals.
DarkDimius Apr 20, 2016
49ace48
MoveStatics: fix a bug.
DarkDimius Apr 20, 2016
2c6a9be
CheckStatic: report error position in case of disallowed override
DarkDimius Apr 20, 2016
9899a06
LazyVals: fix leftover moduleClass usage.
DarkDimius Apr 20, 2016
c428e74
LazyVals: do even more verbose debugging.
DarkDimius Jun 7, 2016
de45fa1
MoveStatics: Fix classes without companion not getting static <clinit>
DarkDimius Jun 7, 2016
3c93c5c
Make class initialisers private. Otherwise they break GenBCode.
DarkDimius Jun 7, 2016
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Prev Previous commit
Next Next commit
MoveStatics: survive absence of companions.
Now moveStatics can correctly create static  constructors for objects.
Those static constructors would later be merged with synthetic module
initialisers by GenBCode. This is a bit of magic, it would be good
to move all this into this phase.
  • Loading branch information
DarkDimius committed Jun 7, 2016
commit f2cfac5771b1b19c733e7be9b3d05dae411bc9e3
38 changes: 24 additions & 14 deletions src/dotty/tools/dotc/transform/MoveStatics.scala
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
package dotty.tools.dotc.transform

import dotty.tools.dotc.ast.{Trees, tpd}
import dotty.tools.dotc.core.Annotations.Annotation
import dotty.tools.dotc.core.Contexts.Context
import dotty.tools.dotc.core.DenotTransformers.{InfoTransformer, SymTransformer}
import dotty.tools.dotc.core.SymDenotations.SymDenotation
Expand All @@ -20,7 +21,7 @@ class MoveStatics extends MiniPhaseTransform with SymTransformer { thisTransform


def transformSym(sym: SymDenotation)(implicit ctx: Context): SymDenotation = {
if (sym.hasAnnotation(defn.ScalaStaticAnnot) && sym.owner.is(Flags.Module)) {
if (sym.hasAnnotation(defn.ScalaStaticAnnot) && sym.owner.is(Flags.Module) && sym.owner.companionClass.exists) {
sym.owner.asClass.delete(sym.symbol)
sym.owner.companionClass.asClass.enter(sym.symbol)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

do you need to remove / unlink the symbol from the module's scope?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm kind of intentionally not doing so.
To make sure it can be found in both places.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ok, i don't know if that's a good idea (literally - maybe it is..)

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Note here we're already near the backend, so I'm preparing the tree specifically for GenBCode.
Phases that go in this block are backend-specific and break some assumptions of compiler.
Eg LabelDefs reorders <label> defs in a magical order that no one should touch.

Copy link
Member
@lrytz lrytz Apr 19, 2016

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

maybe it's related to exactly that discussion: the current PR creates a static field in both T and T$

import annotation.static
class T
object T {
  @static val x = 99
}
public class T {
  public static I x
}
public final class T$ {
  public static I x
}

the one in the module class is not initialized.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

magical

✨ :)

val flags = if (sym.is(Flags.Method)) sym.flags else sym.flags | Flags.Mutable
Expand All @@ -34,32 +35,41 @@ class MoveStatics extends MiniPhaseTransform with SymTransformer { thisTransform
val (classes, others) = trees.partition(x => x.isInstanceOf[TypeDef] && x.symbol.isClass)
val pairs = classes.groupBy(_.symbol.name.stripModuleClassSuffix).asInstanceOf[Map[Name, List[TypeDef]]]
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

why do you need the cast?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Static type is Map[Name, List[Tree]].

Copy link
Member
@lrytz lrytz Apr 19, 2016

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

oh i see now -- since you don't use others, you can write val classes = trees collect { case td: TypeDef if td.symbol.isClass => td } and don't need the cast


def move(companion: TypeDef, module: TypeDef): Thicket = {
if (companion.symbol.is(Flags.Module)) move(module, companion)
def move(module: TypeDef, companion: TypeDef): List[Tree] = {
if (!module.symbol.is(Flags.Module)) move(companion, module)
else {
val allMembers = companion.rhs.asInstanceOf[Template].body ++ module.rhs.asInstanceOf[Template].body
val (newCompanionBody, newModuleBody) = allMembers.partition(x => {assert(x.symbol.exists); x.symbol.owner == companion.symbol})
def rebuild(orig: TypeDef, newBody: List[Tree]) = {
val oldTemplate = orig.rhs.asInstanceOf[Template]
val statics = newBody.filter(x => x.isInstanceOf[ValDef] && x.symbol.hasAnnotation(defn.ScalaStaticAnnot)).asInstanceOf[List[ValDef]]
val allMembers =
if(companion ne null) {companion.rhs.asInstanceOf[Template].body} else Nil ++
module.rhs.asInstanceOf[Template].body
val (newModuleBody, newCompanionBody) = allMembers.partition(x => {assert(x.symbol.exists); x.symbol.owner == module.symbol})
def rebuild(orig: TypeDef, newBody: List[Tree]): Tree = {
if (orig eq null) return EmptyTree

val staticFields = newBody.filter(x => x.isInstanceOf[ValDef] && x.symbol.hasAnnotation(defn.ScalaStaticAnnot)).asInstanceOf[List[ValDef]]
val newBodyWithStaticConstr =
if (statics.nonEmpty) {
val staticCostructor = ctx.newSymbol(orig.symbol, Names.STATIC_CONSTRUCTOR, Flags.Synthetic | Flags.JavaStatic | Flags.Method, MethodType(Nil, defn.UnitType))
val staticAssigns = statics.map(x => Assign(ref(x.symbol), x.rhs.changeOwner(x.symbol, staticCostructor)))
if (staticFields.nonEmpty) {
val staticCostructor = ctx.newSymbol(orig.symbol, Names.STATIC_CONSTRUCTOR, Flags.Synthetic | Flags.Method, MethodType(Nil, defn.UnitType))
staticCostructor.addAnnotation(Annotation(defn.ScalaStaticAnnot))
staticCostructor.entered

val staticAssigns = staticFields.map(x => Assign(ref(x.symbol), x.rhs.changeOwner(x.symbol, staticCostructor)))
tpd.DefDef(staticCostructor, Block(staticAssigns, tpd.unitLiteral)) :: newBody
} else newBody

val oldTemplate = orig.rhs.asInstanceOf[Template]
cpy.TypeDef(orig)(rhs = cpy.Template(orig.rhs)(oldTemplate.constr, oldTemplate.parents, oldTemplate.self, newBodyWithStaticConstr))
}
Thicket(rebuild(companion, newCompanionBody), rebuild(module, newModuleBody))
Trees.flatten(rebuild(companion, newCompanionBody) :: rebuild(module, newModuleBody) :: Nil)
}
}
val newPairs =
for ((name, classes) <- pairs)
yield
if (classes.tail.isEmpty) classes.head
if (classes.tail.isEmpty)
if (classes.head.symbol.is(Flags.Module)) move(classes.head, null)
else List(classes.head)
else move(classes.head, classes.tail.head)
Trees.flatten(newPairs.toList ++ others)
Trees.flatten(newPairs.toList.flatten ++ others)
} else trees
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

maybe invert the condition and move this case up -- makes it easier to follow

}
}
0