Skip to content

Commit

Permalink
SI-4788 Respect RetentionPolicy of Java annotations
Browse files Browse the repository at this point in the history
Also closes SI-5420 and SI-5948.
  • Loading branch information
soc committed Feb 10, 2014
1 parent 5b3f0e6 commit 165da8d
Show file tree
Hide file tree
Showing 13 changed files with 155 additions and 122 deletions.
28 changes: 5 additions & 23 deletions src/compiler/scala/tools/nsc/backend/jvm/BCodeHelpers.scala
Expand Up @@ -19,7 +19,7 @@ import scala.tools.nsc.io.AbstractFile
* @version 1.0
*
*/
abstract class BCodeHelpers extends BCodeTypes with BytecodeWriters {
abstract class BCodeHelpers extends BCodeTypes with BytecodeWriters with GenCommon {

import global._

Expand Down Expand Up @@ -694,17 +694,6 @@ abstract class BCodeHelpers extends BCodeTypes with BytecodeWriters {
}
}

/* Whether an annotation should be emitted as a Java annotation
* .initialize: if 'annot' is read from pickle, atp might be un-initialized
*
* must-single-thread
*/
private def shouldEmitAnnotation(annot: AnnotationInfo) =
annot.symbol.initialize.isJavaDefined &&
annot.matches(definitions.ClassfileAnnotationClass) &&
annot.args.isEmpty &&
!annot.matches(definitions.DeprecatedAttr)

/*
* In general,
* must-single-thread
Expand All @@ -724,7 +713,7 @@ abstract class BCodeHelpers extends BCodeTypes with BytecodeWriters {
for(annot <- annotations; if shouldEmitAnnotation(annot)) {
val AnnotationInfo(typ, args, assocs) = annot
assert(args.isEmpty, args)
val av = cw.visitAnnotation(descriptor(typ), true)
val av = cw.visitAnnotation(descriptor(typ), isRuntimeVisible(annot))
emitAssocs(av, assocs)
}
}
Expand All @@ -736,7 +725,7 @@ abstract class BCodeHelpers extends BCodeTypes with BytecodeWriters {
for(annot <- annotations; if shouldEmitAnnotation(annot)) {
val AnnotationInfo(typ, args, assocs) = annot
assert(args.isEmpty, args)
val av = mw.visitAnnotation(descriptor(typ), true)
val av = mw.visitAnnotation(descriptor(typ), isRuntimeVisible(annot))
emitAssocs(av, assocs)
}
}
Expand All @@ -748,7 +737,7 @@ abstract class BCodeHelpers extends BCodeTypes with BytecodeWriters {
for(annot <- annotations; if shouldEmitAnnotation(annot)) {
val AnnotationInfo(typ, args, assocs) = annot
assert(args.isEmpty, args)
val av = fw.visitAnnotation(descriptor(typ), true)
val av = fw.visitAnnotation(descriptor(typ), isRuntimeVisible(annot))
emitAssocs(av, assocs)
}
}
Expand All @@ -763,7 +752,7 @@ abstract class BCodeHelpers extends BCodeTypes with BytecodeWriters {
annot <- annots) {
val AnnotationInfo(typ, args, assocs) = annot
assert(args.isEmpty, args)
val pannVisitor: asm.AnnotationVisitor = jmethod.visitParameterAnnotation(idx, descriptor(typ), true)
val pannVisitor: asm.AnnotationVisitor = jmethod.visitParameterAnnotation(idx, descriptor(typ), isRuntimeVisible(annot))
emitAssocs(pannVisitor, assocs)
}
}
Expand Down Expand Up @@ -859,13 +848,6 @@ abstract class BCodeHelpers extends BCodeTypes with BytecodeWriters {

trait BCForwardersGen extends BCAnnotGen with BCJGenSigGen {

// -----------------------------------------------------------------------------------------
// Static forwarders (related to mirror classes but also present in
// a plain class lacking companion module, for details see `isCandidateForForwarders`).
// -----------------------------------------------------------------------------------------

val ExcludedForwarderFlags = genASM.ExcludedForwarderFlags

/* Adds a @remote annotation, actual use unknown.
*
* Invoked from genMethod() and addForwarder().
Expand Down
76 changes: 62 additions & 14 deletions src/compiler/scala/tools/nsc/backend/jvm/GenASM.scala
Expand Up @@ -20,7 +20,7 @@ import scala.annotation.tailrec
*
* Documentation at http://lamp.epfl.ch/~magarcia/ScalaCompilerCornerReloaded/2012Q2/GenASM.pdf
*/
abstract class GenASM extends SubComponent with BytecodeWriters with GenJVMASM {
abstract class GenASM extends SubComponent with BytecodeWriters with GenCommon {
import global._
import icodes._
import icodes.opcodes._
Expand Down Expand Up @@ -96,6 +96,63 @@ abstract class GenASM extends SubComponent with BytecodeWriters with GenJVMASM {
}
}

private def isJavaEntryPoint(icls: IClass) = {
val sym = icls.symbol
def fail(msg: String, pos: Position = sym.pos) = {
icls.cunit.warning(sym.pos,
sym.name + " has a main method with parameter type Array[String], but " + sym.fullName('.') + " will not be a runnable program.\n" +
" Reason: " + msg
// TODO: make this next claim true, if possible
// by generating valid main methods as static in module classes
// not sure what the jvm allows here
// + " You can still run the program by calling it as " + sym.javaSimpleName + " instead."
)
false
}
def failNoForwarder(msg: String) = {
fail(msg + ", which means no static forwarder can be generated.\n")
}
val possibles = if (sym.hasModuleFlag) (sym.tpe nonPrivateMember nme.main).alternatives else Nil
val hasApproximate = possibles exists { m =>
m.info match {
case MethodType(p :: Nil, _) => p.tpe.typeSymbol == ArrayClass
case _ => false
}
}
// At this point it's a module with a main-looking method, so either succeed or warn that it isn't.
hasApproximate && {
// Before erasure so we can identify generic mains.
enteringErasure {
val companion = sym.linkedClassOfClass

if (hasJavaMainMethod(companion))
failNoForwarder("companion contains its own main method")
else if (companion.tpe.member(nme.main) != NoSymbol)
// this is only because forwarders aren't smart enough yet
failNoForwarder("companion contains its own main method (implementation restriction: no main is allowed, regardless of signature)")
else if (companion.isTrait)
failNoForwarder("companion is a trait")
// Now either succeeed, or issue some additional warnings for things which look like
// attempts to be java main methods.
else (possibles exists isJavaMainMethod) || {
possibles exists { m =>
m.info match {
case PolyType(_, _) =>
fail("main methods cannot be generic.")
case MethodType(params, res) =>
if (res.typeSymbol :: params exists (_.isAbstractType))
fail("main methods cannot refer to type parameters or abstract types.", m.pos)
else
isJavaMainMethod(m) || fail("main method must have exact signature (Array[String])Unit", m.pos)
case tp =>
fail("don't know what this is: " + tp, m.pos)
}
}
}
}
}
}

override def run() {

if (settings.debug)
Expand Down Expand Up @@ -794,15 +851,6 @@ abstract class GenASM extends SubComponent with BytecodeWriters with GenJVMASM {
for (ThrownException(exc) <- excs.distinct)
yield javaName(exc)

/** Whether an annotation should be emitted as a Java annotation
* .initialize: if 'annot' is read from pickle, atp might be un-initialized
*/
private def shouldEmitAnnotation(annot: AnnotationInfo) =
annot.symbol.initialize.isJavaDefined &&
annot.matches(ClassfileAnnotationClass) &&
annot.args.isEmpty &&
!annot.matches(DeprecatedAttr)

// @M don't generate java generics sigs for (members of) implementation
// classes, as they are monomorphic (TODO: ok?)
private def needsGenericSignature(sym: Symbol) = !(
Expand Down Expand Up @@ -989,7 +1037,7 @@ abstract class GenASM extends SubComponent with BytecodeWriters with GenJVMASM {
for(annot <- annotations; if shouldEmitAnnotation(annot)) {
val AnnotationInfo(typ, args, assocs) = annot
assert(args.isEmpty, args)
val av = cw.visitAnnotation(descriptor(typ), true)
val av = cw.visitAnnotation(descriptor(typ), isRuntimeVisible(annot))
emitAssocs(av, assocs)
}
}
Expand All @@ -998,7 +1046,7 @@ abstract class GenASM extends SubComponent with BytecodeWriters with GenJVMASM {
for(annot <- annotations; if shouldEmitAnnotation(annot)) {
val AnnotationInfo(typ, args, assocs) = annot
assert(args.isEmpty, args)
val av = mw.visitAnnotation(descriptor(typ), true)
val av = mw.visitAnnotation(descriptor(typ), isRuntimeVisible(annot))
emitAssocs(av, assocs)
}
}
Expand All @@ -1007,7 +1055,7 @@ abstract class GenASM extends SubComponent with BytecodeWriters with GenJVMASM {
for(annot <- annotations; if shouldEmitAnnotation(annot)) {
val AnnotationInfo(typ, args, assocs) = annot
assert(args.isEmpty, args)
val av = fw.visitAnnotation(descriptor(typ), true)
val av = fw.visitAnnotation(descriptor(typ), isRuntimeVisible(annot))
emitAssocs(av, assocs)
}
}
Expand All @@ -1019,7 +1067,7 @@ abstract class GenASM extends SubComponent with BytecodeWriters with GenJVMASM {
annot <- annots) {
val AnnotationInfo(typ, args, assocs) = annot
assert(args.isEmpty, args)
val pannVisitor: asm.AnnotationVisitor = jmethod.visitParameterAnnotation(idx, descriptor(typ), true)
val pannVisitor: asm.AnnotationVisitor = jmethod.visitParameterAnnotation(idx, descriptor(typ), isRuntimeVisible(annot))
emitAssocs(pannVisitor, assocs)
}
}
Expand Down
48 changes: 48 additions & 0 deletions src/compiler/scala/tools/nsc/backend/jvm/GenCommon.scala
@@ -0,0 +1,48 @@
/* NSC -- new Scala compiler
* Copyright 2005-2013 LAMP/EPFL
* @author Jason Zaugg
*/

package scala.tools.nsc
package backend.jvm
import scala.tools.nsc.symtab._

trait GenCommon extends BytecodeWriters {
import global._
import definitions.{ AnnotationRetentionAttr, AnnotationRetentionPolicyAttr, ClassfileAnnotationClass }

// -----------------------------------------------------------------------------------------
// Static forwarders (related to mirror classes but also present in
// a plain class lacking companion module, for details see `isCandidateForForwarders`).
// -----------------------------------------------------------------------------------------
val ExcludedForwarderFlags = {
import Flags._
// Should include DEFERRED but this breaks findMember.
(SPECIALIZED | LIFTED | PROTECTED | STATIC | EXPANDEDNAME | BridgeAndPrivateFlags | MACRO)
}

lazy val AnnotationRetentionPolicyModule = AnnotationRetentionPolicyAttr.companionModule
lazy val AnnotationRetentionPolicySourceValue = AnnotationRetentionPolicyModule.tpe.member(TermName("SOURCE"))
lazy val AnnotationRetentionPolicyClassValue = AnnotationRetentionPolicyModule.tpe.member(TermName("CLASS"))
lazy val AnnotationRetentionPolicyRuntimeValue = AnnotationRetentionPolicyModule.tpe.member(TermName("RUNTIME"))

/**
* Whether an annotation should be emitted as a Java annotation
* .initialize: if 'annot' is read from pickle, atp might be un-initialized
*/
def shouldEmitAnnotation(annot: AnnotationInfo) =
annot.symbol.initialize.isJavaDefined &&
annot.matches(ClassfileAnnotationClass) &&
retentionPolicyOf(annot) != AnnotationRetentionPolicySourceValue &&
annot.args.isEmpty

def isRuntimeVisible(annot: AnnotationInfo): Boolean =
annot.atp.getAnnotation(AnnotationRetentionAttr)
.exists(_.assocs.contains((nme.value -> LiteralAnnotArg(Constant(AnnotationRetentionPolicyRuntimeValue)))))

def retentionPolicyOf(annot: AnnotationInfo): Symbol =
annot.atp.getAnnotation(AnnotationRetentionAttr).map(_.assocs).map(assoc =>
assoc.collectFirst {
case (`nme`.value, LiteralAnnotArg(Constant(value: Symbol))) => value
}.getOrElse(AnnotationRetentionPolicyClassValue)).getOrElse(AnnotationRetentionPolicyClassValue)
}
83 changes: 0 additions & 83 deletions src/compiler/scala/tools/nsc/backend/jvm/GenJVMASM.scala

This file was deleted.

2 changes: 1 addition & 1 deletion src/compiler/scala/tools/nsc/symtab/SymbolLoaders.scala
Expand Up @@ -206,7 +206,7 @@ abstract class SymbolLoaders {

override def complete(root: Symbol) {
try {
val start = currentTime
val start = System.nanoTime()
val currentphase = phase
doComplete(root)
phase = currentphase
Expand Down
4 changes: 4 additions & 0 deletions src/reflect/scala/reflect/internal/Definitions.scala
Expand Up @@ -1058,6 +1058,10 @@ trait Definitions extends api.StandardDefinitions {
lazy val ClassfileAnnotationClass = requiredClass[scala.annotation.ClassfileAnnotation]
lazy val StaticAnnotationClass = requiredClass[scala.annotation.StaticAnnotation]

// Java retention annotations
lazy val AnnotationRetentionAttr = requiredClass[java.lang.annotation.Retention]
lazy val AnnotationRetentionPolicyAttr = requiredClass[java.lang.annotation.RetentionPolicy]

// Annotations
lazy val BridgeClass = requiredClass[scala.annotation.bridge]
lazy val ElidableMethodClass = requiredClass[scala.annotation.elidable]
Expand Down
2 changes: 1 addition & 1 deletion src/reflect/scala/reflect/internal/SymbolTable.scala
Expand Up @@ -56,7 +56,7 @@ abstract class SymbolTable extends macros.Universe
def abort(msg: String): Nothing = throw new FatalError(supplementErrorMessage(msg))

protected def elapsedMessage(msg: String, start: Long) =
msg + " in " + (TimeUnit.NANOSECONDS.toMillis(System.nanoTime()) - start) + "ms"
msg + " in " + (TimeUnit.NANOSECONDS.toMillis(System.nanoTime() - start)) + "ms"

def informProgress(msg: String) = if (settings.verbose) inform("[" + msg + "]")
def informTime(msg: String, start: Long) = informProgress(elapsedMessage(msg, start))
Expand Down
2 changes: 2 additions & 0 deletions src/reflect/scala/reflect/runtime/JavaUniverseForce.scala
Expand Up @@ -384,6 +384,8 @@ trait JavaUniverseForce { self: runtime.JavaUniverse =>
definitions.AnnotationClass
definitions.ClassfileAnnotationClass
definitions.StaticAnnotationClass
definitions.AnnotationRetentionAttr
definitions.AnnotationRetentionPolicyAttr
definitions.BridgeClass
definitions.ElidableMethodClass
definitions.ImplicitNotFoundClass
Expand Down
4 changes: 4 additions & 0 deletions test/files/run/t4788.check
@@ -0,0 +1,4 @@
warning: there were 1 deprecation warning(s); re-run with -deprecation for details
None
Some(@Ljavax/jws/soap/InitParam;(name="", value="") // invisible)
Some(@Ljavax/xml/ws/Action;())
2 changes: 2 additions & 0 deletions test/files/run/t4788/C.scala
@@ -0,0 +1,2 @@
@javax.jws.soap.InitParam(name="", value="") // RetentionPolicy.CLASS
class C
2 changes: 2 additions & 0 deletions test/files/run/t4788/R.scala
@@ -0,0 +1,2 @@
@javax.xml.ws.Action // RetentionPolicy.RUNTIME
class R
2 changes: 2 additions & 0 deletions test/files/run/t4788/S.scala
@@ -0,0 +1,2 @@
@javax.annotation.Generated(Array("")) // RetentionPolicy.SOURCE
class S

1 comment on commit 165da8d

@scala-jenkins
Copy link

Choose a reason for hiding this comment

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

Job pr-scala failed for 165da8d Took 24 min. (ping @soc) (results):


To retry exactly this commit, comment "PLS REBUILD/pr-scala@165da8da557ae77bf8d6a30fb57d49868109d1aa" on PR 3459.
NOTE: New commits are rebuilt automatically as they appear. A forced rebuild is only necessary for transient failures.

Please sign in to comment.