Simplify reporters #8338

lrytz · 2019-08-15T10:54:56Z

Messages are counted in the reporter, not in the Severity objects.
The Severity class is moved out of the reporter to the package, with
an @uncheckedStable trick to make pattern matching on severities
source-compatbile (Fixes scala/bug#8908).

ContextReporter no longer extends Reporter. The ContextReporter is
only used in the context, and it reports through Global.reporter. But
it's never assigned to Global.reporter. So better keep the two
separate. This also saves the inherited fields (ContextReporters are
allocated manies, type checking every Apply uses silent).

Remove most reporter mixins to get to a linear inheritance structure,
no more delegating reporters.

Ignore the force parameter of info0.

Restores support for -Xprompt in the repl.

Some quirks to remain source compatible with the sbt compiler interface.
nsc.reporters.Reporter only exists for sbt compatibility, Global
has a FilteringReporter. -Xreporter must be a FilteringReporter.

lrytz · 2019-08-15T11:12:39Z

cc @ghik. Probably we need to open up the API a little to allow for more overrides. Perhaps ForwardingReporter (a reporter that delegates) should come back, but as a subclass of FilteringReporter so that it can be assigned to Global.reporter. Added a ForwardingReporter.

cc @som-snytt let me know what you think. I think preferring internal.Reporter in the compiler is an illusion. This PR makes it explicit that Global.reporter supports limits and does position filtering.

This can be handy for retaining source compatibility. Or for when you just know better. Fixes scala/bug#8908

ContextReporter no longer extends Reporter. The ContextReporter is only used in the context, and it reports through Global.reporter. But it's never assigned to Global.reporter. So better keep the two separate. This also saves the inherited fields (ContextReporters are allocated manies, type checking every Apply uses silent). Remove most reporter mixins to get to a linear inheritance structure, no more delegating reporters. Ignore the `force` parameter of `info0`. Some quirks to remain source compatible with the sbt compiler interface. `nsc.reporters.Reporter` only exists for sbt compatibility, Global has a `FilteringReporter`. `-Xreporter` must be a FilteringReporter.

adriaanm

Nice cleanup! A few small comments.

adriaanm · 2019-08-19T08:25:10Z

src/compiler/scala/tools/nsc/PipelineMain.scala

@@ -566,7 +566,7 @@ class PipelineMainClass(argFiles: Seq[Path], pipelineSettings: PipelineMain.Pipe
              diagnostic.getKind match {
                case Kind.ERROR => reporter.error(position, msg)
                case Kind.WARNING | Kind.MANDATORY_WARNING => reporter.warning(position, msg)


For future reference, should this distinction be captured in the FilteringReporter interface?

Is the question whether we should have multiple kinds of warnings? I personally think simpler is better.

src/compiler/scala/tools/nsc/reporters/ForwardingReporter.scala

adriaanm · 2019-08-19T08:51:29Z

src/compiler/scala/tools/nsc/reporters/ForwardingReporter.scala

+  *   global.reporter = myReporter
+  * }}}
+  */
+class ForwardingReporter(delegate: FilteringReporter) extends FilteringReporter {


Can/should we seal this class?

It's not actually used or subclassed in the compiler, the idea is to provide it as a utility to compiler plugins like silencer, maybe other tools.

The thing I was going to try today was how easy to customize. I remember the whole point of blowing the functionality into traits was that the old AbstractReporter was too rigid.

OK, let me know!

som-snytt · 2019-08-20T06:57:50Z

If you could leave this open another day, I may have comments. I unsquashed the last commit so I could follow along.

src/compiler/scala/tools/nsc/reporters/ForwardingReporter.scala

som-snytt · 2019-08-21T02:03:58Z

src/compiler/scala/tools/nsc/Global.scala

@@ -85,20 +86,24 @@ class Global(var currentSettings: Settings, reporter0: LegacyReporter)

  override def settings = currentSettings

-  private[this] var currentReporter: LegacyReporter = { reporter = reporter0 ; currentReporter }


I guess you didn't like this trick. BTW, this usage of locally is what inspired the recent PR. I just took this up again, and sbt tells me that this is where I left off...

you didn't like this trick

Not that I didn't like it, it made me smile

som-snytt · 2019-08-22T03:45:13Z

src/compiler/scala/tools/nsc/reporters/Reporter.scala

+  *   - positions (only one error at a position, no duplicate messages on a position)
+  */
+abstract class FilteringReporter extends Reporter {
+  def settings: Settings


It seemed obvious that FilteringReporter should take settings as a ctor param, as all reporters are required to have that ctor sig, but InteractiveReporter has an abstract compiler from which it derives settings.

I was literally just thinking, Nobody has to muck with settings at construction time, but:

/** * Called before instantiating the presentation compiler's instance. * You should provide an implementation of this method if you need * to customize the `settings` used to instantiate the presentation compiler. * */ protected def prepareSettings(settings: Settings): Unit = ()

Well, how about you take the compiler as an arg and they can do all their mucking about before they call you?

FilteringReporter should take settings as a ctor param

Either way is fine with me, but I don't see a reason to prefer one or the other.

It's a clear signal or reminder to customizers that you must provide this constructor. Otherwise it's just documented somewhere, or not documented. I did spend a minute wondering if there is a reason to have a preference.

I didn't mean to persuade you!

I'm not sure you did :-) But as I said, either way is fine with me.

[edit. reverted because]

Also it caused

+Caused by: java.lang.NullPointerException + at scala.tools.nsc.interactive.tests.InteractiveTestSettings.prepareSettings(InteractiveTestSettings.scala:50) + at scala.tools.nsc.interactive.tests.InteractiveTestSettings.prepareSettings$(InteractiveTestSettings.scala:39) + at scala.tools.nsc.interactive.tests.InteractiveTest.prepareSettings(InteractiveTest.scala:50) + at scala.tools.nsc.interactive.tests.core.PresentationCompilerInstance.compiler(PresentationCompilerInstance.scala:28) + at scala.tools.nsc.interactive.tests.core.PresentationCompilerInstance.compiler$(PresentationCompilerInstance.scala:27) + at scala.tools.nsc.interactive.tests.InteractiveTest.compiler$lzycompute(InteractiveTest.scala:50) + at scala.tools.nsc.interactive.tests.InteractiveTest.compiler(InteractiveTest.scala:50) + at scala.tools.nsc.interactive.tests.core.PresentationCompilerInstance.$init$(PresentationCompilerInstance.scala:23) + at scala.tools.nsc.interactive.tests.InteractiveTest.<init>(InteractiveTest.scala:51) + at Test$.<init>(Test.scala:3) + at Test$.<clinit>(Test.scala:3)

som-snytt · 2019-08-22T19:54:36Z

src/compiler/scala/tools/reflect/package.scala

      case API_WARNING => reporter.warning(info.pos, info.msg)
      case API_ERROR => reporter.error(info.pos, info.msg)
    }

    def interactive(): Unit = reporter match {
-      case reporter: AbstractReporter => reporter.displayPrompt()
+      case reporter: PrintReporter => reporter.displayPrompt()


It's lunch time, just confirming that interactive was intentionally removed.

Yes, i missed this one. The method is unused.

lrytz · 2019-08-23T07:32:41Z

I'll go through the diff between your and my PR and push (probably most of) the changes here, then we can merge?

lrytz · 2019-08-23T14:01:09Z

@adriaanm / @szeiger good to go for 2.13.1?

adriaanm · 2019-08-26T08:19:56Z

I'll let Stefan decide, but good to go for me.

som-snytt · 2019-08-30T17:51:30Z

I was too busy to notice this went in. Thanks!

@anno

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.

@anno

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 argumnets) 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.

@anno

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 argumnets) 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.

