Skip to content

Commit 67b3930

Browse files
committed
Fix #19951: Align TASTy with the Java annotation model.
Scala annotations are classes, with a real constructor, which has a real signature where order is relevant but names are irrelevant. On the contrary, Java annotations are interfaces, without any real constructors. The names of "fields" are relevant, whereas their order is irrelevant. As illustrated by #19951, trying to shoehorn Java annotations into the Scala annotation model is not sustainable, and breaks in real ways. Therefore, in this commit we align how Java annotations are stored in TASTy with the Java annotation model. During pickling: * Selection of the constructor is pickled without a signature. * Default arguments are dropped. * (Due to the parent commit, all arguments are `NamedArg`s at this point.) During unpickling: * Selection of the constructor resolves to the unique constructor (instead of complaining because a signature-less `SELECT` should not resolve to a member with a signature). * Arguments to the constructor are reordered and extended with defaults to match the target constructor; we can do this because all the arguments are `NamedArg`s. For backward compatibility, during unpickling: * If we read a `SELECTin` for a Java annotation constructor, we disregard its signature and pretend it was a `SELECT`. * We adapt arguments in best-effort way if not all of them are `NamedArg`s.
1 parent 4fd41f7 commit 67b3930

File tree

2 files changed

+100
-7
lines changed

2 files changed

+100
-7
lines changed

Diff for: compiler/src/dotty/tools/dotc/core/tasty/TreePickler.scala

+12-2
Original file line numberDiff line numberDiff line change
@@ -466,7 +466,10 @@ class TreePickler(pickler: TastyPickler, attributes: Attributes) {
466466
}
467467
case _ =>
468468
if passesConditionForErroringBestEffortCode(tree.hasType) then
469-
val sig = tree.tpe.signature
469+
// #19951 The signature of a constructor of a Java annotation is irrelevant
470+
val sig =
471+
if name == nme.CONSTRUCTOR && tree.symbol.owner.is(JavaAnnotation) then Signature.NotAMethod
472+
else tree.tpe.signature
470473
var ename = tree.symbol.targetName
471474
val selectFromQualifier =
472475
name.isTypeName
@@ -507,7 +510,14 @@ class TreePickler(pickler: TastyPickler, attributes: Attributes) {
507510
writeByte(APPLY)
508511
withLength {
509512
pickleTree(fun)
510-
args.foreach(pickleTree)
513+
// #19951 Do not pickle default arguments to Java annotation constructors
514+
if fun.symbol.isClassConstructor && fun.symbol.owner.is(JavaAnnotation) then
515+
for arg <- args do
516+
arg match
517+
case NamedArg(_, Ident(nme.WILDCARD)) => ()
518+
case _ => pickleTree(arg)
519+
else
520+
args.foreach(pickleTree)
511521
}
512522
}
513523
case TypeApply(fun, args) =>

Diff for: compiler/src/dotty/tools/dotc/core/tasty/TreeUnpickler.scala

+88-5
Original file line numberDiff line numberDiff line change
@@ -1281,7 +1281,13 @@ class TreeUnpickler(reader: TastyReader,
12811281
if unpicklingJava && name == tpnme.Object && qual.symbol == defn.JavaLangPackageVal then
12821282
defn.FromJavaObjectSymbol.denot
12831283
else
1284-
accessibleDenot(qual.tpe.widenIfUnstable, name, sig, target)
1284+
val qualType = qual.tpe.widenIfUnstable
1285+
if name == nme.CONSTRUCTOR && qual.symbol.is(JavaAnnotation) then
1286+
// #19951 Disregard the signature (or the absence thereof) for constructors of Java annotations
1287+
// Note that Java annotations always have a single public constructor
1288+
qualType.findMember(name, qualType)
1289+
else
1290+
accessibleDenot(qualType, name, sig, target)
12851291
makeSelect(qual, name, denot)
12861292

12871293
def readQualId(): (untpd.Ident, TypeRef) =
@@ -1335,15 +1341,26 @@ class TreeUnpickler(reader: TastyReader,
13351341
readPathTree()
13361342
}
13371343

1338-
/** Adapt constructor calls where class has only using clauses from old to new scheme.
1344+
/** Adapt constructor calls for Java annot constructors and for the new scheme of `using` clauses.
1345+
*
1346+
* #19951 If the `fn` is the constructor of a Java annotation, reorder and refill
1347+
* arguments against the constructor signature. Only reorder if all the arguments
1348+
* are `NamedArg`s, which is always the case if the TASTy was produced by 3.5+.
1349+
* If some arguments are positional, only *add* missing arguments to the right
1350+
* and hope for the best; this will at least fix #19951 after the fact if the new
1351+
* annotation fields are added after all the existing ones.
1352+
*
1353+
* Otherwise, adapt calls where class has only using clauses from old to new scheme.
13391354
* or class has mixed using clauses and other clauses.
13401355
* Old: leading (), new: nothing, or trailing () if all clauses are using clauses.
13411356
* This is neccessary so that we can read pre-3.2 Tasty correctly. There,
13421357
* constructor calls use the old scheme, but constructor definitions already
13431358
* use the new scheme, since they are reconstituted with normalizeIfConstructor.
13441359
*/
13451360
def constructorApply(fn: Tree, args: List[Tree]): Tree =
1346-
if fn.tpe.widen.isContextualMethod && args.isEmpty then
1361+
if fn.symbol.owner.is(JavaAnnotation) then
1362+
tpd.Apply(fn, fixArgsToJavaAnnotConstructor(fn.tpe.widen, args))
1363+
else if fn.tpe.widen.isContextualMethod && args.isEmpty then
13471364
fn.withAttachment(SuppressedApplyToNone, ())
13481365
else
13491366
val fn1 = fn match
@@ -1365,6 +1382,68 @@ class TreeUnpickler(reader: TastyReader,
13651382
res.withAttachment(SuppressedApplyToNone, ())
13661383
else res
13671384

1385+
def fixArgsToJavaAnnotConstructor(methType: Type, args: List[Tree]): List[Tree] =
1386+
methType match
1387+
case methType: MethodType =>
1388+
val formalNames = methType.paramNames
1389+
val sizeCmp = args.sizeCompare(formalNames)
1390+
1391+
def makeDefault(name: TermName, tpe: Type): NamedArg =
1392+
NamedArg(name, Underscore(tpe))
1393+
1394+
def extendOnly(args: List[NamedArg]): List[NamedArg] =
1395+
if sizeCmp < 0 then
1396+
val argsSize = args.size
1397+
val additionalArgs: List[NamedArg] =
1398+
formalNames.drop(argsSize).lazyZip(methType.paramInfos.drop(argsSize)).map(makeDefault(_, _))
1399+
args ::: additionalArgs
1400+
else
1401+
args // fast path
1402+
1403+
if formalNames.isEmpty then
1404+
// fast path
1405+
args
1406+
else if sizeCmp > 0 then
1407+
// Something's wrong anyway; don't touch anything
1408+
args
1409+
else if args.exists(!_.isInstanceOf[NamedArg]) then
1410+
// Pre 3.5 TASTy -- do our best, assuming that args match as a prefix of the formals
1411+
val prefixMatch = args.lazyZip(formalNames).forall {
1412+
case (NamedArg(actualName, _), formalName) => actualName == formalName
1413+
case _ => true
1414+
}
1415+
// If the prefix does not match, something's wrong; don't touch anything
1416+
if !prefixMatch then
1417+
args
1418+
else
1419+
// Turn non-named args to named and extend with defaults
1420+
extendOnly(args.lazyZip(formalNames).map {
1421+
case (arg: NamedArg, _) => arg
1422+
case (arg, formalName) => NamedArg(formalName, arg)
1423+
})
1424+
else
1425+
// Good TASTy where all the arguments are named; reorder and extend if needed
1426+
val namedArgs = args.asInstanceOf[List[NamedArg]]
1427+
val prefixMatch = namedArgs.lazyZip(formalNames).forall((arg, formalName) => arg.name == formalName)
1428+
if prefixMatch then
1429+
// fast path, extend only
1430+
extendOnly(namedArgs)
1431+
else
1432+
// needs reordering, and possibly fill in holes for default arguments
1433+
val argsByName = mutable.AnyRefMap.from(namedArgs.map(arg => arg.name -> arg))
1434+
val reconstructedArgs = formalNames.lazyZip(methType.paramInfos).map { (name, tpe) =>
1435+
argsByName.remove(name).getOrElse(makeDefault(name, tpe))
1436+
}
1437+
if argsByName.nonEmpty then
1438+
// something's wrong; don't touch anything
1439+
args
1440+
else
1441+
reconstructedArgs
1442+
1443+
case _ =>
1444+
args
1445+
end fixArgsToJavaAnnotConstructor
1446+
13681447
def quotedExpr(fn: Tree, args: List[Tree]): Tree =
13691448
val TypeApply(_, targs) = fn: @unchecked
13701449
untpd.Quote(args.head, Nil).withBodyType(targs.head.tpe)
@@ -1491,8 +1570,12 @@ class TreeUnpickler(reader: TastyReader,
14911570
NoDenotation
14921571

14931572
val denot =
1494-
val d = ownerTpe.decl(name).atSignature(sig, target)
1495-
(if !d.exists then lookupInSuper else d).asSeenFrom(prefix)
1573+
if owner.is(JavaAnnotation) && name == nme.CONSTRUCTOR then
1574+
// #19951 Fix up to read TASTy produced before 3.5.0 -- ignore the signature
1575+
ownerTpe.decl(name).asSeenFrom(prefix)
1576+
else
1577+
val d = ownerTpe.decl(name).atSignature(sig, target)
1578+
(if !d.exists then lookupInSuper else d).asSeenFrom(prefix)
14961579

14971580
makeSelect(qual, name, denot)
14981581
case REPEATED =>

0 commit comments

Comments
 (0)