Merge pull request #2819 from dotty-staging/workaround-2797

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

Author: smarter

compiler/src/dotty/tools/dotc/ast/Desugar.scala

@@ -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

compiler/src/dotty/tools/dotc/core/SymDenotations.scala

@@ -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? */

compiler/src/dotty/tools/dotc/core/classfile/ClassfileParser.scala

@@ -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)

compiler/src/dotty/tools/dotc/transform/Memoize.scala

@@ -6,6 +6,7 @@ import DenotTransformers._
 import Phases.Phase
 import Contexts.Context
 import SymDenotations.SymDenotation
+import Denotations._
 import Types._
 import Symbols._
 import SymUtils._
@@ -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)
@@ -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)
@@ -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

compiler/src/dotty/tools/dotc/typer/Applications.scala

@@ -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`.
      */
@@ -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 {
@@ -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()

docs/docs/reference/dropped/weak-conformance.md

@@ -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]
 
@@ -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

tests/pending/pos/i2797/Fork_1.java

@@ -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" };
+}

tests/pending/pos/i2797/Test_2.scala

@@ -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

tests/pos/i2797a/Fork.scala

@@ -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

tests/pos/i2797a/Test.scala

@@ -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