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

Add @static SIP #491

Merged
merged 4 commits into from Nov 25, 2016

Conversation

Projects
None yet
@DarkDimius
Member

DarkDimius commented Jan 18, 2016

Add @static SIP

Show outdated Hide outdated sips/pending/_posts/2016-01-11-static-members.md
{% endhighlight %}
```
## Comparisson with mirror classes ##

This comment has been minimized.

@dwijnand

dwijnand Jan 18, 2016

Member

Comparison

@dwijnand

dwijnand Jan 18, 2016

Member

Comparison

DarkDimius added a commit to dotty-staging/dotty that referenced this pull request Mar 7, 2016

Implement @static sip.
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

DarkDimius added a commit to dotty-staging/dotty that referenced this pull request Mar 7, 2016

Implement @static sip.
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

@DarkDimius DarkDimius referenced this pull request Mar 7, 2016

Merged

Implement @static sip. #1155

@DarkDimius

This comment has been minimized.

Show comment
Hide comment
@DarkDimius

DarkDimius Mar 9, 2016

Member

The implementation was merged into Dotty.

Member

DarkDimius commented Mar 9, 2016

The implementation was merged into Dotty.

Show outdated Hide outdated sips/pending/_posts/2016-01-11-static-members.md
For example, classes extending `android.os.Parcelable` are required to have a static field named `CREATOR` of type `android.os.Parcelable$Creator`.
Another example is using an [`AtomicReferenceFieldUpdater`](http://docs.oracle.com/javase/7/docs/api/java/util/concurrent/atomic/AtomicReferenceFieldUpdater.html).

This comment has been minimized.

@janekdb

janekdb Apr 20, 2016

Member

s/7/8/ ?

@janekdb

janekdb Apr 20, 2016

Member

s/7/8/ ?

Show outdated Hide outdated sips/pending/_posts/2016-01-11-static-members.md
## Overview ##
In order for method or field to be considered static it needs to be defined in an `object` and annotated `@static`.

This comment has been minimized.

@janekdb

janekdb Apr 20, 2016

Member

for method or field -> for a method or field.

@janekdb

janekdb Apr 20, 2016

Member

for method or field -> for a method or field.

Show outdated Hide outdated sips/pending/_posts/2016-01-11-static-members.md
{% endhighlight %}
```
Intuively, the presence of the `@static` annotation ensures that a field/method is declared as a static member of the companion class.

This comment has been minimized.

@janekdb

janekdb Apr 20, 2016

Member

Intuitively

@janekdb

janekdb Apr 20, 2016

Member

Intuitively

Show outdated Hide outdated sips/pending/_posts/2016-01-11-static-members.md
## Restrictions ##
The following rules ensure that method can be correctly compiled into static member on both JVM and JavaScript:

This comment has been minimized.

@janekdb

janekdb Apr 20, 2016

Member

... that method can be ... into static member ... -> ... that methods can be ... into static members ...

@janekdb

janekdb Apr 20, 2016

Member

... that method can be ... into static member ... -> ... that methods can be ... into static members ...

Show outdated Hide outdated sips/pending/_posts/2016-01-11-static-members.md
- extend `LambdaLift` to trigger an error if a method annotated with `@static` method cannot be lifted to the top level scope;
- extend `GenBCode` to emit static fields and methods in companion classes and forwarders to them in companion modules.
## Overriding&Hiding ##

This comment has been minimized.

@janekdb

janekdb Apr 20, 2016

Member

Overriding&Hiding -> Overriding & Hiding ?

@janekdb

janekdb Apr 20, 2016

Member

Overriding&Hiding -> Overriding & Hiding ?

Show outdated Hide outdated sips/pending/_posts/2016-01-11-static-members.md
This proposal does not need to introduce this notion as we do not support such calls.
## Comparison with [@lrytz's proposal](https://gist.github.com/lrytz/80f3141de8240f9629da) ##
Lucas Rytz has proposed a similar SIP, but his SIP requires changes to the typer to ensure that `@static` fields do not capture `this`, as in his proposal `@static` fields are defined in the class, rather than its companion object.

