Skip to content

[backport] Make annotation typechecking lazier#9310

Merged
retronym merged 1 commit intoscala:2.12.xfrom
retronym:backport/extra-lazy-annotation-info
Nov 10, 2020
Merged

[backport] Make annotation typechecking lazier#9310
retronym merged 1 commit intoscala:2.12.xfrom
retronym:backport/extra-lazy-annotation-info

Conversation

@retronym
Copy link
Member

@retronym retronym commented Nov 10, 2020

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 #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 #8338). Backporting this fix makes that test
start passing on 2.12.x.

Backports #9302

Fixes scala/bug#12216

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.

Backports scala#9302
@scala-jenkins scala-jenkins added this to the 2.12.14 milestone Nov 10, 2020
@retronym retronym modified the milestones: 2.12.14, 2.12.13 Nov 10, 2020
@retronym
Copy link
Member Author

The Travis CI build is failing with Java SDK installation (network issues?), but Jenkins is happy.

@retronym retronym merged commit 76f197e into scala:2.12.x Nov 10, 2020
@SethTisue
Copy link
Member

The Travis CI build is failing with Java SDK installation (network issues?)

the mergely build failed too, twice:
scala/scala-dev#738

* definitions) have to be lazy (#1782)
*/
final class LazyAnnotationInfo(lazyInfo: => AnnotationInfo) extends AnnotationInfo {
class LazyAnnotationInfo(lazyInfo: => AnnotationInfo) extends AnnotationInfo {
Copy link
Member

Choose a reason for hiding this comment

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

Could be sealed aka final--.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants