Skip to content

Fix #2797: Don't lift Java annotation arguments out of call #2819

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

Merged
merged 11 commits into from
Jul 11, 2017
17 changes: 9 additions & 8 deletions compiler/src/dotty/tools/dotc/ast/Desugar.scala
Original file line number Diff line number Diff line change
Expand Up @@ -106,22 +106,23 @@ object desugar {
def valDef(vdef: ValDef)(implicit ctx: Context): Tree = {
val ValDef(name, tpt, rhs) = vdef
val mods = vdef.mods
def setterNeeded =
val setterNeeded =
(mods is Mutable) && ctx.owner.isClass && (!(mods is PrivateLocal) || (ctx.owner is Trait))
if (setterNeeded) {
// todo: copy of vdef as getter needed?
// val getter = ValDef(mods, name, tpt, rhs) withPos vdef.pos ?
// TODO: copy of vdef as getter needed?
// val getter = ValDef(mods, name, tpt, rhs) withPos vdef.pos?
// right now vdef maps via expandedTree to a thicket which concerns itself.
// I don't see a problem with that but if there is one we can avoid it by making a copy here.
val setterParam = makeSyntheticParameter(tpt = (new SetterParamTree).watching(vdef))
// The rhs gets filled in later, when field is generated and getter has parameters (see Memoize miniphase)
val setterRhs = if (vdef.rhs.isEmpty) EmptyTree else unitLiteral
val setter = cpy.DefDef(vdef)(
name = name.setterName,
tparams = Nil,
name = name.setterName,
tparams = Nil,
vparamss = (setterParam :: Nil) :: Nil,
tpt = TypeTree(defn.UnitType),
rhs = setterRhs
).withMods((mods | Accessor) &~ CaseAccessor) // rhs gets filled in later, when field is generated and getter has parameters
tpt = TypeTree(defn.UnitType),
rhs = setterRhs
).withMods((mods | Accessor) &~ CaseAccessor)
Thicket(vdef, setter)
}
else vdef
Expand Down
2 changes: 1 addition & 1 deletion compiler/src/dotty/tools/dotc/core/SymDenotations.scala
Original file line number Diff line number Diff line change
Expand Up @@ -277,7 +277,7 @@ object SymDenotations {
}

/** Update the annotations of this denotation */
private[core] final def annotations_=(annots: List[Annotation]): Unit =
final def annotations_=(annots: List[Annotation]): Unit =
myAnnotations = annots

/** Does this denotation have an annotation matching the given class symbol? */
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -637,7 +637,7 @@ class ClassfileParser(
val constr = ctx.newSymbol(
owner = classRoot.symbol,
name = nme.CONSTRUCTOR,
flags = Flags.Synthetic,
flags = Flags.Synthetic | Flags.JavaDefined,
info = constrType
).entered
for ((attr, i) <- attrs.zipWithIndex)
Expand Down
46 changes: 30 additions & 16 deletions compiler/src/dotty/tools/dotc/transform/Memoize.scala
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@ import DenotTransformers._
import Phases.Phase
import Contexts.Context
import SymDenotations.SymDenotation
import Denotations._
import Types._
import Symbols._
import SymUtils._
Expand Down Expand Up @@ -50,7 +51,7 @@ import Decorators._
if !ddef.symbol.is(Deferred) &&
!ddef.symbol.isConstructor && // constructors bodies are added later at phase Constructors
ddef.rhs == EmptyTree =>
errorLackImplementation(ddef)
errorLackImplementation(ddef)
case tdef: TypeDef
if tdef.symbol.isClass && !tdef.symbol.is(Deferred) && tdef.rhs == EmptyTree =>
errorLackImplementation(tdef)
Expand All @@ -76,24 +77,33 @@ import Decorators._

ctx.newSymbol(
owner = ctx.owner,
name = sym.name.asTermName.fieldName,
name = sym.name.asTermName.fieldName,
flags = Private | (if (sym is Stable) EmptyFlags else Mutable),
info = fieldType,
coord = tree.pos)
.withAnnotationsCarrying(sym, defn.FieldMetaAnnot)
.enteredAfter(thisTransform)
info = fieldType,
coord = tree.pos
).withAnnotationsCarrying(sym, defn.FieldMetaAnnot)
.enteredAfter(thisTransform)
}

/** Can be used to filter annotations on getters and setters; not used yet */
def keepAnnotations(denot: SymDenotation, meta: ClassSymbol) = {
val cpy = sym.copySymDenotation()
cpy.filterAnnotations(_.symbol.derivesFrom(meta))
if (cpy.annotations ne denot.annotations) cpy.installAfter(thisTransform)
}
def addAnnotations(denot: Denotation): Unit =
denot match {
case fieldDenot: SymDenotation if sym.annotations.nonEmpty =>
val cpy = fieldDenot.copySymDenotation()
cpy.annotations = sym.annotations
cpy.installAfter(thisTransform)
case _ => ()
}

def removeAnnotations(denot: SymDenotation): Unit =
if (sym.annotations.nonEmpty) {
val cpy = sym.copySymDenotation()
cpy.annotations = Nil
cpy.installAfter(thisTransform)
}

lazy val field = sym.field.orElse(newField).asTerm

def adaptToField(tree: Tree) =
def adaptToField(tree: Tree): Tree =
if (tree.isEmpty) tree else tree.ensureConforms(field.info.widen)