This comment has been minimized.

@janekdb

janekdb Apr 20, 2016

Member

Lukas

@janekdb

janekdb Apr 20, 2016

Member

Lukas

## See Also ##
* [SI-4581](https://issues.scala-lang.org/browse/SI-4581) is a request for a `@static` annotation
* [Scala.js issue #1902](https://github.com/scala-js/scala-js/issues/1902) is a request for defining static fields in Scala.js-defined JS classes
* [Another proposal by @lrytz](https://gist.github.com/lrytz/80f3141de8240f9629da)

This comment has been minimized.

@janekdb

janekdb Apr 20, 2016

Member

Intentionally repeated?

@janekdb

janekdb Apr 20, 2016

Member

Intentionally repeated?

This comment has been minimized.

@DarkDimius

DarkDimius Apr 20, 2016

Member

Yes.

@DarkDimius
{% endhighlight %}
```
Under the proposed scheme users will be able to opt-in to have the field `f` defined in the inner object `I` emited as a static field.

This comment has been minimized.

@lrytz

lrytz Apr 20, 2016

Member

maybe mention that you need a companion inner class I.

@lrytz

lrytz Apr 20, 2016

Member

maybe mention that you need a companion inner class I.

This comment has been minimized.

@DarkDimius

DarkDimius Apr 20, 2016

Member

The current implementation, if companion is missing, is going to generate static field in the O.I$.
This is used internally to implement lazy vals since lampepfl/dotty#1226.
I'm not sure if I want to spec this, as this is subject to change.

@DarkDimius

DarkDimius Apr 20, 2016

Member

The current implementation, if companion is missing, is going to generate static field in the O.I$.
This is used internally to implement lazy vals since lampepfl/dotty#1226.
I'm not sure if I want to spec this, as this is subject to change.

Show outdated Hide outdated sips/pending/_posts/2016-01-11-static-members.md
```
Under the proposed scheme users will be able to opt-in to have the field `f` defined in the inner object `I` emited as a static field.
In case `O.d` is annotated with `@static` the field will be crated as a static field in `class O`.

This comment has been minimized.

@lrytz

lrytz Apr 20, 2016

Member

this is a bit confusing given the phrase above, it sounds like "the field" refers to I.f

@lrytz

lrytz Apr 20, 2016

Member

this is a bit confusing given the phrase above, it sounds like "the field" refers to I.f

@kjsingh

This comment has been minimized.

Show comment
Hide comment
@kjsingh

kjsingh Nov 24, 2016

why can't we have static as a keyword?

kjsingh commented Nov 24, 2016

why can't we have static as a keyword?

@sjrd

This comment has been minimized.

Show comment
Hide comment
@sjrd

sjrd Nov 24, 2016

Member

Because adding new keywords breaks existing code that used them as identifiers. Adding keywords must be done very sparingly, only when there is no other option.

Member

sjrd commented Nov 24, 2016

Because adding new keywords breaks existing code that used them as identifiers. Adding keywords must be done very sparingly, only when there is no other option.

@kjsingh

This comment has been minimized.

Show comment
Hide comment
@kjsingh

kjsingh Nov 24, 2016

I guess libs doing interop with Java code will be avoiding that.

kjsingh commented Nov 24, 2016

I guess libs doing interop with Java code will be avoiding that.

@jvican

This comment has been minimized.

Show comment
Hide comment
@jvican

jvican Nov 24, 2016

Member

@DarkDimius is there any change you want to do here before the SIP review?

Member

jvican commented Nov 24, 2016

@DarkDimius is there any change you want to do here before the SIP review?

@DarkDimius

This comment has been minimized.

Show comment
Hide comment
@DarkDimius

DarkDimius Nov 24, 2016

Member

I guess libs doing interop with Java code will be avoiding that.

Why would they? You may want some data to be seen as Java static field, and you can use @static for this.

Member

DarkDimius commented Nov 24, 2016

I guess libs doing interop with Java code will be avoiding that.

Why would they? You may want some data to be seen as Java static field, and you can use @static for this.

@DarkDimius

This comment has been minimized.

Show comment
Hide comment
@DarkDimius

DarkDimius Nov 25, 2016

Member

@DarkDimius is there any change you want to do here before the SIP review?

@jvican, I've applied those changes now and cleared git history. Feel free to merge if it looks good.

Member

DarkDimius commented Nov 25, 2016

@DarkDimius is there any change you want to do here before the SIP review?

@jvican, I've applied those changes now and cleared git history. Feel free to merge if it looks good.

@jvican

This comment has been minimized.

Show comment
Hide comment
@jvican

jvican Nov 25, 2016

Member

Great changes, let's merge it for next week's discussion.

Member

jvican commented Nov 25, 2016

Great changes, let's merge it for next week's discussion.

@jvican jvican merged commit 53f10e5 into scala:master Nov 25, 2016

1 check passed

continuous-integration/travis-ci/pr The Travis CI build passed
Details
@xuwei-k

This comment has been minimized.

Show comment
Hide comment
@xuwei-k

xuwei-k Nov 28, 2016

Contributor
Contributor

xuwei-k commented Nov 28, 2016

@adriaanm

This comment has been minimized.

Show comment
Hide comment
@adriaanm

adriaanm Nov 29, 2016

Member
  • I don't think it's a good idea to let code gen depend in non-obvious ways on an annotation. Can we break this down further? How about....
    • Always emit object members as class statics, as suggested by Martin (solves initialization mismatch that killed scala/scala#894, and keeps static members out of class, which would be problematic in our type system)
    • Deal with name clashes using an @exportedName annotation?
    • Allow emitting a public static field without accessor using @suppressAccessors?

The proposal should go into more detail on:

  • What about binary compat (adding/removing @static annotation)
  • Interaction with initialization semantics?

Some relevant bugs:

Member

adriaanm commented Nov 29, 2016

  • I don't think it's a good idea to let code gen depend in non-obvious ways on an annotation. Can we break this down further? How about....
    • Always emit object members as class statics, as suggested by Martin (solves initialization mismatch that killed scala/scala#894, and keeps static members out of class, which would be problematic in our type system)
    • Deal with name clashes using an @exportedName annotation?
    • Allow emitting a public static field without accessor using @suppressAccessors?

The proposal should go into more detail on:

  • What about binary compat (adding/removing @static annotation)
  • Interaction with initialization semantics?

Some relevant bugs:

@retronym

This comment has been minimized.

Show comment
Hide comment
@retronym

retronym Feb 15, 2017

Member

I'd like to see the "Compilation scheme" section fleshed out. Currently, it talks about how this would fit into the compiler pipeline, but lacks details about how things would be compiled to bytecode.

Here's an example I'm interested in:

package p1;  object C { @static val foo: AnyRef = C; val bar: AnyRef = C }

If I translate this according to the intuitive description herein, taking a few guesses at the details, I start with:

public class Test {
  public static void main(String[] args) {
    System.out.println(C$.MODULE$.foo()); // null
  }
}

class C {
  public static final Object foo;
  static {
    foo = C$.MODULE$; // initializer moved to CompanionClass.<clinit>
  }
}
final class C$ {
  public static C$ MODULE$;
  private final Object bar;
  public static Object foo() { return C.foo; }
  static {
     Object x = C.foo; // or UNSAFE.ensureClassInitialized(classOf[C])
     new C$();
  }
  private C$() {
    super();
    MODULE$ = this;
    bar = this;
  }
}

Running this prints null. I guess we should move the call to C.<init> after the super constructor call and the assignment of the module var:

final class C$ {
  public static C$ MODULE$;
  private final Object bar;
  public static Object foo() { return C.foo; }
  static {
     new C$();
  }
  private C$() {
    super();
    MODULE$ = this;
    Object x = C.foo; // or UNSAFE.ensureClassInitialized(classOf[C])
    bar = this;
  }
}

Which avoids the NPE.

But in that case, triggering C.<clinit> before C$.<clinit> would reverse the order that the initalizer for foo and the super constructor of the module class is run.

Both of these variations have another drawback: References to C in the RHS of static initializers introduce a cyclic dependency between C.<clinit> and C$.<clinit>. This is deadlock prone if a a pair of threads simultaneously statically initialize the class and module class.

So maybe we instead should put leave all the initalization code in the module class?

class C {
  public static Object foo; // can't be final
}
final class C$ {
  public static C$ MODULE$;
  private final Object bar;
  public static Object foo() { return C.foo; }
  static {
     new C$();
  }
  private C$() {
    super();
    MODULE$ = this;
    C.foo = this;
    bar = this;
  }
}

This looks more promising for thread-safe initialization, and preserving semantics. But we are no unable to declare foo as final, which is a bit of a drag, because one reason you want to make a field static is to hold a method handle "constant", and hotspot only is able to fully inline invocations through that handle if the field is marked final. (Or, if it has the JDK internal annotation, java.lang.invoke.Stable, which isn't in our box of tricks.)

Member

retronym commented Feb 15, 2017

I'd like to see the "Compilation scheme" section fleshed out. Currently, it talks about how this would fit into the compiler pipeline, but lacks details about how things would be compiled to bytecode.

Here's an example I'm interested in:

package p1;  object C { @static val foo: AnyRef = C; val bar: AnyRef = C }

If I translate this according to the intuitive description herein, taking a few guesses at the details, I start with:

public class Test {
  public static void main(String[] args) {
    System.out.println(C$.MODULE$.foo()); // null
  }
}

class C {
  public static final Object foo;
  static {
    foo = C$.MODULE$; // initializer moved to CompanionClass.<clinit>
  }
}
final class C$ {
  public static C$ MODULE$;
  private final Object bar;
  public static Object foo() { return C.foo; }
  static {
     Object x = C.foo; // or UNSAFE.ensureClassInitialized(classOf[C])
     new C$();
  }
  private C$() {
    super();
    MODULE$ = this;
    bar = this;
  }
}

Running this prints null. I guess we should move the call to C.<init> after the super constructor call and the assignment of the module var:

final class C$ {
  public static C$ MODULE$;
  private final Object bar;
  public static Object foo() { return C.foo; }
  static {
     new C$();
  }
  private C$() {
    super();
    MODULE$ = this;
    Object x = C.foo; // or UNSAFE.ensureClassInitialized(classOf[C])
    bar = this;
  }
}

Which avoids the NPE.

But in that case, triggering C.<clinit> before C$.<clinit> would reverse the order that the initalizer for foo and the super constructor of the module class is run.

Both of these variations have another drawback: References to C in the RHS of static initializers introduce a cyclic dependency between C.<clinit> and C$.<clinit>. This is deadlock prone if a a pair of threads simultaneously statically initialize the class and module class.

So maybe we instead should put leave all the initalization code in the module class?

class C {
  public static Object foo; // can't be final
}
final class C$ {
  public static C$ MODULE$;
  private final Object bar;
  public static Object foo() { return C.foo; }
  static {
     new C$();
  }
  private C$() {
    super();
    MODULE$ = this;
    C.foo = this;
    bar = this;
  }
}

This looks more promising for thread-safe initialization, and preserving semantics. But we are no unable to declare foo as final, which is a bit of a drag, because one reason you want to make a field static is to hold a method handle "constant", and hotspot only is able to fully inline invocations through that handle if the field is marked final. (Or, if it has the JDK internal annotation, java.lang.invoke.Stable, which isn't in our box of tricks.)

@lrytz

This comment has been minimized.

Show comment
Hide comment
@lrytz

lrytz Feb 15, 2017

Member

We could also consider omitting the accessor methods for @static fields and emitting direct accesses to the field.

Member

lrytz commented Feb 15, 2017

We could also consider omitting the accessor methods for @static fields and emitting direct accesses to the field.

@DarkDimius

This comment has been minimized.

Show comment
Hide comment
@DarkDimius

DarkDimius Feb 15, 2017

Member
class C {
  public static Object foo;
  static {
    foo = C$.MODULE$;
  }
}
final class C$ {
  public static C$ MODULE$;
  private final Object bar;
  public static Object foo() { return C.foo; }
  static {
     new C$();
  }
  private C$() {
    super();
    MODULE$ = this;
    Object x = C.foo; // or UNSAFE.ensureClassInitialized(classOf[C])
    bar = this;
  }
}

Is the proposed desugaring.

@retronym

I guess we should move the call to C.<init> after the super constructor call and the assignment of the module var:

I guess you've meant C.<clinit>. Than that's true.

But in that case, triggering C.<clinit> before C$.<clinit> would reverse the order that the initalizer for foo and the super constructor of the module class is run.

C$.<clinit> isn't user defined. Did you mean C$.<init>?

In case you did, the order in which initializer for foo and the super constructor of the module class is run indeed may change under the proposed sip. It's discussed here https://github.com/scala/scala.github.com/pull/658/files#diff-9284321b7900073f358a16a971c06881R159 , do you want me to expand this discussion?

This is deadlock prone if a a pair of threads simultaneously statically initialize the class and module class.

If we always trigger initialization of C.<clinit> from C$.<init> we will introduce ordering here that should help. But in case user-code inherently has cycles in inititaliziation, I don't think we can do anything.

Thanks for the questions, I'll expand the "Compilation scheme" section.

Member

DarkDimius commented Feb 15, 2017

class C {
  public static Object foo;
  static {
    foo = C$.MODULE$;
  }
}
final class C$ {
  public static C$ MODULE$;
  private final Object bar;
  public static Object foo() { return C.foo; }
  static {
     new C$();
  }
  private C$() {
    super();
    MODULE$ = this;
    Object x = C.foo; // or UNSAFE.ensureClassInitialized(classOf[C])
    bar = this;
  }
}

Is the proposed desugaring.

@retronym

I guess we should move the call to C.<init> after the super constructor call and the assignment of the module var:

I guess you've meant C.<clinit>. Than that's true.

But in that case, triggering C.<clinit> before C$.<clinit> would reverse the order that the initalizer for foo and the super constructor of the module class is run.

C$.<clinit> isn't user defined. Did you mean C$.<init>?

In case you did, the order in which initializer for foo and the super constructor of the module class is run indeed may change under the proposed sip. It's discussed here https://github.com/scala/scala.github.com/pull/658/files#diff-9284321b7900073f358a16a971c06881R159 , do you want me to expand this discussion?

This is deadlock prone if a a pair of threads simultaneously statically initialize the class and module class.

If we always trigger initialization of C.<clinit> from C$.<init> we will introduce ordering here that should help. But in case user-code inherently has cycles in inititaliziation, I don't think we can do anything.

Thanks for the questions, I'll expand the "Compilation scheme" section.

@retronym

This comment has been minimized.

Show comment
Hide comment
@retronym

retronym Feb 15, 2017

Member

do you want me to expand this discussion?

Yes, it would be useful to talk about the super constructor thing explicitly,
perhaps with an example showing how the order could vary depending on how
whether the class or module was initialized first.

If we always trigger initialization of C. from C$. we will
introduce ordering here that should help. But in case user-code inherently
has cycles in inititaliziation, I don't think we can do anything.

I still think that we're introducing a new form of cycle by moving
the code into 'C$'-s static initializer, but I guess that's what the user
is asking for with this annotation, so we can argue that it is the user's
responsibility to code around this possibility.

Member

retronym commented Feb 15, 2017

do you want me to expand this discussion?

Yes, it would be useful to talk about the super constructor thing explicitly,
perhaps with an example showing how the order could vary depending on how
whether the class or module was initialized first.

If we always trigger initialization of C. from C$. we will
introduce ordering here that should help. But in case user-code inherently
has cycles in inititaliziation, I don't think we can do anything.

I still think that we're introducing a new form of cycle by moving
the code into 'C$'-s static initializer, but I guess that's what the user
is asking for with this annotation, so we can argue that it is the user's
responsibility to code around this possibility.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment