Skip to content

Commit

Permalink
Make annotation typechecking lazier
Browse files Browse the repository at this point in the history
Now that `Typer.stabilize` ends up checking for `@uncheckedStable`
annotations to support defs's as stable paths, new opportunities
arise for cyclic errors.

The enclosed test case started to fail since scala#8338 with:

```
|-- object Test BYVALmode-EXPRmode (site: package <empty>)
|    |-- super APPSELmode-EXPRmode-POLYmode-QUALmode (silent: <init> in Test)
|    |    |-- this EXPRmode (silent: <init> in Test)
|    |    |    \-> Test.type
|    |    \-> Test.type
|    |-- def foo BYVALmode-EXPRmode (site: object Test)
|    |    |-- attr EXPRmode (site: method foo in Test)
|    |    |    |-- Int TYPEmode (site: method attr in Test)
|    |    |    |    \-> Int
|    |    |    |-- new anno APPSELmode-BYVALmode-EXPRmode-FUNmode-POLYmode (site: object Test)
|    |    |    |    |-- new anno APPSELmode-EXPRmode-POLYmode-QUALmode (site: object Test)
|    |    |    |    |    |-- anno FUNmode-TYPEmode (site: object Test)
|    |    |    |    |    |    \-> anno
|    |    |    |    |    \-> anno
|    |    |    |    \-> (a: Any): anno
|    |    |    |-- (a: Any): anno : pt=anno EXPRmode (site: value <local Test> in Test)
|    |    |    |    |-- foo : pt=Any BYVALmode-EXPRmode (site: value <local Test> in Test)
|    |    |    |    |    caught scala.reflect.internal.Symbols$CyclicReference: illegal cyclic reference involving method foo: while typing foo
/Users/jz/code/scala/sandbox/test.scala:7: error: recursive method foo needs result type
  @anno(foo) def attr: Int = foo
        ^
|    |    |    |    \-> anno
```

Another variant detailed in scala/bug#12216 fails only on 2.12.x
(after the backport of scala#8338). Backporting this fix will fix
that problem, too.

This reduces the likelihood of cycles by typechecking the only core of
an annotation (_not_ the full application to the annotation argumnets)
to determine its type symbol.
  • Loading branch information
retronym committed Nov 5, 2020
1 parent 65fa331 commit 77a7eba
Show file tree
Hide file tree
Showing 3 changed files with 35 additions and 6 deletions.
23 changes: 18 additions & 5 deletions src/compiler/scala/tools/nsc/typechecker/Namers.scala
Expand Up @@ -1900,11 +1900,24 @@ trait Namers extends MethodSynthesis {
annotations filterNot (_ eq null) map { ann =>
val ctx = typer.context
// need to be lazy, #1782. enteringTyper to allow inferView in annotation args, scala/bug#5892.
AnnotationInfo lazily {
enteringTyper {
val annotSig = newTyper(ctx.makeNonSilent(ann)).typedAnnotation(ann, Some(annotee))
if (pred(annotSig)) annotSig else UnmappableAnnotation // UnmappableAnnotation will be dropped in typedValDef and typedDefDef
}
def computeInfo: AnnotationInfo = enteringTyper {
val annotSig = newTyper(ctx.makeNonSilent(ann)).typedAnnotation(ann, Some(annotee))
if (pred(annotSig)) annotSig else UnmappableAnnotation // UnmappableAnnotation will be dropped in typedValDef and typedDefDef
}
ann match {
case treeInfo.Applied(fun, _, _) =>
def computeSymbol = enteringTyper {
val fun1 = newTyper(ctx.makeNonSilent(fun.duplicate)).typed(fun, context.defaultModeForTyped.forFunMode, WildcardType)
fun1.tpe.finalResultType.typeSymbol
}
AnnotationInfo.lazily(computeSymbol, computeInfo)
case _ =>
AnnotationInfo lazily {
enteringTyper {
val annotSig = newTyper(ctx.makeNonSilent(ann)).typedAnnotation(ann, Some(annotee))
if (pred(annotSig)) annotSig else UnmappableAnnotation // UnmappableAnnotation will be dropped in typedValDef and typedDefDef
}
}
}
}

Expand Down
10 changes: 9 additions & 1 deletion src/reflect/scala/reflect/internal/AnnotationInfos.scala
Expand Up @@ -110,6 +110,9 @@ trait AnnotationInfos extends api.Annotations { self: SymbolTable =>
def lazily(lazyInfo: => AnnotationInfo) =
new LazyAnnotationInfo(lazyInfo)

def lazily(lazySymbol: => Symbol, lazyInfo: => AnnotationInfo) =
new ExtraLazyAnnotationInfo(lazySymbol, lazyInfo)

def apply(atp: Type, args: List[Tree], assocs: List[(Name, ClassfileAnnotArg)]): AnnotationInfo =
new CompleteAnnotationInfo(atp, args, assocs)

Expand Down Expand Up @@ -154,7 +157,7 @@ trait AnnotationInfos extends api.Annotations { self: SymbolTable =>
/** Symbol annotations parsed in `Namer` (typeCompleter of
* definitions) have to be lazy (#1782)
*/
final class LazyAnnotationInfo(lazyInfo: => AnnotationInfo) extends AnnotationInfo {
class LazyAnnotationInfo(lazyInfo: => AnnotationInfo) extends AnnotationInfo {
private[this] var forced = false
private lazy val forcedInfo = try lazyInfo finally forced = true

Expand All @@ -172,6 +175,11 @@ trait AnnotationInfos extends api.Annotations { self: SymbolTable =>
override def completeInfo(): Unit = forcedInfo
}

final class ExtraLazyAnnotationInfo(sym: => Symbol, lazyInfo: => AnnotationInfo) extends LazyAnnotationInfo(lazyInfo) {
private[this] lazy val typeSymbol = sym
override def symbol: Symbol = typeSymbol
}

/** Typed information about an annotation. It can be attached to either
* a symbol or an annotated type.
*
Expand Down
8 changes: 8 additions & 0 deletions test/files/pos/unchecked-stable-cyclic-error.scala
@@ -0,0 +1,8 @@
import scala.annotation.StaticAnnotation

class anno(a: Any) extends StaticAnnotation

object Test {
def foo = attr
@anno(foo) def attr: Int = foo
}

0 comments on commit 77a7eba

Please sign in to comment.