Skip to content

Fix handling of arrays in java annotation arguments #8360

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 4 commits into from
Mar 22, 2020
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
54 changes: 54 additions & 0 deletions compiler/src/dotty/tools/dotc/ast/Trees.scala
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@ package ast

import core._
import Types._, Names._, NameOps._, Flags._, util.Spans._, Contexts._, Constants._
import typer.ProtoTypes
import SymDenotations._, Symbols._, Denotations._, StdNames._, Comments._
import language.higherKinds
import collection.mutable.ListBuffer
Expand Down Expand Up @@ -1505,5 +1506,58 @@ object Trees {
case tree: TypeDef => cpy.TypeDef(tree)(name = newName.asTypeName)
}
}.asInstanceOf[tree.ThisTree[T]]

/** Delegate to FunProto or FunProtoTyped depending on whether the prefix is `untpd` or `tpd`. */
protected def FunProto(args: List[Tree], resType: Type)(using Context): ProtoTypes.FunProto

/** Construct the application `$receiver.$method[$targs]($args)` using overloading resolution
* to find a matching overload of `$method` if necessary.
* This is useful when overloading resolution needs to be performed in a phase after typer.
* Note that this will not perform any kind of implicit search.
*
* @param expectedType An expected type of the application used to guide overloading resolution
*/
def applyOverloaded(
receiver: tpd.Tree, method: TermName, args: List[Tree], targs: List[Type],
expectedType: Type)(using parentCtx: Context): tpd.Tree = {
given ctx as Context = parentCtx.retractMode(Mode.ImplicitsEnabled)

val typer = ctx.typer
val proto = FunProto(args, expectedType)
val denot = receiver.tpe.member(method)
assert(denot.exists, i"no member $receiver . $method, members = ${receiver.tpe.decls}")
val selected =
if (denot.isOverloaded) {
def typeParamCount(tp: Type) = tp.widen match {
case tp: PolyType => tp.paramInfos.length
case _ => 0
}
val allAlts = denot.alternatives
.map(denot => TermRef(receiver.tpe, denot.symbol))
.filter(tr => typeParamCount(tr) == targs.length)
.filter { _.widen match {
case MethodTpe(_, _, x: MethodType) => !x.isImplicitMethod
Copy link
Contributor

Choose a reason for hiding this comment

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

Why drop only implicit methods that are followed by some other method type? Should this not be done for all implicit methods?

Copy link
Member Author

Choose a reason for hiding this comment

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

Note that I didn't write this code, I just moved it from tpd.scala That said, this drops methods whose result type is an implicit method, e.g. if you have def foo(x: A) and def foo(x: A)(implicit y: B), it will pick the first alternative, which seems reasonable to me.

case _ => true
}}
val alternatives = ctx.typer.resolveOverloaded(allAlts, proto)
assert(alternatives.size == 1,
i"${if (alternatives.isEmpty) "no" else "multiple"} overloads available for " +
i"$method on ${receiver.tpe.widenDealiasKeepAnnots} with targs: $targs%, %; args: $args%, %; expectedType: $expectedType." +
i"all alternatives: ${allAlts.map(_.symbol.showDcl).mkString(", ")}\n" +
i"matching alternatives: ${alternatives.map(_.symbol.showDcl).mkString(", ")}.") // this is parsed from bytecode tree. there's nothing user can do about it
alternatives.head
}
else TermRef(receiver.tpe, denot.symbol)
val fun = receiver.select(selected).appliedToTypes(targs)

val apply = untpd.Apply(fun, args)
typer.ApplyTo(apply, fun, selected, proto, expectedType)
}


def resolveConstructor(atp: Type, args: List[Tree])(implicit ctx: Context): tpd.Tree = {
val targs = atp.argTypes
applyOverloaded(tpd.New(atp.typeConstructor), nme.CONSTRUCTOR, args, targs, atp)
}
}
}
46 changes: 5 additions & 41 deletions compiler/src/dotty/tools/dotc/ast/tpd.scala
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@ package dotc
package ast

import dotty.tools.dotc.transform.{ExplicitOuter, Erasure}
import dotty.tools.dotc.typer.ProtoTypes.FunProtoTyped
import typer.ProtoTypes
import transform.SymUtils._
import transform.TypeUtils._
import core._
Expand Down Expand Up @@ -1152,41 +1152,6 @@ object tpd extends Trees.Instance[Type] with TypedTreeInfo {
}
}

