Skip to content
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

Merged
merged 7 commits into from Mar 9, 2016
Merged

Implement @static sip. #1155

merged 7 commits into from Mar 9, 2016

Conversation

DarkDimius
Copy link
Member

This pull request implements most of machinery needed for
scala/docs.scala-lang#491

Only 3-rd check is not implemented by this commit. I propose to get this in
faster to fix #1149

This pull request implements most of machinery needed for
scala/docs.scala-lang#491

Only 3-rd check is not implemented by this commit.
I propose to get this in faster to fix scala#1149
@@ -337,6 +337,8 @@ object tpd extends Trees.Instance[Type] with TypedTreeInfo {
else if (prefixIsElidable(tp)) Ident(tp)
else if (tp.symbol.is(Module) && ctx.owner.isContainedIn(tp.symbol.moduleClass))
followOuterLinks(This(tp.symbol.moduleClass.asClass))
else if (tp.symbol hasAnnotation defn.ScalaStaticAnnot)
Ident(tp)
Copy link
Member

Choose a reason for hiding this comment

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

Indentation issue?

/** Add given annotation to the annotations of this denotation */
final def addAnnotation(annot: ClassSymbol)(implicit ctx: Context): Unit =
addAnnotation(ConcreteAnnotation(tpd.ref(annot)))

Copy link
Contributor

Choose a reason for hiding this comment

The 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

addAnnotation(Annotation(cls, Nil))

If one objects to the Nil, one can add another variant of apply to Annotation.

Copy link
Member Author

Choose a reason for hiding this comment

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

symbol.addAnnotation(Annotation(defn.ScalaStaticAnnot)))

seems like too much repetition to me, but if you insist, I'll make the change.

Implemented by checking that tree is allowed to access the static member
and all the members on the path to it. Needed as typer has a tendency
to desugar calls into series of selections&calls to This.
@@ -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))
Copy link
Contributor

Choose a reason for hiding this comment

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

To avoid the repetition, why not define in LazyVals:

def staticAnnot = Annotation(defn.ScalaStaticAnnot, Nil)

@odersky
Copy link
Contributor

odersky commented Mar 9, 2016

LGTM

odersky added a commit that referenced this pull request Mar 9, 2016
@odersky odersky merged commit bdd8c35 into scala:master Mar 9, 2016
@allanrenucci allanrenucci deleted the static branch December 14, 2017 19:24
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Locks for volatile lazy vals should be static
5 participants