Skip to content

Commit 8c91e97

Browse files
mbovelmbovel
committed
Move checks to class Checking, handle boundType explicitly
1 parent 94a69db commit 8c91e97

File tree

5 files changed

+159
-88
lines changed

5 files changed

+159
-88
lines changed

‎compiler/src/dotty/tools/dotc/transform/TreeChecker.scala‎

Lines changed: 11 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -69,9 +69,6 @@ class TreeChecker extends Phase with SymTransformer {
6969
def transformSym(symd: SymDenotation)(using Context): SymDenotation = {
7070
val sym = symd.symbol
7171

72-
if symd.isCompleted then
73-
Checking.checkWellFormedType(symd.info)
74-
7572
if (sym.isClass && !sym.isAbsent()) {
7673
val validSuperclass = sym.isPrimitiveValueClass || defn.syntheticCoreClasses.contains(sym) ||
7774
(sym eq defn.ObjectClass) || sym.isOneOf(NoSuperClassFlags) || (sym.asClass.superClass.exists) ||
@@ -830,8 +827,17 @@ object TreeChecker {
830827
|${mismatch.message}${mismatch.explanation}
831828
|tree = $tree ${tree.className}""".stripMargin
832829
})
833-
Checking.checkWellFormedType(tp1)
834-
Checking.checkWellFormedType(tp2)
830+
checkWellFormedType(tp1)
831+
checkWellFormedType(tp2)
832+
833+
/** Check that the type `tp` is well-formed. Currently this only means
834+
* checking that annotated types have valid annotation arguments.
835+
*/
836+
private def checkWellFormedType(tp: Type)(using Context): Unit =
837+
tp.foreachPart:
838+
case AnnotatedType(underlying, annot) => checkAnnot(annot.tree)
839+
case _ => ()
840+
835841
}
836842

837843
/** Tree checker that can be applied to a local tree. */

‎compiler/src/dotty/tools/dotc/typer/Checking.scala‎

Lines changed: 74 additions & 79 deletions
Original file line numberDiff line numberDiff line change
@@ -604,17 +604,6 @@ object Checking {
604604
checkNoConflict(Lazy, ParamAccessor, em"parameter may not be `lazy`")
605605
}
606606

607-
/** Check that the type `tp` is well-formed. Currently this only means
608-
* checking that annotated types have valid annotation arguments.
609-
*/
610-
def checkWellFormedType(tp: Type)(using Context) =
611-
tp.foreachPart:
612-
case AnnotatedType(underlying, annot) =>
613-
// Not checking `checkAnnotClass` because `Transform.toTypeTree` creates
614-
// `Annotated` whose trees are not annotation instantiations.
615-
checkAnnotArgs(annot.tree)
616-
case _ => ()
617-
618607
/** Check for illegal or redundant modifiers on modules. This is done separately
619608
* from checkWellformed, since the original module modifiers don't surivive desugaring
620609
*/
@@ -925,73 +914,6 @@ object Checking {
925914
annot
926915
case _ => annot
927916
end checkNamedArgumentForJavaAnnotation
928-
929-
/** Check arguments of annotations */
930-
private def checkAnnotArgs(tree: Tree)(using Context): Tree =
931-
val cls = Annotations.annotClass(tree)
932-
tree match
933-
case Apply(tycon, arg :: Nil) if cls == defn.TargetNameAnnot =>
934-
arg match
935-
case Literal(Constant("")) =>
936-
report.error(em"target name cannot be empty", arg.srcPos)
937-
case Literal(_) => // ok
938-
case _ =>
939-
report.error(em"@${cls.name} needs a string literal as argument", arg.srcPos)
940-
case _ =>
941-
if cls.isRetainsLike then () // Do not check @retain annotations
942-
else if cls == defn.ThrowsAnnot then
943-
// Do not check @throws annotations.
944-
// TODO(mbovel): in tests/run/t6380.scala, an annotation tree is
945-
// `new throws[Exception](throws.<init>[Exception])`. What is this?
946-
()
947-
else
948-
tpd.allTermArguments(tree).foreach(checkAnnotArg)
949-
tree
950-
951-
private def checkAnnotArg(tree: Tree)(using Context): Unit =
952-
def isTupleModule(sym: Symbol): Boolean =
953-
ctx.definitions.isTupleClass(sym.companionClass)
954-
955-
def isFunctionAllowed(t: Tree): Boolean =
956-
t match
957-
case Select(qual, nme.apply) =>
958-
qual.symbol == defn.ArrayModule
959-
|| qual.symbol == defn.ClassTagModule // class tags are used as arguments to Array.apply
960-
|| qual.symbol == defn.SymbolModule // used in Akka
961-
|| qual.symbol == defn.JSSymbolModule // used in Scala.js
962-
|| isTupleModule(qual.symbol)
963-
case Select(New(clazz), nme.CONSTRUCTOR) => clazz.symbol.isAnnotation
964-
case Apply(fun, _) => isFunctionAllowed(fun)
965-
case TypeApply(fun, _) => isFunctionAllowed(fun)
966-
case _ => false
967-
968-
def valid(t: Tree): Boolean =
969-
t.tpe.isEffectivelySingleton
970-
|| (
971-
t match
972-
case Literal(_) => true
973-
// `_` is used as placeholder for unspecified arguments of Java
974-
// annotations. Example: tests/run/java-ann-super-class
975-
case Ident(nme.WILDCARD) => true
976-
case Apply(fun, args) => isFunctionAllowed(fun) && args.forall(valid)
977-
case TypeApply(fun, args) => isFunctionAllowed(fun)
978-
// Support for `x.isInstanceOf[T]`. Probably not needed.
979-
//case TypeApply(meth @ Select(arg, _), _) if meth.symbol == defn.Any_asInstanceOf => valid(arg)
980-
case SeqLiteral(elems, _) => elems.forall(valid)
981-
case Typed(expr, _) => valid(expr)
982-
case NamedArg(_, arg) => valid(arg)
983-
case Splice(_) => true
984-
case Hole(_, _, _, _) => true
985-
case _ => false
986-
)
987-
988-
if !valid(tree) then
989-
report.error(
990-
i"""Implementation restriction: not a valid annotation argument.
991-
| Argument: $tree
992-
| Type: ${tree.tpe}""",
993-
tree.srcPos
994-
)
995917
}
996918

997919
trait Checking {
@@ -1463,7 +1385,13 @@ trait Checking {
14631385
report.error(em"$what can only be used in an inline method", pos)
14641386

14651387
def checkAnnot(tree: Tree)(using Context): Tree =
1466-
Checking.checkAnnotArgs(checkAnnotClass(tree))
1388+
tree match
1389+
case Ident(tpnme.BOUNDTYPE_ANNOT) =>
1390+
// `FirstTransform.toTypeTree` creates `Annotated` nodes whose `annot` are
1391+
// `Ident`s, not annotation instances. See `tests/pos/annot-boundtype.scala`.
1392+
tree
1393+
case _ =>
1394+
checkAnnotArgs(checkAnnotClass(tree))
14671395

14681396
/** Check that the class corresponding to this tree is either a Scala or Java annotation.
14691397
*
@@ -1481,6 +1409,73 @@ trait Checking {
14811409
else if !cls.derivesFrom(defn.AnnotationClass) then
14821410
errorTree(tree, em"$cls is not a valid Scala annotation: it does not extend `scala.annotation.Annotation`")
14831411
else tree
1412+
1413+
/** Check arguments of annotations */
1414+
private def checkAnnotArgs(tree: Tree)(using Context): Tree =
1415+
val cls = Annotations.annotClass(tree)
1416+
tree match
1417+
case Apply(tycon, arg :: Nil) if cls == defn.TargetNameAnnot =>
1418+
arg match
1419+
case Literal(Constant("")) =>
1420+
report.error(em"target name cannot be empty", arg.srcPos)
1421+
case Literal(_) => // ok
1422+
case _ =>
1423+
report.error(em"@${cls.name} needs a string literal as argument", arg.srcPos)
1424+
case _ =>
1425+
if cls.isRetainsLike then () // Do not check @retain annotations
1426+
else if cls == defn.ThrowsAnnot then
1427+
// Do not check @throws annotations.
1428+
// TODO(mbovel): in tests/run/t6380.scala, an annotation tree is
1429+
// `new throws[Exception](throws.<init>[Exception])`. What is this?
1430+
()
1431+
else
1432+
tpd.allTermArguments(tree).foreach(checkAnnotArg)
1433+
tree
1434+
1435+
private def checkAnnotArg(tree: Tree)(using Context): Unit =
1436+
def isTupleModule(sym: Symbol): Boolean =
1437+
ctx.definitions.isTupleClass(sym.companionClass)
1438+
1439+
def isFunctionAllowed(t: Tree): Boolean =
1440+
t match
1441+
case Select(qual, nme.apply) =>
1442+
qual.symbol == defn.ArrayModule
1443+
|| qual.symbol == defn.ClassTagModule // class tags are used as arguments to Array.apply
1444+
|| qual.symbol == defn.SymbolModule // used in Akka
1445+
|| qual.symbol == defn.JSSymbolModule // used in Scala.js
1446+
|| isTupleModule(qual.symbol)
1447+
case Select(New(clazz), nme.CONSTRUCTOR) => clazz.symbol.isAnnotation
1448+
case Apply(fun, _) => isFunctionAllowed(fun)
1449+
case TypeApply(fun, _) => isFunctionAllowed(fun)
1450+
case _ => false
1451+
1452+
def valid(t: Tree): Boolean =
1453+
t.tpe.isEffectivelySingleton
1454+
|| (
1455+
t match
1456+
case Literal(_) => true
1457+
// `_` is used as placeholder for unspecified arguments of Java
1458+
// annotations. Example: tests/run/java-ann-super-class
1459+
case Ident(nme.WILDCARD) => true
1460+
case Apply(fun, args) => isFunctionAllowed(fun) && args.forall(valid)
1461+
case TypeApply(fun, args) => isFunctionAllowed(fun)
1462+
// Support for `x.isInstanceOf[T]`. Probably not needed.
1463+
//case TypeApply(meth @ Select(arg, _), _) if meth.symbol == defn.Any_asInstanceOf => valid(arg)
1464+
case SeqLiteral(elems, _) => elems.forall(valid)
1465+
case Typed(expr, _) => valid(expr)
1466+
case NamedArg(_, arg) => valid(arg)
1467+
case Splice(_) => true
1468+
case Hole(_, _, _, _) => true
1469+
case _ => false
1470+
)
1471+
1472+
if !valid(tree) then
1473+
report.error(
1474+
i"""Implementation restriction: not a valid annotation argument.
1475+
| Argument: $tree
1476+
| Type: ${tree.tpe}""",
1477+
tree.srcPos
1478+
)
14841479

14851480
/** 1. Check that all case classes that extend `scala.reflect.Enum` are `enum` cases
14861481
* 2. Check that parameterised `enum` cases do not extend java.lang.Enum.

‎tests/neg/annot-invalid.check‎

Lines changed: 54 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,54 @@
1+
-- Error: tests/neg/annot-invalid.scala:7:21 ---------------------------------------------------------------------------
2+
7 | val x1: Int @annot(n + 1) = 0 // error
3+
| ^^^^^
4+
| Implementation restriction: not a valid annotation argument.
5+
| Argument: n.+(1)
6+
| Type: Int
7+
-- Error: tests/neg/annot-invalid.scala:8:22 ---------------------------------------------------------------------------
8+
8 | val x2: Int @annot(f(2)) = 0 // error
9+
| ^^^^
10+
| Implementation restriction: not a valid annotation argument.
11+
| Argument: f(2)
12+
| Type: Unit
13+
-- Error: tests/neg/annot-invalid.scala:9:21 ---------------------------------------------------------------------------
14+
9 | val x3: Int @annot(throw new Error()) = 0 // error
15+
| ^^^^^^^^^^^^^^^^^
16+
| Implementation restriction: not a valid annotation argument.
17+
| Argument: throw new Error()
18+
| Type: Nothing
19+
-- Error: tests/neg/annot-invalid.scala:10:21 --------------------------------------------------------------------------
20+
10 | val x4: Int @annot((x: Int) => x) = 0 // error
21+
| ^^^^^^^^^^^^^
22+
| Implementation restriction: not a valid annotation argument.
23+
| Argument: {
24+
| def $anonfun(x: Int): Int = x
25+
| closure($anonfun)
26+
| }
27+
| Type: Int => Int
28+
-- Error: tests/neg/annot-invalid.scala:12:9 ---------------------------------------------------------------------------
29+
12 | @annot(n + 1) val y1: Int = 0 // error
30+
| ^^^^^
31+
| Implementation restriction: not a valid annotation argument.
32+
| Argument: n.+(1)
33+
| Type: Int
34+
-- Error: tests/neg/annot-invalid.scala:13:10 --------------------------------------------------------------------------
35+
13 | @annot(f(2)) val y2: Int = 0 // error
36+
| ^^^^
37+
| Implementation restriction: not a valid annotation argument.
38+
| Argument: f(2)
39+
| Type: Unit
40+
-- Error: tests/neg/annot-invalid.scala:14:9 ---------------------------------------------------------------------------
41+
14 | @annot(throw new Error()) val y3: Int = 0 // error
42+
| ^^^^^^^^^^^^^^^^^
43+
| Implementation restriction: not a valid annotation argument.
44+
| Argument: throw new Error()
45+
| Type: Nothing
46+
-- Error: tests/neg/annot-invalid.scala:15:9 ---------------------------------------------------------------------------
47+
15 | @annot((x: Int) => x) val y4: Int = 0 // error
48+
| ^^^^^^^^^^^^^
49+
| Implementation restriction: not a valid annotation argument.
50+
| Argument: {
51+
| def $anonfun(x: Int): Int = x
52+
| closure($anonfun)
53+
| }
54+
| Type: Int => Int

‎tests/neg/annot-invalid.scala‎

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -9,9 +9,9 @@ def main =
99
val x3: Int @annot(throw new Error()) = 0 // error
1010
val x4: Int @annot((x: Int) => x) = 0 // error
1111

12-
@annot(m1(2)) val y1: Int = 0 // error
13-
@annot(throw new Error()) val y2: Int = 0 // error
14-
@annot((x: Int) => x) val y3: Int = 0 // error
15-
@annot(x + 1) val y4: Int = 0 // error
12+
@annot(n + 1) val y1: Int = 0 // error
13+
@annot(f(2)) val y2: Int = 0 // error
14+
@annot(throw new Error()) val y3: Int = 0 // error
15+
@annot((x: Int) => x) val y4: Int = 0 // error
1616

1717
()

‎tests/pos/annot-boundtype.scala‎

Lines changed: 16 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,16 @@
1+
// `FirstTransform.toTypeTree` creates `Annotated` nodes whose `annot` are
2+
// `Ident`s, not annotation instances. This is relevant for `Checking.checkAnnot`.
3+
//
4+
// See also:
5+
// - tests/run/t2755.scala
6+
// - tests/neg/i13044.scala
7+
8+
def f(a: Array[?]) =
9+
a match
10+
case x: Array[?] => ()
11+
12+
def f2(t: Tuple) =
13+
t match
14+
case _: (t *: ts) => ()
15+
case _ => ()
16+

0 commit comments

Comments
 (0)