Skip to content

false positive warning #23119

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
mr-git opened this issue May 7, 2025 · 3 comments · May be fixed by #23121
Open

false positive warning #23119

mr-git opened this issue May 7, 2025 · 3 comments · May be fixed by #23121
Assignees
Labels
area:linting Linting warnings enabled with -W or -Xlint itype:bug

Comments

@mr-git
Copy link

mr-git commented May 7, 2025

Compiler version

3.7.0

Minimized example

@main
def make: IndexedSeq[FalsePositive] =
  for {
    i <- 1 to 2
    given Int = i
    fp = FalsePositive()
  } yield fp

class FalsePositive(using Int):
  def usage(): Unit =
    println(summon[Int])

Output Error/Warning message

[warn] -- [E198] Unused Symbol Warning: /<path>/main/src/main/scala/FalsePositive.scala:5:4 
[warn] 5 |    given Int = i
[warn]   |    ^^^^^^^^^
[warn]   |    unused pattern variable
[warn] one warning found

Why this Error/Warning was not helpful

The message was unhelpful because the given Int is actually used - this is a "false positive".

Suggested improvement

The issue is with how code is structured in for-comprehension. If for-comprehension gets restructured to:

  for {
    i <- 1 to 2
    given Int = i
  } yield FalsePositive() // <=== notice that initialization happens without intermediate variable

The warning is not triggered any more!

@mr-git mr-git added itype:enhancement area:reporting Error reporting including formatting, implicit suggestions, etc stat:needs triage Every issue needs to have an "area" and "itype" label better-errors Issues concerned with improving confusing/unhelpful diagnostic messages labels May 7, 2025
@mr-git
Copy link
Author

mr-git commented May 7, 2025

Forgot to mention - the issue is triggered starting with 3.7.0!

3.6.4 compiles both forms just fine, both with and without "-language:experimental.betterFors" flag.

Could it be related to some sugar-improvements for for-comprehensions in 3.7.0? I remember reading about reduction of .map calls...

@som-snytt
Copy link
Contributor

som-snytt commented May 7, 2025

3.6 doesn't track pattern variables. Thanks for the report!

Also thanks for the hint, I'll look at interactions with the different for desugaring; the lint just relies on correct positions.

@som-snytt som-snytt self-assigned this May 7, 2025
@som-snytt som-snytt added itype:bug area:linting Linting warnings enabled with -W or -Xlint and removed itype:enhancement area:reporting Error reporting including formatting, implicit suggestions, etc stat:needs triage Every issue needs to have an "area" and "itype" label better-errors Issues concerned with improving confusing/unhelpful diagnostic messages labels May 7, 2025
@som-snytt
Copy link
Contributor

som-snytt commented May 7, 2025

betterFors is enabled under 3.7 with the flag -preview, TIL.

There is no false positive under -preview, curiously enough. I see that eliminates the tupling step, which appears to desugar the same as 3.6.4.

The problem is the desugaring of given Int = i which is not a simple assignment but should be the same as j: Int = i. (The reason it's not is you can't write case given s: Int by analogy to given val s: Int.)

@som-snytt som-snytt linked a pull request May 8, 2025 that will close this issue
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area:linting Linting warnings enabled with -W or -Xlint itype:bug
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants