Implement @static annotation on singleton object fields. #894

axel22 · 2012-07-12T22:36:08Z

This commit introduces the @static annotation on fields of static singleton objects.

A static singleton object is a top-level singleton object or an object nested within
a static singleton object.

The field annotated with @static is generated as a true static field in the companion
class of the object. The initialization of the field is done in the static initializer of the
companion class, instead of the object itself.

Here's an example. This:

object Foo {
  @static val FIELD = 123
}
class Foo

generates :

object Foo {
  def FIELD(): Int = Foo.FIELD
}
class Foo {
  <static> val FIELD = 123
}

The accessor in object Foo is changed to return the static
field in class Foo (the companion class is generated if it is
not already present, and the same is done for getters if FIELD
is a var).

Furthermore, all the callsites to accessor Foo.FIELD are rewritten
to access the static field directly.

It is illegal to annotate a field in the singleton object as @static if there is
already a member of the same name in class Foo.

This allows better Java interoperability with frameworks which rely on static fields being
present in the class.
The AtomicReferenceFieldUpdaters are one such example. Instead of having
a Java base class holding the volatile mutable field f and a static field containing
a reference to the AtomicReferenceFieldUpdater that mutates f atomically,
and extending this Java base class from Scala, we can now simply do:

object Foo {
  @static val arfu = AtomicReferenceUpdater.newUpdater(
    classOf[Foo], classOf[String], "f"
  )
}
class Foo {
  @volatile var f = null
  import Foo._
  def CAS(ov: String, nv: String) = arfu.compareAndSet(this, null, "")
}

In summary, this commit introduces the following:

adds the @static annotation
for objects without a companion class and @static members, creates a companion class
(this is done in CleanUp)
checks for conflicting names and if @static is used on static singleton object member
creates the static field in the companion class for @static members
moves the field initialization from the companion object to the static initializer of
the class (the initialization of @static members is bypassed in the Constructors phase,
and added to static ctors in CleanUp)
all callsites to the accessors of @static are rewritten to access the static fields directly
(this is done in GenICode)
getters and setters to @static fields access the static field directly, and the field in the
singleton object is not emitted (this is done in GenICode)
changes in GenJVM, GenASM - now computing local var indices in static initializers
as well - this was an oversight before, now is necessary

Future work: allow the @static annotation on methods as well - this will
be added next.

Review by @odersky
Review by @dragos

axel22 · 2012-07-12T22:37:25Z

https://scala-webapps.epfl.ch/jenkins/job/scala-checkin-manual/209/console

soc · 2012-07-13T10:48:22Z

Copying from the closed pull request:

I remember that there was an earlier hack included to fix an issue with Android compatibility, I think it would make sense to remove that hack now and check that "static" also works for this "real world use case".

Look for Parcelable in GenJVM.scala, JAndroidBuilder in GenASM.scala and the whole GenAndroid.scala file.

retronym · 2012-07-13T11:29:23Z

https://issues.scala-lang.org/browse/SI-4581

soc · 2012-07-13T20:36:43Z

Thinking about it again ... I guess it would make sense to check for Parcelable and issue a migration warning. Probably also just deprecate the functionality, so that Android keeps working in 2.10. But then special care would need to be taken that something like @static final val CREATOR: Parcelable.Creator[...] wouldn't emit code twice.

soc · 2012-07-14T14:20:54Z

src/library/scala/annotation/static.scala

