scala3 icon indicating copy to clipboard operation
scala3 copied to clipboard

Fix Closure span assignment in makeClosure

Open Florian3k opened this issue 3 years ago • 0 comments

Fixes #15098

Wrong line numbers were coming from Closure. Previously it's span was inherited from block end position, now it's assigned explicitly.

This fix changes behaviour of test tests/neg/i9299.scala:

type F <: F = 1 match { // error
  case _ => foo.foo // error // error
}
def foo(a: Int): Unit = ???

Previously there were 3 errors generated:

Compiler output before fix (3 errors)
-- Error: tests/neg/i9299.scala:1:10 -------------------------------------------
1 |type F <: F = 1 match { // error
  |          ^
  |Recursion limit exceeded.
  |Maybe there is an illegal cyclic reference?
  |If that's not the case, you could also try to increase the stacksize using the -Xss JVM option.
  |A recurring operation is (inner to outer):
  |
  |  type parameters of F
  |  type parameters of  <: F
  |  type parameters of F
  |  type parameters of  <: F
  |  type parameters of F
  |  type parameters of  <: F
  |  type parameters of F
  |  type parameters of  <: F
  |  type parameters of F
  |  type parameters of  <: F
  |  ...
  |
  |  type parameters of  <: F
  |  type parameters of F
  |  type parameters of  <: F
  |  type parameters of F
  |  type parameters of  <: F
  |  type parameters of F
  |  type parameters of  <: F
  |  type parameters of F
  |  type parameters of  <: F
  |  type parameters of F
-- Error: tests/neg/i9299.scala:2:12 -------------------------------------------
2 |  case _ => foo.foo // error // error
  |            ^
  |Recursion limit exceeded.
  |Maybe there is an illegal cyclic reference?
  |If that's not the case, you could also try to increase the stacksize using the -Xss JVM option.
  |A recurring operation is (inner to outer):
  |
  |  base classes of F
  |  base classes of F
  |  base classes of F
  |  base classes of F
  |  base classes of F
  |  base classes of F
  |  base classes of F
  |  base classes of F
  |  base classes of F
  |  base classes of F
  |  ...
  |
  |  base classes of F
  |  base classes of F
  |  base classes of F
  |  base classes of F
  |  base classes of F
  |  base classes of F
  |  base classes of F
  |  base classes of F
  |  base classes of F
  |  base classes of  <: F
-- [E046] Cyclic Error: tests/neg/i9299.scala:2:15 -----------------------------
2 |  case _ => foo.foo // error // error
  |               ^
  |               Cyclic reference involving method $anonfun
  |-----------------------------------------------------------------------------
  | Explanation (enabled by `-explain`)
  |- - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - -
  | method $anonfun is declared as part of a cycle which makes it impossible for the
  | compiler to decide upon $anonfun's type.
  | To avoid this error, try giving $anonfun an explicit type.
   -----------------------------------------------------------------------------
3 errors found

Now there are 2 errors:

Compiler output after fix (2 errors)
-- Error: tests/neg/i9299.scala:1:10 -------------------------------------------
1 |type F <: F = 1 match { // error
  |          ^
  |Recursion limit exceeded.
  |Maybe there is an illegal cyclic reference?
  |If that's not the case, you could also try to increase the stacksize using the -Xss JVM option.
  |A recurring operation is (inner to outer):
  |
  |  type parameters of F
  |  type parameters of  <: F
  |  type parameters of F
  |  type parameters of  <: F
  |  type parameters of F
  |  type parameters of  <: F
  |  type parameters of F
  |  type parameters of  <: F
  |  type parameters of F
  |  type parameters of  <: F
  |  ...
  |
  |  type parameters of  <: F
  |  type parameters of F
  |  type parameters of  <: F
  |  type parameters of F
  |  type parameters of  <: F
  |  type parameters of F
  |  type parameters of  <: F
  |  type parameters of F
  |  type parameters of  <: F
  |  type parameters of F
-- Error: tests/neg/i9299.scala:2:12 -------------------------------------------
2 |  case _ => foo.foo // error // error
  |            ^
  |Recursion limit exceeded.
  |Maybe there is an illegal cyclic reference?
  |If that's not the case, you could also try to increase the stacksize using the -Xss JVM option.
  |A recurring operation is (inner to outer):
  |
  |  base classes of F
  |  base classes of F
  |  base classes of F
  |  base classes of F
  |  base classes of F
  |  base classes of F
  |  base classes of F
  |  base classes of F
  |  base classes of F
  |  base classes of F
  |  ...
  |
  |  base classes of F
  |  base classes of F
  |  base classes of F
  |  base classes of F
  |  base classes of F
  |  base classes of F
  |  base classes of F
  |  base classes of F
  |  base classes of F
  |  base classes of  <: F
2 errors found

This is because now in function UniqueMessagePositions.isHidden:

trait UniqueMessagePositions extends Reporter {
  // ...
  /** 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 =
    super.isHidden(dia)
    ||
      dia.pos.exists
      && !ctx.settings.YshowSuppressedErrors.value
      && (dia.pos.start to dia.pos.end).exists(pos =>
            positions.get((ctx.source, pos)).exists(_.hides(dia)))
  // ...
}

this fragment:

(dia.pos.start to dia.pos.end).exists(pos =>
    positions.get((ctx.source, pos)).exists(_.hides(dia)))

evaluates to true with third error. New position is considered as overlapping with position of previous error and it's not printed.

I think this may be desired behaviour, but I am not entirely sure.

Florian3k avatar Aug 10 '22 13:08 Florian3k