def applyOverloaded(receiver: Tree, method: TermName, args: List[Tree], targs: List[Type],
expectedType: Type, isContextual: Boolean = false)(implicit ctx: Context): Tree = {
val typer = ctx.typer
val proto = FunProtoTyped(args, expectedType)(typer, isContextual)
val denot = receiver.tpe.member(method)
assert(denot.exists, i"no member $receiver . $method, members = ${receiver.tpe.decls}")
val selected =
if (denot.isOverloaded) {
def typeParamCount(tp: Type) = tp.widen match {
case tp: PolyType => tp.paramInfos.length
case _ => 0
}
var allAlts = denot.alternatives
.map(denot => TermRef(receiver.tpe, denot.symbol))
.filter(tr => typeParamCount(tr) == targs.length)
.filter { _.widen match {
case MethodTpe(_, _, x: MethodType) => !x.isImplicitMethod
case _ => true
}}
if (targs.isEmpty) allAlts = allAlts.filterNot(_.widen.isInstanceOf[PolyType])
val alternatives = ctx.typer.resolveOverloaded(allAlts, proto)
assert(alternatives.size == 1,
i"${if (alternatives.isEmpty) "no" else "multiple"} overloads available for " +
i"$method on ${receiver.tpe.widenDealiasKeepAnnots} with targs: $targs%, %; args: $args%, % of types ${args.tpes.map(_.widenDealiasKeepAnnots)}%, %; expectedType: $expectedType." +
i"all alternatives: ${allAlts.map(_.symbol.showDcl).mkString(", ")}\n" +
i"matching alternatives: ${alternatives.map(_.symbol.showDcl).mkString(", ")}.") // this is parsed from bytecode tree. there's nothing user can do about it
alternatives.head
}
else TermRef(receiver.tpe, denot.symbol)
val fun = receiver.select(selected).appliedToTypes(targs)

val apply = untpd.Apply(fun, args)
typer.ApplyToTyped(apply, fun, selected, args, expectedType).result.asInstanceOf[Tree] // needed to handle varargs
}

@tailrec
def sameTypes(trees: List[tpd.Tree], trees1: List[tpd.Tree]): Boolean =
if (trees.isEmpty) trees.isEmpty
Expand Down Expand Up @@ -1361,11 +1326,6 @@ object tpd extends Trees.Instance[Type] with TypedTreeInfo {
transformer.transform(tree)
}

def resolveConstructor(atp: Type, args:List[Tree])(implicit ctx: Context): Tree = {
val targs = atp.argTypes
tpd.applyOverloaded(New(atp.typeConstructor), nme.CONSTRUCTOR, args, targs, atp)
}

/** Convert a list of trees to a vararg-compatible tree.
* Used to make arguments for methods that accept varargs.
*/
Expand All @@ -1385,4 +1345,8 @@ object tpd extends Trees.Instance[Type] with TypedTreeInfo {
ref(defn.ListModule).select(nme.apply)
.appliedToTypeTree(tpe)
.appliedToVarargs(trees, tpe)


protected def FunProto(args: List[Tree], resType: Type)(using ctx: Context) =
ProtoTypes.FunProtoTyped(args, resType)(ctx.typer, isUsingApply = false)
}
4 changes: 4 additions & 0 deletions compiler/src/dotty/tools/dotc/ast/untpd.scala
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@ package ast

import core._
import Types._, Contexts._, Constants._, Names._, Flags._
import dotty.tools.dotc.typer.ProtoTypes
import Symbols._, StdNames._, Trees._
import util.{Property, SourceFile, NoSource}
import util.Spans.Span
Expand Down Expand Up @@ -756,4 +757,7 @@ object untpd extends Trees.Instance[Untyped] with UntypedTreeInfo {
}
acc(false, tree)
}

