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

Multiple fixes to @static #1226

Merged
merged 25 commits into from Jun 22, 2016
Merged

Multiple fixes to @static #1226

merged 25 commits into from Jun 22, 2016

Conversation

DarkDimius
Copy link
Member

@DarkDimius DarkDimius commented Apr 18, 2016

This PR fixes multiple issues in @static and makes lazy vals use them instead of fields in companion classes.

@DarkDimius
Copy link
Member Author

DarkDimius commented Apr 18, 2016

As a funny side effect of those changes:

import annotation.static 

class T
object T {
  @static val dummy: Unit = println("I am static initializer")
}

Compiles to code equivalent to:

public class T {
    public static BoxedUnit dummy;

    static {
        Predef..MODULE$.println((Object)"I am static initializer");
        dummy = BoxedUnit.UNIT;
    }
}

@DarkDimius
Copy link
Member Author

@lrytz, would you kindly take a look?

import annotation.static
object T {
@static val foo = 10 // error: needs companion class
}
Copy link
Member

Choose a reason for hiding this comment

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

don't you need a .check file for this test?

Copy link
Member Author

Choose a reason for hiding this comment

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

In dotty, our neg-tests test for presence of errors on lines containing // error:.
We don't check the error message.

Copy link
Member

Choose a reason for hiding this comment

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

cool

@DarkDimius
Copy link
Member Author

This pr revealed a bug in lazy vals. Fixing it.

override def transformStats(trees: List[Tree])(implicit ctx: Context, info: TransformerInfo): List[Tree] = {
if (ctx.owner.is(Flags.Package)) {
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
Member 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

@DarkDimius
Copy link
Member Author

I have several more fixes pending. This pr at the moment has problems with initialization of lazy vals.
This is why CI died...

They aren't inherited and can be entered into frozen owners.
As a funny side-effect this allows to execute arbitrary code in static
initialisers:

    @static val a: Unit = {println("loaded")}
It's not clear how they should be implemented.
There used to be a rare test when companion class and companion
object would have gotten the very same offset,
causing undefined behaviour in runtime.
Unlink the static from the old scope,
and don't drop top-level trees that are not TypeDefs.
Helps to spot usage of unsafe that would lead to undefined behaviour.
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.
This method is only used to find static initialisers.
Previously, it was always wrong, but we didn't care as we never had them.
GenBCode checks if class already has static initialiser,
the check is fooled if class inherited a static initialisers.
@DarkDimius
Copy link
Member Author

@odersky please review.

@odersky
Copy link
Contributor

odersky commented Jun 22, 2016

LGTM

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.

None yet

4 participants