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
Implement @static sip. #1155
Implement @static sip. #1155
Changes from 4 commits
c73fbaa
976702a
65961a9
15f42a5
51dfcb8
ead3094
a068498
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 |
---|---|---|
|
@@ -285,6 +285,10 @@ object SymDenotations { | |
final def addAnnotation(annot: Annotation): Unit = | ||
annotations = annot :: myAnnotations | ||
|
||
/** Add given annotation to the annotations of this denotation */ | ||
final def addAnnotation(annot: ClassSymbol)(implicit ctx: Context): Unit = | ||
addAnnotation(ConcreteAnnotation(tpd.ref(annot))) | ||
|
||
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. I don't think you need that here. Annotation already contains a bunch of overloaded apply methods to create annotations; we should not add more overloadings in other places because then there are two places to look for the right variant instead of one. You can get what you want already by writing
If one objects to the 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.
seems like too much repetition to me, but if you insist, I'll make the change. |
||
/** Remove annotation with given class from this denotation */ | ||
final def removeAnnotation(cls: Symbol)(implicit ctx: Context): Unit = | ||
annotations = myAnnotations.filterNot(_ matches cls) | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,83 @@ | ||
package dotty.tools.dotc | ||
package transform | ||
|
||
import core._ | ||
import Names._ | ||
import StdNames.nme | ||
import Types._ | ||
import dotty.tools.dotc.transform.TreeTransforms.{AnnotationTransformer, TransformerInfo, MiniPhaseTransform, TreeTransformer} | ||
import ast.Trees._ | ||
import Flags._ | ||
import Contexts.Context | ||
import Symbols._ | ||
import Constants._ | ||
import Denotations._, SymDenotations._ | ||
import Decorators.StringInterpolators | ||
import dotty.tools.dotc.ast.tpd | ||
import dotty.tools.dotc.core.Annotations.ConcreteAnnotation | ||
import scala.collection.mutable | ||
import DenotTransformers._ | ||
import Names.Name | ||
import NameOps._ | ||
import Decorators._ | ||
import TypeUtils._ | ||
|
||
/** A transformer that check that requirements of Static fields\methods are implemented: | ||
* 1. Only objects can have members annotated with `@static` | ||
* 2. The fields annotated with `@static` should preceed any non-`@static` fields. | ||
* This ensures that we do not introduce surprises for users in initialization order. | ||
* 3. If a member `foo` of an `object C` is annotated with `@static`, | ||
* the companion class `C` is not allowed to define term members with name `foo`. | ||
* 4. If a member `foo` of an `object C` is annotated with `@static`, the companion class `C` | ||
* is not allowed to inherit classes that define a term member with name `foo`. | ||
* 5. Only `@static` methods and vals are supported in companions of traits. | ||
* Java8 supports those, but not vars, and JavaScript does not have interfaces at all. | ||
*/ | ||
class CheckStatic extends MiniPhaseTransform { thisTransformer => | ||
import ast.tpd._ | ||
|
||
override def phaseName = "elimRepeated" | ||
|
||
|
||
def check(tree: tpd.DefTree)(implicit ctx: Context) = { | ||
|
||
} | ||
|
||
override def transformTemplate(tree: tpd.Template)(implicit ctx: Context, info: TransformerInfo): tpd.Tree = { | ||
val defns = tree.body.collect{case t: ValOrDefDef => t} | ||
var hadNonStaticField = false | ||
for(defn <- defns) { | ||
if (defn.symbol.hasAnnotation(ctx.definitions.ScalaStaticAnnot)) { | ||
if(!ctx.owner.is(Module)) { | ||
ctx.error("@static fields are only allowed inside objects", defn.pos) | ||
} | ||
|
||
if (defn.isInstanceOf[ValDef] && hadNonStaticField) { | ||
ctx.error("@static fields should preceed non-static ones", defn.pos) | ||
} | ||
|
||
val companion = ctx.owner.companionClass | ||
if (!companion.exists) { | ||
ctx.error("object that conatin @static members should have companion class", defn.pos) | ||
} | ||
|
||
val clashes = companion.asClass.membersNamed(defn.name) | ||
if (clashes.exists) { | ||
ctx.error("companion classes cannot define members with same name as @static member", defn.pos) | ||
} | ||
|
||
if (defn.symbol.is(Flags.Mutable) && companion.is(Flags.Trait)) { | ||
ctx.error("Companions of traits cannot define mutable @static fields") | ||
} | ||
} else hadNonStaticField = hadNonStaticField || defn.isInstanceOf[ValDef] | ||
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. Drop { ... } around single-statement blocks? |
||
|
||
} | ||
tree | ||
} | ||
|
||
override def transformSelect(tree: tpd.Select)(implicit ctx: Context, info: TransformerInfo): tpd.Tree = { | ||
if (tree.symbol.hasAnnotation(defn.ScalaStaticAnnot)) | ||
ref(tree.symbol) | ||
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. This will silently drop any side effect of evaluating the qualifier. If the qualifier is a simple reference to the enclosing object, that's fine (that's part of the semantic changes of 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. fix incoming |
||
else tree | ||
} | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -91,6 +91,7 @@ class LazyVals extends MiniPhaseTransform with IdentityDenotTransformer with Nee | |
appendOffsetDefs.get(cls) match { | ||
case None => template | ||
case Some(data) => | ||
data.defs.foreach(_.symbol.addAnnotation(defn.ScalaStaticAnnot)) | ||
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. To avoid the repetition, why not define in LazyVals:
|
||
cpy.Template(template)(body = addInFront(data.defs, template.body)) | ||
} | ||
|
||
|
@@ -357,6 +358,7 @@ class LazyVals extends MiniPhaseTransform with IdentityDenotTransformer with Nee | |
.symbol.asTerm | ||
} else { // need to create a new flag | ||
offsetSymbol = ctx.newSymbol(companion.moduleClass, (StdNames.nme.LAZY_FIELD_OFFSET + id.toString).toTermName, Flags.Synthetic, defn.LongType).enteredAfter(this) | ||
offsetSymbol.addAnnotation(defn.ScalaStaticAnnot) | ||
val flagName = (StdNames.nme.BITMAP_PREFIX + id.toString).toTermName | ||
val flagSymbol = ctx.newSymbol(claz, flagName, containerFlags, defn.LongType).enteredAfter(this) | ||
flag = ValDef(flagSymbol, Literal(Constants.Constant(0L))) | ||
|
@@ -366,6 +368,7 @@ class LazyVals extends MiniPhaseTransform with IdentityDenotTransformer with Nee | |
|
||
case None => | ||
offsetSymbol = ctx.newSymbol(companion.moduleClass, (StdNames.nme.LAZY_FIELD_OFFSET + "0").toTermName, Flags.Synthetic, defn.LongType).enteredAfter(this) | ||
offsetSymbol.addAnnotation(defn.ScalaStaticAnnot) | ||
val flagName = (StdNames.nme.BITMAP_PREFIX + "0").toTermName | ||
val flagSymbol = ctx.newSymbol(claz, flagName, containerFlags, defn.LongType).enteredAfter(this) | ||
flag = ValDef(flagSymbol, Literal(Constants.Constant(0L))) | ||
|
@@ -375,9 +378,10 @@ class LazyVals extends MiniPhaseTransform with IdentityDenotTransformer with Nee | |
|
||
val containerName = ctx.freshName(x.name.asTermName.lazyLocalName).toTermName | ||
val containerSymbol = ctx.newSymbol(claz, containerName, (x.mods &~ containerFlagsMask | containerFlags).flags, tpe, coord = x.symbol.coord).enteredAfter(this) | ||
|
||
val containerTree = ValDef(containerSymbol, defaultValue(tpe)) | ||
|
||
val offset = ref(companion).ensureApplied.select(offsetSymbol) | ||
val offset = ref(offsetSymbol) | ||
val getFlag = Select(ref(helperModule), lazyNme.RLazyVals.get) | ||
val setFlag = Select(ref(helperModule), lazyNme.RLazyVals.setFlag) | ||
val wait = Select(ref(helperModule), lazyNme.RLazyVals.wait4Notification) | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,13 @@ | ||
package scala.annotation | ||
|
||
import scala.annotation.meta._ | ||
|
||
/** https://github.com/scala/scala.github.com/pull/491 */ | ||
|
||
@field | ||
@getter | ||
@beanGetter | ||
@beanSetter | ||
@param | ||
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. Why 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. For consistency. Now if any kind of member is created from a 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. But parameters cannot be @static? 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. Not if written by user. But I would like to leave the door open for tools\optimizers\analyzers. 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. I guess the question is: what is a static param, even if produced by a tool/optimizer/analyzer? I don't have any idea what that would look like in the bytecode. 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. Simply a static field. 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. Then it's covered by |
||
@setter | ||
final class static extends StaticAnnotation |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,40 @@ | ||
import scala.annotation.static | ||
|
||
class Foo{ | ||
class Bar { | ||
def qwa = | ||
Bar.field | ||
// 0: invokestatic #31 // Method Foo$Bar$.field:()I | ||
// 3: ireturn | ||
} | ||
object Bar { | ||
@static | ||
val field = 1 | ||
} | ||
} | ||
|
||
object Foo{ | ||
@static | ||
def method = 1 | ||
|
||
@static | ||
val field = 2 | ||
|
||
@static | ||
var mutable = 3 | ||
|
||
@static | ||
def accessor = field | ||
} | ||
|
||
object Test { | ||
import Foo._ | ||
def main(args: Array[String]): Unit = { | ||
method + field + mutable + accessor | ||
} | ||
} | ||
|
||
class WithLazies{ | ||
@volatile lazy val s = 1 | ||
// 98: getstatic #30 // Field WithLazies$.OFFSET$0:J | ||
} |
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.
Indentation issue?