Merge pull request #6978 from dotty-staging/fix-annot-constructor-cycle

odersky · web-flow · commit b3fac385f511 · 2019-08-02T14:29:46.000+02:00
Improve handling of Java annotations, avoid cycles
diff --git a/compiler/src/dotty/tools/backend/jvm/DottyBackendInterface.scala b/compiler/src/dotty/tools/backend/jvm/DottyBackendInterface.scala
@@ -255,16 +255,15 @@ class DottyBackendInterface(outputDirectory: AbstractFile, val superCallsMap: Ma
         }
       case t: TypeApply if (t.fun.symbol == Predef_classOf) =>
         av.visit(name, t.args.head.tpe.classSymbol.denot.info.toTypeKind(bcodeStore)(innerClasesStore).toASMType)
+      case Ident(nme.WILDCARD) =>
+        // An underscore argument indicates that we want to use the default value for this parameter, so do not emit anything
       case t: tpd.RefTree =>
-        if (t.symbol.denot.owner.isAllOf(Flags.JavaEnumTrait)) {
-          val edesc = innerClasesStore.typeDescriptor(t.tpe.asInstanceOf[bcodeStore.int.Type]) // the class descriptor of the enumeration class.
-          val evalue = t.symbol.name.mangledString // value the actual enumeration value.
-          av.visitEnum(name, edesc, evalue)
-        } else {
-            // println(i"not an enum: ${t.symbol} / ${t.symbol.denot.owner} / ${t.symbol.denot.owner.isTerm} / ${t.symbol.denot.owner.flagsString}")
-            assert(toDenot(t.symbol).name.is(DefaultGetterName),
-              s"${toDenot(t.symbol).name.debugString}") // this should be default getter. do not emit.
-        }
+        assert(t.symbol.denot.owner.isAllOf(Flags.JavaEnumTrait),
+          i"not an enum: $t / ${t.symbol} / ${t.symbol.denot.owner} / ${t.symbol.denot.owner.isTerm} / ${t.symbol.denot.owner.flagsString}")
+
+        val edesc = innerClasesStore.typeDescriptor(t.tpe.asInstanceOf[bcodeStore.int.Type]) // the class descriptor of the enumeration class.
+        val evalue = t.symbol.name.mangledString // value the actual enumeration value.
+        av.visitEnum(name, edesc, evalue)
       case t: SeqLiteral =>
         val arrAnnotV: AnnotationVisitor = av.visitArray(name)
         for (arg <- t.elems) { emitArgument(arrAnnotV, null, arg, bcodeStore)(innerClasesStore) }
