Fix Closure span assignment in makeClosure by Florian3k · Pull Request #15841 · scala/scala3

Florian3k · 2022-08-10T13:19:54Z

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.

ckipp01 · 2023-05-09T18:04:38Z

I see a bunch of performance requests here, @mbovel was there performance concerns here, or what should we do about this pr?

mbovel · 2023-05-09T18:06:29Z

@ckipp01 no, not at all, I was just testing the benchmarks bot. Sorry for the spam, should have done it on an empty PR 😅

ckipp01 · 2023-05-09T18:07:32Z

@ckipp01 no, not at all, I was just testing the benchmarks bot. Sorry for the spam, should have done it on an empty PR 😅

No worries! I'll re-trigger the CI here and see if we can move it along.

EDIT: actually I don't see the option to even do that since it's so old. @Florian3k do you mind rebasing this, and we can try to get it across the finish line?

Florian3k · 2023-05-10T08:31:00Z

@ckipp01 The story behind this PR is that my fix did not pass some tests in completions. I tried to fix it, but without success.

OndrejSpanel · 2024-03-20T14:04:32Z

What will be the fate of this issue? Unreliable debugger variables on last lambda line are quite unfortunate.

See also https://youtrack.jetbrains.com/issue/SCL-22145 and https://youtrack.jetbrains.com/issue/SCL-22144

This is something even users using no advanced language features will hit, as lambdas are everywhere. (For-comprehensions anyone?)

Florian3k · 2024-03-20T18:29:31Z

We recently got back to this, and I think we're close to fixing this (thanks to @rochala for helping with completions).
I don't want to make any promises, but hopefully next week 🤞

OndrejSpanel · 2024-05-15T13:39:19Z

Could this be backported to 3.3.4? The issue makes debugging any code with lambdas, esp. single line lambdas, very hard.

Fixes #15098 [Cherry-picked c8c3bde][modified]

Backports #15841 to the LTS branch. PR submitted by the release tooling. [skip ci]

Florian3k requested a review from dwijnand August 10, 2022 13:23

KacperFKorban assigned dwijnand Aug 15, 2022

dwijnand approved these changes Aug 15, 2022

View reviewed changes

dwijnand assigned Florian3k and unassigned dwijnand Aug 15, 2022

This comment was marked as off-topic.

Sign in to view

Florian3k force-pushed the fix-15098 branch from b93523d to a382793 Compare May 10, 2023 08:25

This comment was marked as off-topic.

Sign in to view

scala deleted a comment from dottybot Jul 13, 2023

Florian3k force-pushed the fix-15098 branch from a382793 to 5c830d8 Compare March 22, 2024 11:07

Florian3k force-pushed the fix-15098 branch from 5c830d8 to 4ab2c8f Compare March 25, 2024 09:42

Fix Closure span assignment in makeClosure

0a67b15

Florian3k force-pushed the fix-15098 branch from 4ab2c8f to 0a67b15 Compare March 25, 2024 11:08

Florian3k requested a review from rochala March 25, 2024 14:10

rochala approved these changes Mar 29, 2024

View reviewed changes

Florian3k merged commit c8c3bde into scala:main Mar 29, 2024

OndrejSpanel mentioned this pull request Apr 2, 2024

3.4.0 debugging info not reliable in inner lambdas #20067

Closed

Kordyjan added this to the 3.5.0 milestone May 10, 2024

WojciechMazur added a commit that referenced this pull request Jul 4, 2024

Fix Closure span assignment in makeClosure (#15841)

50a844d

Fixes #15098 [Cherry-picked c8c3bde][modified]

WojciechMazur mentioned this pull request Jul 4, 2024

Backport "Fix Closure span assignment in makeClosure" to LTS #21027

Merged

WojciechMazur added a commit that referenced this pull request Jul 5, 2024

Backport "Fix Closure span assignment in makeClosure" to LTS (#21027)

ab57803

Backports #15841 to the LTS branch. PR submitted by the release tooling. [skip ci]

Conversation

Florian3k commented Aug 10, 2022

Uh oh!

This comment was marked as off-topic.

This comment was marked as off-topic.

This comment was marked as off-topic.

This comment was marked as off-topic.

This comment was marked as off-topic.

This comment was marked as off-topic.

This comment was marked as off-topic.

This comment was marked as off-topic.

This comment was marked as off-topic.

This comment was marked as off-topic.

This comment was marked as off-topic.

ckipp01 commented May 9, 2023

Uh oh!

mbovel commented May 9, 2023

Uh oh!

ckipp01 commented May 9, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Florian3k commented May 10, 2023

Uh oh!

This comment was marked as off-topic.

This comment was marked as off-topic.

OndrejSpanel commented Mar 20, 2024

Uh oh!

Florian3k commented Mar 20, 2024

Uh oh!

OndrejSpanel commented May 15, 2024

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

8 participants

ckipp01 commented May 9, 2023 •

edited

Loading