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.

This PR reduces the likelihood of cycles by typechecking the only core of
an annotation (_not_ the full application to the annotation arguments)
to determine its type symbol.

pos/unchecked-stable-cyclic-error.scala 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, pos/annotation-cycle.scala, fails only on 2.12.x
(after the backport of scala#8338). Backporting this fix makes that test
start passing on 2.12.x.

(cherry picked from commit 9d9635d)
  • Loading branch information
retronym committed Nov 9, 2020
1 parent 42f2bd8 commit 5def565
Show file tree
Hide file tree
Showing 4 changed files with 45 additions and 6 deletions.
26 changes: 21 additions & 5 deletions src/compiler/scala/tools/nsc/typechecker/Namers.scala
Expand Up @@ -1883,11 +1883,27 @@ 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(Select(New(tpt), _), _, _) =>
def computeSymbol = enteringTyper {
val tptCopy = tpt.duplicate
val silentTyper = newTyper(ctx.makeSilent(newtree = tptCopy))
// Discard errors here, we'll report them in `computeInfo`.
val tpt1 = silentTyper.typedTypeConstructor(tptCopy)
tpt1.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 @@ -175,6 +175,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 @@ -219,7 +222,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 var forced = false
private lazy val forcedInfo = try lazyInfo finally forced = true

Expand All @@ -237,6 +240,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
7 changes: 7 additions & 0 deletions test/files/pos/annotation-cycle.scala
@@ -0,0 +1,7 @@
import scala.annotation.StaticAnnotation

class anno(attr: Boolean) extends StaticAnnotation

object Test {
@anno(attr = true) def attr: Int = 1
}
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 5def565

Please sign in to comment.