@anno

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 argumnets) 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.

@anno

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 argumnets) 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 db2b8f6)

@anno

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.

@anno

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.

@anno

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)

@anno

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)

@anno

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

Problem ======= Upgrade source to https://github.com/scala/scala/releases/tag/v2.12.13 from https://github.com/scala/scala/releases/tag/v2.12.12. This upgrade includes a highlighted feature of configurable warnings and errors (https://www.scala-lang.org/2021/01/12/configuring-and-suppressing-warnings.html / scala/scala#9248). Other noticable changes our code based will experience * Exhaustivity warnings for patterns involving tuples * Better type checking for `CharSequence` arguments scala/scala#9292 * Simplified Reporters scala/scala#8338 * Promotion of `-deprecation` to `-Xlint:deprecation` scala/scala#7714 * Improves performance of building `immutable.{TreeMap,TreeSet}` by using mutation within the builder scala/scala#8794 Full list of changes can be found at https://github.com/scala/scala/pulls?q=is%3Amerged+milestone%3A2.12.13+ Solution ======== * Bump `BUILD` file `SCALA_REV` && `SCALAC_REV_FOR_DEPS`. * Depdnencies which are not built for scala 2.12.13 needed to be bumped `SEMANTICDB_PLUGIN_REV` && `SEMANTICDB_REV` && `SCALAFIX_REV`. * Include `-S-Xlint:-deprecation` to `pants.ini` preventing build failures on deprecated annotations (existing behavior) * Bump `cache_key_gen_version` in `pants.ini` for newly built artifacts on different scala version. * Removed scalafix plugin (`scalac-profiling`) which is not built for 2.12.13. `scalac-profiling` code looks abandoned by ~3 years. * Updated all failing tests that could have depended or expected ordered sequences when the sequence was generated from non ordered collections. Notes ===== It seems a few tests and golden files in source have depended or tested against a stable sort order when sorted order is not guaranteed. This has been seen in a few places such as output json objects (map key fields), code that uses `groupBy` for rekeying results to the user and transforming into sequences, strings built from sequences or maps that are not guaranteed to be ordered. The following PR are related to this change in expectations scala/scala#8794 scala/scala#8948 scala/scala#9218 scala/scala#9376 scala/scala#9091 scala/scala#9216 We took the liberty updating tests with what the current map order would be or updating the test in a way that wouldn't depend on order. Since we may not fully understand all the context of the tests we are hoping this either signals to the owners that there might be some issue with assumed order or signal that upgrading might break implementations due to bug in scala 2.12.13. {D611202} will improve the files changed for workflow builder outside of this change. Please feel to reach out directly and discuss the changes here or bring up issues with this upgrade. Slack [[https://app.slack.com/client/T86S8GHEG/C01NZAFRLFK|#scala-upgrade-2-12-13]] JIRA Issues: SCALA-25 Differential Revision: https://phabricator.twitter.biz/D607826