protected def FunProto(args: List[Tree], resType: Type)(using ctx: Context) =
ProtoTypes.FunProto(args, resType)(ctx.typer, isUsingApply = false)
}
4 changes: 2 additions & 2 deletions compiler/src/dotty/tools/dotc/core/Annotations.scala
Original file line number Diff line number Diff line change
Expand Up @@ -142,8 +142,8 @@ object Annotations {
def deferred(atp: Type, args: List[Tree])(implicit ctx: Context): Annotation =
deferred(atp.classSymbol)(New(atp, args))

def deferredResolve(atp: Type, args: List[Tree])(implicit ctx: Context): Annotation =
deferred(atp.classSymbol)(resolveConstructor(atp, args))
def deferredResolve(atp: Type, args: List[ast.untpd.Tree])(implicit ctx: Context): Annotation =
deferred(atp.classSymbol)(ast.untpd.resolveConstructor(atp, args))

/** Extractor for child annotations */
object Child {
Expand Down
34 changes: 22 additions & 12 deletions compiler/src/dotty/tools/dotc/core/classfile/ClassfileParser.scala
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@ import dotty.tools.tasty.{ TastyReader, TastyHeaderUnpickler }
import Contexts._, Symbols._, Types._, Names._, StdNames._, NameOps._, Scopes._, Decorators._
import SymDenotations._, unpickleScala2.Scala2Unpickler._, Constants._, Annotations._, util.Spans._
import NameKinds.DefaultGetterName
import ast.{ tpd, untpd }
import ast.tpd._, util._
import java.io.{ ByteArrayInputStream, ByteArrayOutputStream, DataInputStream, IOException }

Expand Down Expand Up @@ -480,18 +481,30 @@ class ClassfileParser(
}
// sigToType

def parseAnnotArg(skip: Boolean = false)(implicit ctx: Context): Option[Tree] = {
def parseAnnotArg(skip: Boolean = false)(implicit ctx: Context): Option[untpd.Tree] = {

// If we encounter an empty array literal, we need the type of the corresponding
// parameter to properly type it, but that would require forcing the annotation
// early. To avoid this we store annotation arguments as untyped trees
import untpd._

// ... but constants need to actually be typed with a ConstantType, so we
// can't rely on type inference, and type them early.
def lit(c: Constant): Tree = TypedSplice(ast.tpd.Literal(c))

val tag = in.nextByte.toChar
val index = in.nextChar


tag match {
case STRING_TAG =>
if (skip) None else Some(Literal(Constant(pool.getName(index).toString)))
if (skip) None else Some(lit(Constant(pool.getName(index).toString)))
case BOOL_TAG | BYTE_TAG | CHAR_TAG | SHORT_TAG =>
if (skip) None else Some(Literal(pool.getConstant(index, tag)))
if (skip) None else Some(lit(pool.getConstant(index, tag)))
case INT_TAG | LONG_TAG | FLOAT_TAG | DOUBLE_TAG =>
if (skip) None else Some(Literal(pool.getConstant(index)))
if (skip) None else Some(lit(pool.getConstant(index)))
case CLASS_TAG =>
if (skip) None else Some(Literal(Constant(pool.getType(index))))
if (skip) None else Some(lit(Constant(pool.getType(index))))
case ENUM_TAG =>
val t = pool.getType(index)
val n = pool.getName(in.nextChar)
Expand All @@ -500,7 +513,7 @@ class ClassfileParser(
if (skip)
None
else if (s != NoSymbol)
Some(Literal(Constant(s)))
Some(lit(Constant(s)))
else {
ctx.warning(s"""While parsing annotations in ${in.file}, could not find $n in enum $module.\nThis is likely due to an implementation restriction: an annotation argument cannot refer to a member of the annotated class (SI-7014).""")
None
Expand All @@ -517,10 +530,7 @@ class ClassfileParser(
else if (skip) None
else {
val elems = arr.toList
val elemType =
if (elems.isEmpty) defn.ObjectType
else ctx.typeComparer.lub(elems.tpes).widen
Some(JavaSeqLiteral(elems, TypeTree(elemType)))
Some(untpd.JavaSeqLiteral(elems, TypeTree()))
}
case ANNOTATION_TAG =>
parseAnnotation(index, skip) map (_.tree)
Expand All @@ -533,12 +543,12 @@ class ClassfileParser(
def parseAnnotation(attrNameIndex: Char, skip: Boolean = false)(implicit ctx: Context): Option[Annotation] = try {
val attrType = pool.getType(attrNameIndex)
val nargs = in.nextChar
val argbuf = new ListBuffer[Tree]
val argbuf = new ListBuffer[untpd.Tree]
var hasError = false
for (i <- 0 until nargs) {
val name = pool.getName(in.nextChar)
parseAnnotArg(skip) match {
case Some(arg) => argbuf += NamedArg(name, arg)
case Some(arg) => argbuf += untpd.NamedArg(name, arg)
case None => hasError = !skip
}
}
Expand Down
1 change: 1 addition & 0 deletions compiler/src/dotty/tools/dotc/transform/TreeChecker.scala
Original file line number Diff line number Diff line change
Expand Up @@ -287,6 +287,7 @@ class TreeChecker extends Phase with SymTransformer {

override def typed(tree: untpd.Tree, pt: Type = WildcardType)(implicit ctx: Context): Tree = {
val tpdTree = super.typed(tree, pt)
Typer.assertPositioned(tree)
if (ctx.erasedTypes)
// Can't be checked in earlier phases since `checkValue` is only run in
// Erasure (because running it in Typer would force too much)
Expand Down
18 changes: 11 additions & 7 deletions compiler/src/dotty/tools/dotc/typer/Applications.scala
Original file line number Diff line number Diff line change
Expand Up @@ -860,14 +860,9 @@ trait Applications extends Compatibility {
def simpleApply(fun1: Tree, proto: FunProto)(implicit ctx: Context): Tree =
methPart(fun1).tpe match {
case funRef: TermRef =>
val app =
if (proto.allArgTypesAreCurrent())
new ApplyToTyped(tree, fun1, funRef, proto.typedArgs(), pt)
else
new ApplyToUntyped(tree, fun1, funRef, proto, pt)(
using fun1.nullableInArgContext(using argCtx(tree)))
val app = ApplyTo(tree, fun1, funRef, proto, pt)
convertNewGenericArray(
postProcessByNameArgs(funRef, app.result).computeNullable())
postProcessByNameArgs(funRef, app).computeNullable())
case _ =>
handleUnexpectedFunType(tree, fun1)
}
Expand Down Expand Up @@ -991,6 +986,15 @@ trait Applications extends Compatibility {
}
}

/** Typecheck an Apply node with a typed function and possibly-typed arguments coming from `proto` */
def ApplyTo(app: untpd.Apply, fun: tpd.Tree, methRef: TermRef, proto: FunProto, resultType: Type)(using ctx: Context): tpd.Tree =
val typer = ctx.typer
if (proto.allArgTypesAreCurrent())
typer.ApplyToTyped(app, fun, methRef, proto.typedArgs(), resultType).result
else
typer.ApplyToUntyped(app, fun, methRef, proto, resultType)(
using fun.nullableInArgContext(using argCtx(app))).result

/** Overridden in ReTyper to handle primitive operations that can be generated after erasure */
protected def handleUnexpectedFunType(tree: untpd.Apply, fun: Tree)(implicit ctx: Context): Tree =
if ctx.reporter.errorsReported then
Expand Down
2 changes: 2 additions & 0 deletions compiler/src/dotty/tools/dotc/typer/ProtoTypes.scala
Original file line number Diff line number Diff line change
Expand Up @@ -399,6 +399,8 @@ object ProtoTypes {
*/
class FunProtoTyped(args: List[tpd.Tree], resultType: Type)(typer: Typer, isUsingApply: Boolean)(implicit ctx: Context) extends FunProto(args, resultType)(typer, isUsingApply)(ctx) {
override def typedArgs(norm: (untpd.Tree, Int) => untpd.Tree)(implicit ctx: Context): List[tpd.Tree] = args
override def typedArg(arg: untpd.Tree, formal: Type)(implicit ctx: Context): tpd.Tree = arg.asInstanceOf[tpd.Tree]
override def allArgTypesAreCurrent()(implicit ctx: Context): Boolean = true
override def withContext(ctx: Context): FunProtoTyped = this
}

Expand Down
3 changes: 2 additions & 1 deletion compiler/src/dotty/tools/dotc/typer/Typer.scala
Original file line number Diff line number Diff line change
Expand Up @@ -2295,7 +2295,8 @@ class Typer extends Namer
trace(i"typing $tree, pt = $pt", typr, show = true) {
record(s"typed $getClass")
record("typed total")
assertPositioned(tree)
if (ctx.phase.isTyper)
assertPositioned(tree)
if (tree.source != ctx.source && tree.source.exists)
typed(tree, pt, locked)(ctx.withSource(tree.source))
else
Expand Down
3 changes: 3 additions & 0 deletions tests/pos-java-interop-separate/array-annot/Annot_1.java
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
public @interface Annot_1 {
String[] values() default {};
}
8 changes: 8 additions & 0 deletions tests/pos-java-interop-separate/array-annot/Class_1.java
Original file line number Diff line number Diff line change
@@ -0,0 +1,8 @@
public class Class_1 {
@Annot_1(values = { "foo", "bar" })
void foo() {}

@Annot_1(values = {})
void foo2() {}
}

1 change: 1 addition & 0 deletions tests/pos-java-interop-separate/array-annot/Test_2.scala
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
class Test extends Class_1