diff --git a/compiler/src/dotty/tools/dotc/core/classfile/ClassfileParser.scala b/compiler/src/dotty/tools/dotc/core/classfile/ClassfileParser.scala
@@ -166,7 +166,10 @@ class ClassfileParser(
       for (i <- 0 until in.nextChar) parseMember(method = false)
       for (i <- 0 until in.nextChar) parseMember(method = true)
       classInfo = parseAttributes(classRoot.symbol, classInfo)
-      if (isAnnotation) addAnnotationConstructor(classInfo)
+      if (isAnnotation)
+        // classInfo must be a TempClassInfoType and not a TempPolyType,
+        // because Java annotations cannot have type parameters.
+        addAnnotationConstructor(classInfo.asInstanceOf[TempClassInfoType])
 
       classRoot.registerCompanion(moduleRoot.symbol)
       moduleRoot.registerCompanion(classRoot.symbol)
@@ -632,67 +635,23 @@ class ClassfileParser(
     cook.apply(newType)
   }
 
-  /** Add synthetic constructor(s) and potentially also default getters which
-   *  reflects the fields of the annotation with given `classInfo`.
-   *  Annotations in Scala are assumed to get all their arguments as constructor
+  /** Annotations in Scala are assumed to get all their arguments as constructor
    *  parameters. For Java annotations we need to fake it by making up the constructor.
-   *  Note that default getters have type Nothing. That's OK because we need
-   *  them only to signal that the corresponding parameter is optional.
    */
-  def addAnnotationConstructor(classInfo: Type, tparams: List[TypeSymbol] = Nil)(implicit ctx: Context): Unit = {
-    def addDefaultGetter(attr: Symbol, n: Int) =
-      ctx.newSymbol(
-        owner = moduleRoot.symbol,
-        name = DefaultGetterName(nme.CONSTRUCTOR, n),
-        flags = attr.flags & Flags.AccessFlags,
-        info = defn.NothingType).entered
-
-    classInfo match {
-      case classInfo @ TempPolyType(tparams, restpe) if tparams.isEmpty =>
-        addAnnotationConstructor(restpe, tparams)
-      case classInfo: TempClassInfoType =>
-        val attrs = classInfo.decls.toList.filter(_.isTerm)
-        val targs = tparams.map(_.typeRef)
-        val paramNames = attrs.map(_.name.asTermName)
-        val paramTypes = attrs.map(_.info.resultType)
-
-        def addConstr(ptypes: List[Type]) = {
-          val mtype = MethodType(paramNames, ptypes, classRoot.typeRef.appliedTo(targs))
-          val constrType = if (tparams.isEmpty) mtype else TempPolyType(tparams, mtype)
-          val constr = ctx.newSymbol(
-            owner = classRoot.symbol,
-            name = nme.CONSTRUCTOR,
-            flags = Flags.Synthetic | Flags.JavaDefined | Flags.Method,
-            info = constrType
-          ).entered
-          for ((attr, i) <- attrs.zipWithIndex)
-            if (attr.hasAnnotation(defn.AnnotationDefaultAnnot)) {
-              constr.setFlag(Flags.DefaultParameterized)
-              addDefaultGetter(attr, i)
-            }
-        }
-
-        addConstr(paramTypes)
-
-        // The code below added an extra constructor to annotations where the
-        // last parameter of the constructor is an Array[X] for some X, the
-        // array was replaced by a vararg argument. Unfortunately this breaks
-        // inference when doing:
-        //   @Annot(Array())
-        // The constructor is overloaded so the expected type of `Array()` is
-        // WildcardType, and the type parameter of the Array apply method gets
-        // instantiated to `Nothing` instead of `X`.
-        // I'm leaving this commented out in case we improve inference to make this work.
-        // Note that if this is reenabled then JavaParser will also need to be modified
-        // to add the extra constructor (this was not implemented before).
-        /*
-        if (paramTypes.nonEmpty)
-          paramTypes.last match {
-            case defn.ArrayOf(elemtp) =>
-              addConstr(paramTypes.init :+ defn.RepeatedParamType.appliedTo(elemtp))
-            case _ =>
-        }
-        */
+  def addAnnotationConstructor(classInfo: TempClassInfoType)(implicit ctx: Context): Unit =
+    ctx.newSymbol(
+      owner = classRoot.symbol,
+      name = nme.CONSTRUCTOR,
+      flags = Flags.Synthetic | Flags.JavaDefined | Flags.Method,
+      info = new AnnotConstructorCompleter(classInfo)
+    ).entered
+
+  class AnnotConstructorCompleter(classInfo: TempClassInfoType) extends LazyType {
+    def complete(denot: SymDenotation)(implicit ctx: Context): Unit = {
+      val attrs = classInfo.decls.toList.filter(sym => sym.isTerm && sym != denot.symbol)
+      val paramNames = attrs.map(_.name.asTermName)
+      val paramTypes = attrs.map(_.info.resultType)
+      denot.info = MethodType(paramNames, paramTypes, classRoot.typeRef)
     }
   }
 
diff --git a/compiler/src/dotty/tools/dotc/parsing/JavaParsers.scala b/compiler/src/dotty/tools/dotc/parsing/JavaParsers.scala
@@ -127,8 +127,8 @@ object JavaParsers {
 
     def makeSyntheticParam(count: Int, tpt: Tree): ValDef =
       makeParam(nme.syntheticParamName(count), tpt)
-    def makeParam(name: TermName, tpt: Tree, defaultValue: Tree = EmptyTree): ValDef =
-      ValDef(name, tpt, defaultValue).withMods(Modifiers(Flags.JavaDefined | Flags.Param))
+    def makeParam(name: TermName, tpt: Tree): ValDef =
+      ValDef(name, tpt, EmptyTree).withMods(Modifiers(Flags.JavaDefined | Flags.Param))
 
     def makeConstructor(formals: List[Tree], tparams: List[TypeDef], flags: FlagSet = Flags.JavaDefined): DefDef = {
       val vparams = formals.zipWithIndex.map { case (p, i) => makeSyntheticParam(i + 1, p) }
@@ -768,17 +768,7 @@ object JavaParsers {
       val (statics, body) = typeBody(AT, name, List())
       val constructorParams = body.collect {
         case dd: DefDef =>
-          val hasDefault =
-            dd.mods.annotations.exists {
-              case Apply(Select(New(Select(_, tpnme.AnnotationDefaultATTR)), nme.CONSTRUCTOR), Nil) =>
-                true
-              case _ =>
-                false
-            }
-          // If the annotation has a default value we don't need to parse it, providing
-          // any value at all is enough to typecheck usages of annotations correctly.
-          val defaultParam = if (hasDefault) unimplementedExpr else EmptyTree
-          makeParam(dd.name, dd.tpt, defaultParam)
+          makeParam(dd.name, dd.tpt)
       }
       val constr = DefDef(nme.CONSTRUCTOR,
         List(), List(constructorParams), TypeTree(), EmptyTree).withMods(Modifiers(Flags.JavaDefined))
diff --git a/compiler/src/dotty/tools/dotc/typer/Applications.scala b/compiler/src/dotty/tools/dotc/typer/Applications.scala
@@ -543,17 +543,39 @@ trait Applications extends Compatibility { self: Typer with Dynamic =>
           }
 
           def tryDefault(n: Int, args1: List[Arg]): Unit = {
-            val getter =
-              // `methRef.symbol` doesn't exist for structural calls
-              if (methRef.symbol.exists) findDefaultGetter(n + numArgs(normalizedFun))
-              else EmptyTree
-            if (getter.isEmpty) missingArg(n)
-            else {
-              val substParam = addTyped(
-                  treeToArg(spliceMeth(getter.withSpan(normalizedFun.span), normalizedFun)),
-                  formal)
+            val sym = methRef.symbol
+
+            val defaultExpr =
+              if (isJavaAnnotConstr(sym)) {
+                val cinfo = sym.owner.asClass.classInfo
+                val pname = methodType.paramNames(n)
+                val hasDefault = cinfo.member(pname)
+                  .suchThat(d => d.is(Method) && d.hasAnnotation(defn.AnnotationDefaultAnnot)).exists
+
+                // Use `_` as a placeholder for the default value of an
+                // annotation parameter, this will be recognized by the backend.
+                if (hasDefault)
+                  tpd.Underscore(formal)
+                else
+                  EmptyTree
+              } else {
+                val getter =
+                  if (sym.exists) // `sym` doesn't exist for structural calls
+                    findDefaultGetter(n + numArgs(normalizedFun))
+                  else
+                    EmptyTree
+
+                if (!getter.isEmpty)
+                  spliceMeth(getter.withSpan(normalizedFun.span), normalizedFun)
+                else
+                  EmptyTree
+              }
+
+            if (!defaultExpr.isEmpty) {
+              val substParam = addTyped(treeToArg(defaultExpr), formal)
               matchArgs(args1, formals1.mapconserve(substParam), n + 1)
-            }
+            } else
+              missingArg(n)
           }
 
           if (formal.isRepeatedParam)
diff --git a/tests/pos-java-interop-separate/i6868/MyJava_1.java b/tests/pos-java-interop-separate/i6868/MyJava_1.java
@@ -0,0 +1,15 @@
+public @interface MyJava_1 {
+
+    public String value() default "MyJava";
+
+    public MyClassTypeA typeA();
+
+    public MyClassTypeB typeB() default @MyClassTypeB;
+
+    public enum MyClassTypeA {
+	A, B
+    }
+
+    public @interface MyClassTypeB {}
+}
+
diff --git a/tests/pos-java-interop-separate/i6868/MyScala_2.scala b/tests/pos-java-interop-separate/i6868/MyScala_2.scala
@@ -0,0 +1,13 @@
+@MyJava_1("MyScala1", typeA = MyJava_1.MyClassTypeA.B)
+object MyScala {
+  def a(mj: MyJava_1): Unit = {
+    println("MyJava")
+  }
+
+  @MyJava_1(typeA = MyJava_1.MyClassTypeA.A)
+  def b(): Int = 1
+
+  @MyJava_1(value = "MyScala2", typeA = MyJava_1.MyClassTypeA.B)
+  def c(): Int = 2
+}
+