scala-jenkins added this to the 2.13.1 milestone Aug 15, 2019

lrytz added 2 commits August 15, 2019 14:35

Allow uncheckedStable on defs, vars

ee18631

This can be handy for retaining source compatibility. Or for when you just know better. Fixes scala/bug#8908

Move Severity out of Reporter

9ced2d8

lrytz force-pushed the slim-reporters branch from d5dc915 to 573d697 Compare August 15, 2019 12:35

lrytz force-pushed the slim-reporters branch from 573d697 to 5f6b28c Compare August 16, 2019 11:48

adriaanm approved these changes Aug 19, 2019

View reviewed changes

mkeskells reviewed Aug 20, 2019

View reviewed changes

src/compiler/scala/tools/nsc/reporters/ForwardingReporter.scala Show resolved Hide resolved

som-snytt reviewed Aug 21, 2019

View reviewed changes

som-snytt reviewed Aug 22, 2019

View reviewed changes

lrytz force-pushed the slim-reporters branch from 7d10848 to aa16cbc Compare August 23, 2019 06:13

Review feedback

b13d671

lrytz force-pushed the slim-reporters branch from aa16cbc to b13d671 Compare August 23, 2019 07:02

lrytz force-pushed the slim-reporters branch 2 times, most recently from 2ccb12d to 0ff56aa Compare August 23, 2019 12:13

Changes from som's branch

3e7e0ab

lrytz force-pushed the slim-reporters branch from 0ff56aa to 3e7e0ab Compare August 23, 2019 13:14

This was referenced Aug 26, 2019

Wconf flag for configurable warnings [ci: last-only #8372

Closed

add -Wconf flag for configurable warnings, @nowarn annotation for local suppression #8373

Merged

szeiger merged commit 34d3acb into scala:2.13.x Aug 27, 2019

SethTisue added the release-notes worth highlighting in next release notes label Aug 30, 2019

hrhino mentioned this pull request Jan 31, 2020

2.13.1 scalac StackOverflowError when annotation argument references the annottee scala/bug#11870

Closed

ghik mentioned this pull request Feb 17, 2020

Silencer plugin suppresses compilation errors when scalafix semantic db is enabled ghik/silencer#43

Open

ghik mentioned this pull request Feb 17, 2020

Use delegating reporter error/warn/info count scalameta/scalameta#1505

Closed

dwijnand mentioned this pull request Oct 16, 2020

[2.12] Configurable warnings (backported) #9248

Merged

12 tasks

retronym mentioned this pull request Nov 4, 2020

uncheckedStable backport to 2.12.x caused regression in annotations scala/bug#12216

Closed

retronym mentioned this pull request Nov 5, 2020

Make annotation typechecking lazier #9302

Merged

retronym mentioned this pull request Nov 5, 2020

Make annotation typechecking lazier retronym/scala#106

Closed

retronym mentioned this pull request Nov 10, 2020

[backport] Make annotation typechecking lazier #9310

Merged

SethTisue mentioned this pull request Oct 23, 2023

scala.tools.nsc.reporters.StoreReporter: force logic is reverted scala/bug#6866

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Simplify reporters #8338

Simplify reporters #8338

lrytz commented Aug 15, 2019 •

edited

lrytz commented Aug 15, 2019 •

edited

adriaanm left a comment

adriaanm Aug 19, 2019

lrytz Aug 22, 2019

adriaanm Aug 19, 2019

lrytz Aug 22, 2019

som-snytt Aug 22, 2019

lrytz Aug 22, 2019

som-snytt commented Aug 20, 2019

som-snytt Aug 21, 2019

lrytz Aug 22, 2019

som-snytt Aug 22, 2019

som-snytt Aug 22, 2019

lrytz Aug 22, 2019

som-snytt Aug 22, 2019

som-snytt Aug 23, 2019

lrytz Aug 23, 2019 •

edited

som-snytt Aug 22, 2019

lrytz Aug 22, 2019

lrytz commented Aug 23, 2019 •

edited

lrytz commented Aug 23, 2019

adriaanm commented Aug 26, 2019

som-snytt commented Aug 30, 2019

		@@ -85,20 +86,24 @@ class Global(var currentSettings: Settings, reporter0: LegacyReporter)

		override def settings = currentSettings

		private[this] var currentReporter: LegacyReporter = { reporter = reporter0 ; currentReporter }

Simplify reporters #8338

Simplify reporters #8338

Conversation

lrytz commented Aug 15, 2019 • edited

lrytz commented Aug 15, 2019 • edited

adriaanm left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

som-snytt commented Aug 20, 2019

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

lrytz Aug 23, 2019 • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

lrytz commented Aug 23, 2019 • edited

lrytz commented Aug 23, 2019

adriaanm commented Aug 26, 2019

som-snytt commented Aug 30, 2019

lrytz commented Aug 15, 2019 •

edited

lrytz commented Aug 15, 2019 •

edited

lrytz Aug 23, 2019 •

edited

lrytz commented Aug 23, 2019 •

edited