From 3de42be39fbd168286e05b51ddd4481491a47a7a Mon Sep 17 00:00:00 2001 From: odersky Date: Mon, 4 Apr 2022 20:04:12 +0200 Subject: [PATCH] Revise duplicate diagnostics logic The logic that hid duplicate messages was fragile and overly aggressive. - Because of mixin ordering, it could be that a non-sensical message that was in the end not reported could prevent a message at overlapping positions to be hidden. This could mean that no errors at all were reported even through the program was erroneous. - A message that ended up to be hidden since it overlapped with another message could still hide further messages with which it overlapped as well. Fixing the logic meant that some tests have more error messages reported. Fixes #14834 Fixes #12457 --- .../dotty/tools/dotc/reporting/Reporter.scala | 3 ++ .../reporting/UniqueMessagePositions.scala | 31 ++++++++++--------- .../erased/tupled-function-instances.scala | 2 +- tests/neg/i12457.scala | 3 ++ tests/neg/i14834.scala | 2 ++ tests/neg/i6056.scala | 2 +- tests/neg/i7818.scala | 2 +- tests/neg/i9328.scala | 2 +- tests/neg/parser-stability-17.scala | 2 +- tests/neg/parser-stability-9.scala | 2 +- tests/neg/t5702-neg-bad-and-wild.check | 8 ++++- tests/neg/t5702-neg-bad-and-wild.scala | 2 +- 12 files changed, 39 insertions(+), 22 deletions(-) create mode 100644 tests/neg/i12457.scala create mode 100644 tests/neg/i14834.scala diff --git a/compiler/src/dotty/tools/dotc/reporting/Reporter.scala b/compiler/src/dotty/tools/dotc/reporting/Reporter.scala index a5e6af1d0b60..1d629110a400 100644 --- a/compiler/src/dotty/tools/dotc/reporting/Reporter.scala +++ b/compiler/src/dotty/tools/dotc/reporting/Reporter.scala @@ -153,6 +153,7 @@ abstract class Reporter extends interfaces.ReporterResult { case w: Warning if ctx.settings.XfatalWarnings.value => w.toError case _ => dia if !isHidden(d) then // avoid isHidden test for summarized warnings so that message is not forced + markReported(d) withMode(Mode.Printing)(doReport(d)) d match { case _: Warning => _warningCount += 1 @@ -236,6 +237,8 @@ abstract class Reporter extends interfaces.ReporterResult { def isHidden(dia: Diagnostic)(using Context): Boolean = ctx.mode.is(Mode.Printing) + def markReported(dia: Diagnostic)(using Context): Unit = () + /** Does this reporter contain errors that have yet to be reported by its outer reporter ? * Note: this is always false when there is no outer reporter. */ diff --git a/compiler/src/dotty/tools/dotc/reporting/UniqueMessagePositions.scala b/compiler/src/dotty/tools/dotc/reporting/UniqueMessagePositions.scala index d745d8d20081..98fd7da3032a 100644 --- a/compiler/src/dotty/tools/dotc/reporting/UniqueMessagePositions.scala +++ b/compiler/src/dotty/tools/dotc/reporting/UniqueMessagePositions.scala @@ -12,24 +12,27 @@ trait UniqueMessagePositions extends Reporter { private val positions = new mutable.HashMap[(SourceFile, Integer), Diagnostic] + extension (dia1: Diagnostic) + private def hides(dia2: Diagnostic)(using Context): Boolean = + if dia2.msg.showAlways then dia1.msg.getClass == dia2.msg.getClass + else dia1.level >= dia2.level + /** Logs a position and returns true if it was already logged. * @note Two positions are considered identical for logging if they have the same point. */ override def isHidden(dia: Diagnostic)(using Context): Boolean = - extension (dia1: Diagnostic) def hides(dia2: Diagnostic): Boolean = - if dia2.msg.showAlways then dia1.msg.getClass == dia2.msg.getClass - else dia1.level >= dia2.level - super.isHidden(dia) || { + super.isHidden(dia) + || dia.pos.exists && !ctx.settings.YshowSuppressedErrors.value - && { - var shouldHide = false - for (pos <- dia.pos.start to dia.pos.end) - positions get (ctx.source, pos) match { - case Some(dia1) if dia1.hides(dia) => shouldHide = true - case _ => positions((ctx.source, pos)) = dia - } - shouldHide - } - } + && (dia.pos.start to dia.pos.end).exists(pos => + positions.get((ctx.source, pos)).exists(_.hides(dia))) + + override def markReported(dia: Diagnostic)(using Context): Unit = + if dia.pos.exists then + for (pos <- dia.pos.start to dia.pos.end) + positions.get(ctx.source, pos) match + case Some(dia1) if dia1.hides(dia) => + case _ => positions((ctx.source, pos)) = dia + super.markReported(dia) } diff --git a/tests/neg-custom-args/erased/tupled-function-instances.scala b/tests/neg-custom-args/erased/tupled-function-instances.scala index 6994463fb84a..3574125285e4 100644 --- a/tests/neg-custom-args/erased/tupled-function-instances.scala +++ b/tests/neg-custom-args/erased/tupled-function-instances.scala @@ -4,7 +4,7 @@ object Test { type T type R - summon[TupledFunction[(erased T) => R, erased Tuple1[T] => R]] // error + summon[TupledFunction[(erased T) => R, erased Tuple1[T] => R]] // error // error summon[TupledFunction[(erased T, T) => R, (erased (T, T)) => R]] // error summon[TupledFunction[(erased T, T, T) => R,(erased (T, T, T)) => R]] // error summon[TupledFunction[(erased T, T, T, T) => R,(erased (T, T, T, T)) => R]] // error diff --git a/tests/neg/i12457.scala b/tests/neg/i12457.scala new file mode 100644 index 000000000000..8ebc061d6d59 --- /dev/null +++ b/tests/neg/i12457.scala @@ -0,0 +1,3 @@ +import language.`3.1-migration` + +trait X [ X <: Z , Z >: X [ R ] ] // error diff --git a/tests/neg/i14834.scala b/tests/neg/i14834.scala new file mode 100644 index 000000000000..ae49cb39d0b4 --- /dev/null +++ b/tests/neg/i14834.scala @@ -0,0 +1,2 @@ +type F[_] = A +type A = F[?] // error: cyclic diff --git a/tests/neg/i6056.scala b/tests/neg/i6056.scala index ee8c6dab6c87..dc836e319770 100644 --- a/tests/neg/i6056.scala +++ b/tests/neg/i6056.scala @@ -2,6 +2,6 @@ object i0{ import i0.i0 // error def i0={ import _ // error - import + import // error } // error } \ No newline at end of file diff --git a/tests/neg/i7818.scala b/tests/neg/i7818.scala index 78cbee506784..1dc243cfeedc 100644 --- a/tests/neg/i7818.scala +++ b/tests/neg/i7818.scala @@ -1 +1 @@ -def foo = (x: @) => () // error \ No newline at end of file +def foo = (x: @) => () // error // error \ No newline at end of file diff --git a/tests/neg/i9328.scala b/tests/neg/i9328.scala index 3fb748a44888..dabde498e1dc 100644 --- a/tests/neg/i9328.scala +++ b/tests/neg/i9328.scala @@ -7,7 +7,7 @@ class Foo2[T <: Id[T]] // error // error object Foo { // error object Foo { } - Foo { } + Foo { } // error } implicit class Foo(a: Float) // error case class Foo() diff --git a/tests/neg/parser-stability-17.scala b/tests/neg/parser-stability-17.scala index ff603a677378..db1f212fc4ab 100644 --- a/tests/neg/parser-stability-17.scala +++ b/tests/neg/parser-stability-17.scala @@ -1,2 +1,2 @@ trait x0[] { x0: x0 => } // error // error - class x0[x1] extends x0[x0 x0] x2 x0 // error // error + class x0[x1] extends x0[x0 x0] x2 x0 // error // error // error diff --git a/tests/neg/parser-stability-9.scala b/tests/neg/parser-stability-9.scala index 932f6a15ad52..aaa77f216f37 100644 --- a/tests/neg/parser-stability-9.scala +++ b/tests/neg/parser-stability-9.scala @@ -1,2 +1,2 @@ -import +import // error // error \ No newline at end of file diff --git a/tests/neg/t5702-neg-bad-and-wild.check b/tests/neg/t5702-neg-bad-and-wild.check index 72b3d952241a..623bf3002cca 100644 --- a/tests/neg/t5702-neg-bad-and-wild.check +++ b/tests/neg/t5702-neg-bad-and-wild.check @@ -27,7 +27,7 @@ | | longer explanation available when compiling with `-explain` -- [E032] Syntax Error: tests/neg/t5702-neg-bad-and-wild.scala:23:17 --------------------------------------------------- -23 | val K(ns @ _*, xx) = k // error: pattern expected +23 | val K(ns @ _*, xx) = k // error: pattern expected // error | ^ | pattern expected | @@ -50,3 +50,9 @@ | Not found: * | | longer explanation available when compiling with `-explain` +-- [E045] Cyclic Error: tests/neg/t5702-neg-bad-and-wild.scala:23:19 --------------------------------------------------- +23 | val K(ns @ _*, xx) = k // error: pattern expected // error + | ^ + | Recursive value $1$ needs type + | + | longer explanation available when compiling with `-explain` diff --git a/tests/neg/t5702-neg-bad-and-wild.scala b/tests/neg/t5702-neg-bad-and-wild.scala index d9fec80cf493..95d00c270e89 100644 --- a/tests/neg/t5702-neg-bad-and-wild.scala +++ b/tests/neg/t5702-neg-bad-and-wild.scala @@ -20,7 +20,7 @@ object Test { // good syntax, bad semantics, detected by typer //gowild.scala:14: error: star patterns must correspond with varargs parameters val K(x @ _*) = k - val K(ns @ _*, xx) = k // error: pattern expected + val K(ns @ _*, xx) = k // error: pattern expected // error val K(x) = k // error: x is already defined as value x val (b, _ * ) = (5,6) // ok // no longer complains