+/**
+ * An annotation that marks a member in the companion object as static
+ * and ensures that the compiler generates static fields/methods for it.
+ * This is important for Java interoperability and performance reasons.


performance reasons

As much I this can be true, I fear that people will start sprinkling it over their code to get "magically" faster performance. Maybe it would be better to leave that part of the documentation out?

Maybe we should. In fact, maybe we should also put it behind a language import.

Good idea! Yes, seems to be reasonable. I fear the day were beginners start to write code like @specialized @inline @static def foo .... :-)

rkuhn · 2012-07-14T19:11:47Z

One question: why move the initialization out of the object’s constructor? The object is initialized from the static initializer anyway (at least in 2.10.0-M5 as I verified looking at some byte code), so which problems would ensue by just having the constructor happen “in sequence”? It would for sure make understanding the code flow easier, since the non-static parts are intermingled lexically with the static ones. And you could remove some code, which is always good ;-)

axel22 · 2012-07-14T22:18:05Z

That's exactly the first thing I've intended to do. The problem is that AtomicReferenceUpdaters will (on creation) check to see if the caller is the owner of the field they're supposed to modify, and throw an exception if it's not. This is why it's imperative that the creation of the arfu occurs in the static initializer of the class itself.

rkuhn · 2012-07-15T06:09:06Z

What a pity that this very special and extremely quirky use-case should force such a questionable design choice. If someone goes to the trouble of putting an ARFU into a static field, why don’t they go the full nine yards and use Unsafe? The whole j.u.c. package has been moved this way for JDK7 and onward.

adriaanm · 2012-07-16T11:39:50Z

I don't think this should go into 2.10.x since it's not a critical bugfix AFAICT.
Please feel free to argue otherwise and reopen.

axel22 · 2012-07-16T11:55:10Z

I think Martin was very interested to get this into 2.10. It was his idea to do this.

heathermiller · 2012-07-16T11:56:06Z

I can vouch for that ^

adriaanm · 2012-07-16T11:58:34Z

I see. I didn't know, sorry. It still feels like an improvement instead of a critical bug fix.

Let's discuss it at the meeting tomorrow.

axel22 · 2012-07-16T12:02:10Z

Sure!

adriaanm · 2012-07-18T11:59:25Z

@axel22 could you please rebase on 2.10.x and push -f?

axel22 · 2012-07-18T13:40:31Z

Yes, I'll do this now.

@odersky

This commit introduces the `@static` annotation on fields of static singleton objects. A static singleton object is a top-level singleton object or an object nested within a static singleton object. The field annotated with `@static` is generated as a true static field in the companion class of the object. The initialization of the field is done in the static initializer of the companion class, instead of the object itself. Here's an example. This: object Foo { @static val FIELD = 123 } class Foo generates : object Foo { def FIELD(): Int = Foo.FIELD } class Foo { <static> val FIELD = 123 } The accessor in `object Foo` is changed to return the static field in `class Foo` (the companion class is generated if it is not already present, and the same is done for getters if `FIELD` is a `var`). Furthermore, all the callsites to accessor `Foo.FIELD` are rewritten to access the static field directly. It is illegal to annotate a field in the singleton object as `@static` if there is already a member of the same name in `class Foo`. This allows better Java interoperability with frameworks which rely on static fields being present in the class. The `AtomicReferenceFieldUpdater`s are one such example. Instead of having a Java base class holding the `volatile` mutable field `f` and a static field containing a reference to the `AtomicReferenceFieldUpdater` that mutates `f` atomically, and extending this Java base class from Scala, we can now simply do: object Foo { @static val arfu = AtomicReferenceUpdater.newUpdater( classOf[Foo], classOf[String], "f" ) } class Foo { @volatile var f = null import Foo._ def CAS(ov: String, nv: String) = arfu.compareAndSet(this, null, "") } In summary, this commit introduces the following: - adds the `@static` annotation - for objects without a companion class and `@static` members, creates a companion class (this is done in `CleanUp`) - checks for conflicting names and if `@static` is used on static singleton object member - creates the `static` field in the companion class for `@static` members - moves the field initialization from the companion object to the static initializer of the class (the initialization of `@static` members is bypassed in the `Constructors` phase, and added to static ctors in `CleanUp`) - all callsites to the accessors of `@static` are rewritten to access the static fields directly (this is done in `GenICode`) - getters and setters to `@static` fields access the static field directly, and the field in the singleton object is not emitted (this is done in `GenICode`) - changes in `GenJVM`, `GenASM` - now computing local var indices in static initializers as well - this was an oversight before, now is necessary Future work: allow the `@static` annotation on methods as well - this will be added in a future commit. Review by @odersky, @dragos, @paulp, @heathermiller.

axel22 · 2012-07-18T15:14:25Z

I did a push -f, should be ok for merge now.

PLS REBUILD ALL

scala-jenkins · 2012-07-18T16:09:27Z

Started jenkins job pr-scala-testsuite-linux-opt at https://scala-webapps.epfl.ch/jenkins/job/pr-scala-testsuite-linux-opt/520/

scala-jenkins · 2012-07-18T16:10:53Z

jenkins job pr-scala-testsuite-linux-opt: Success - https://scala-webapps.epfl.ch/jenkins/job/pr-scala-testsuite-linux-opt/520/

adriaanm · 2012-07-18T16:13:12Z

Ok, so now we wait for some one to sign off :-)

dragos · 2012-07-19T10:01:22Z

src/compiler/scala/tools/nsc/backend/icode/GenICode.scala

+          if (m.symbol.isAccessor && m.symbol.accessed.hasStaticAnnotation) {
+            // in companion object accessors to @static fields, we access the static field directly
+            val hostClass = m.symbol.owner.companionClass
+            val staticfield = hostClass.info.decls.find(_.name.toString.trim == m.symbol.accessed.name.toString.trim)


Can't you store the static symbol somewhere? I'm worried linear search might be too slow, plus it's comparing strings, which isn't fast either.

I could store it somewhere during cleanup, but I'm not sure where so that it's visible later during icode. Will look into it.

Also, maybe findMember could help.

axel22 · 2012-07-19T11:14:36Z

I will address these issues and add a commit once I do.

scala-jenkins · 2012-07-19T13:16:17Z

Started jenkins job pr-scala-testsuite-linux-opt at https://scala-webapps.epfl.ch/jenkins/job/pr-scala-testsuite-linux-opt/607/

scala-jenkins · 2012-07-19T14:53:59Z

jenkins job pr-scala-testsuite-linux-opt: Success - https://scala-webapps.epfl.ch/jenkins/job/pr-scala-testsuite-linux-opt/607/

adriaanm · 2012-07-19T15:32:37Z

@dragos, if the added commits address your feedback to your liking, could you sign off so that we can send @axel22 off to his vacation destination? :-)

The upcoming `findMember` optimizations should ensure that this is fast enough.

axel22 · 2012-07-20T13:18:07Z

Thanks @adriaanm :))
I think that if the last commit passes the build, pretty much everything in the feedback should be addressed.

axel22 · 2012-07-20T13:18:13Z

PLS REBUILD ALL

scala-jenkins · 2012-07-20T14:04:48Z

Started jenkins job pr-scala-testsuite-linux-opt at https://scala-webapps.epfl.ch/jenkins/job/pr-scala-testsuite-linux-opt/623/

scala-jenkins · 2012-07-20T15:57:30Z

jenkins job pr-scala-testsuite-linux-opt: Success - https://scala-webapps.epfl.ch/jenkins/job/pr-scala-testsuite-linux-opt/623/

Implement @static annotation on singleton object fields.

soc reviewed Jul 14, 2012
View reviewed changes

adriaanm closed this Jul 16, 2012

adriaanm reopened this Jul 16, 2012

dragos reviewed Jul 19, 2012
View reviewed changes

WIP add private/lazy checks and a few tests.

2272390

Use findMember to lookup the static field in the host class.

124f316

The upcoming `findMember` optimizations should ensure that this is fast enough.

lrytz added a commit that referenced this pull request Jul 20, 2012

Merge pull request #894 from axel22/topic/static-annot-cherry-2.10.x

b67828d

Implement @static annotation on singleton object fields.

lrytz merged commit b67828d into scala:2.10.x Jul 20, 2012

adriaanm mentioned this pull request Nov 29, 2016

Add @static SIP scala/docs.scala-lang#491

Merged

scabug mentioned this pull request Apr 7, 2017

Support @static annotation to create static fields or methods. scala/bug#4581

Open

Implement @static annotation on singleton object fields. #894

Implement @static annotation on singleton object fields. #894

Uh oh!

Conversation

axel22 commented Jul 12, 2012

Uh oh!

axel22 commented Jul 12, 2012

Uh oh!

soc commented Jul 13, 2012

Uh oh!

retronym commented Jul 13, 2012

Uh oh!

soc commented Jul 13, 2012

Uh oh!

soc Jul 14, 2012

Choose a reason for hiding this comment

Uh oh!

axel22 Jul 14, 2012

Choose a reason for hiding this comment

Uh oh!

soc Jul 15, 2012

Choose a reason for hiding this comment

Uh oh!

rkuhn commented Jul 14, 2012

Uh oh!

axel22 commented Jul 14, 2012

Uh oh!

rkuhn commented Jul 15, 2012

Uh oh!

adriaanm commented Jul 16, 2012

Uh oh!

axel22 commented Jul 16, 2012

Uh oh!

heathermiller commented Jul 16, 2012

Uh oh!

adriaanm commented Jul 16, 2012

Uh oh!

axel22 commented Jul 16, 2012

Uh oh!

adriaanm commented Jul 18, 2012

Uh oh!

axel22 commented Jul 18, 2012

Uh oh!

axel22 commented Jul 18, 2012

Uh oh!

scala-jenkins commented Jul 18, 2012

Uh oh!

scala-jenkins commented Jul 18, 2012

Uh oh!

adriaanm commented Jul 18, 2012

Uh oh!

dragos Jul 19, 2012

Choose a reason for hiding this comment

Uh oh!

axel22 Jul 19, 2012

Choose a reason for hiding this comment

Uh oh!

axel22 commented Jul 19, 2012

Uh oh!

scala-jenkins commented Jul 19, 2012

Uh oh!

scala-jenkins commented Jul 19, 2012

Uh oh!

adriaanm commented Jul 19, 2012

Uh oh!

axel22 commented Jul 20, 2012

Uh oh!

axel22 commented Jul 20, 2012

Uh oh!

scala-jenkins commented Jul 20, 2012

Uh oh!

scala-jenkins commented Jul 20, 2012

Uh oh!

Uh oh!