val NoFieldNeeded = Lazy | Deferred | JavaDefined | (if (ctx.settings.YnoInline.value) EmptyFlags else Inline)
Expand Down Expand Up @@ -125,14 +135,18 @@ import Decorators._
if (isErasableBottomField(rhsClass)) erasedBottomTree(rhsClass)
else transformFollowingDeep(ref(field))(ctx.withOwner(sym), info)
val getterDef = cpy.DefDef(tree)(rhs = getterRhs)
addAnnotations(fieldDef.denot)
removeAnnotations(sym)
Thicket(fieldDef, getterDef)
} else if (sym.isSetter) {
if (!sym.is(ParamAccessor)) { val Literal(Constant(())) = tree.rhs } // this is intended as an assertion
field.setFlag(Mutable) // necessary for vals mixed in from Scala2 traits
if (!sym.is(ParamAccessor)) { val Literal(Constant(())) = tree.rhs } // This is intended as an assertion
field.setFlag(Mutable) // Necessary for vals mixed in from Scala2 traits
if (isErasableBottomField(tree.vparamss.head.head.tpt.tpe.classSymbol)) tree
else {
val initializer = Assign(ref(field), adaptToField(ref(tree.vparamss.head.head.symbol)))
cpy.DefDef(tree)(rhs = transformFollowingDeep(initializer)(ctx.withOwner(sym), info))
val setterDef = cpy.DefDef(tree)(rhs = transformFollowingDeep(initializer)(ctx.withOwner(sym), info))
removeAnnotations(sym)
setterDef
}
}
else tree // curiously, some accessors from Scala2 have ' ' suffixes. They count as
Expand Down
8 changes: 6 additions & 2 deletions compiler/src/dotty/tools/dotc/typer/Applications.scala
Original file line number Diff line number Diff line change
Expand Up @@ -395,6 +395,10 @@ trait Applications extends Compatibility { self: Typer with Dynamic =>
}
}

/** Is `sym` a constructor of a Java-defined annotation? */
def isJavaAnnotConstr(sym: Symbol) =
sym.is(JavaDefined) && sym.isConstructor && sym.owner.derivesFrom(defn.AnnotationClass)

/** Match re-ordered arguments against formal parameters
* @param n The position of the first parameter in formals in `methType`.
*/
Expand All @@ -420,7 +424,7 @@ trait Applications extends Compatibility { self: Typer with Dynamic =>
}

def tryDefault(n: Int, args1: List[Arg]): Unit = {
liftFun()
if (!isJavaAnnotConstr(methRef.symbol)) liftFun()
val getter = findDefaultGetter(n + numArgs(normalizedFun))
if (getter.isEmpty) missingArg(n)
else {
Expand Down Expand Up @@ -607,7 +611,7 @@ trait Applications extends Compatibility { self: Typer with Dynamic =>
val app1 =
if (!success) app0.withType(UnspecifiedErrorType)
else {
if (!sameSeq(args, orderedArgs)) {
if (!sameSeq(args, orderedArgs) && !isJavaAnnotConstr(methRef.symbol)) {
// need to lift arguments to maintain evaluation order in the
// presence of argument reorderings.
liftFun()
Expand Down
3 changes: 2 additions & 1 deletion docs/docs/reference/dropped/weak-conformance.md
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@ title: Dropped: Weak Conformance
In some situations, Scala used a _weak conformance_ relation when
testing type compatibility or computing the least upper bound of a set
of types. The principal motivation behind weak conformance was to
make as expression like this have type `List[Double]`:
make an expression like this have type `List[Double]`:

List(1.0, math.sqrt(3.0), 0, -3.3) // : List[Double]

Expand Down Expand Up @@ -37,6 +37,7 @@ assigning a type to a constant expression. The new rule is:
- the alternatives of an if-then-else or match expression, or
- the body and catch results of a try expression,


and all expressions have primitive numeric types, but they do not
all have the same type, then the following is attempted: Every
constant expression `E` in `Es` is widened to the least primitive
Expand Down
7 changes: 7 additions & 0 deletions tests/pos/i2797/Fork_1.java
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
// Test is pending because we have no good way to test it.
// We need to: Compile Fork.java, and then compile Test.scala
// with Fork.class on the classpath.
public @interface Fork_1 {
int value() default -1;
String[] jvmArgs() default { "nope" };
}
5 changes: 5 additions & 0 deletions tests/pos/i2797/Test_2.scala
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
// Test is pending because we have no good way to test it.
// We need to: Compile Fork.java, and then compile Test.scala
// with Fork.class on the classpath.
@Fork_1(jvmArgs = Array("I'm", "hot"))
class Test
5 changes: 5 additions & 0 deletions tests/pos/i2797a/Fork.scala
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
// Test is pending because we have no good way to test it.
// We need to: Compile Fork.java, and then compile Test.scala
// with Fork.class on the classpath.
class Fork(value: Int = -1, jvmArgs: Array[String] = Array("nope"))
extends annotation.Annotation
5 changes: 5 additions & 0 deletions tests/pos/i2797a/Test.scala
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
// Test is pending because we have no good way to test it.
// We need to: Compile Fork.java, and then compile Test.scala
// with Fork.class on the classpath.
@Fork(jvmArgs = Array("I'm", "hot"))
class Test