Skip to content
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

Spurious reachability warning #14407

Closed
griggt opened this issue Feb 3, 2022 · 2 comments · Fixed by #14550
Closed

Spurious reachability warning #14407

griggt opened this issue Feb 3, 2022 · 2 comments · Fixed by #14550
Assignees
Labels
area:pattern-matching itype:bug regression This worked in a previous version but doesn't anymore
Milestone

Comments

@griggt
Copy link
Contributor

griggt commented Feb 3, 2022

Compiler version

3.1.2-RC1, 3.1.3-RC1-bin-20220201-ef25672-NIGHTLY

Minimized code

@main def Test =
  val it = 42
  42L match
    case `it` => println("pass")
    case _    => println("fail") 

Output

sbt:reach> run
[info] compiling 1 Scala source to /tmp/reach/target/scala-3.1.2-RC1/classes ...
[warn] -- [E030] Match case Unreachable Warning: /tmp/reach/reach.scala:4:9 -----------
[warn] 4 |    case `it` => println("pass")
[warn]   |         ^^^^
[warn]   |         Unreachable case
[warn] one warning found
[info] done compiling
[info] running Test 
pass

This issue came up while updating fs2 in the community build in #14395, where -Xfatal-warnings is in use. Can reproduce in fs2 like so:

$ git clone https://github.com/typelevel/fs2.git && cd fs2
$ git rev-parse HEAD
cd280871b516243d120bcd16090f7cb51daddb27
$ sbt "++3.1.2-RC1 coreJVM/compile"
[info] welcome to sbt 1.6.1 (AdoptOpenJDK Java 1.8.0_292)
[info] loading global plugins from /home/tgrigg/.sbt/1.0/plugins
[info] loading settings for project fs2-build from plugins.sbt ...
[info] loading project definition from /tmp/fs2/project
[info] loading settings for project root from build.sbt ...
[info] resolving key references (16343 settings) ...
[info] set current project to root (in build file:/tmp/fs2/)
[info] Setting Scala version to 3.1.2-RC1 on 16 projects.
[info] Reapplying settings...
[info] set current project to root (in build file:/tmp/fs2/)
[info] compiling 37 Scala sources to /tmp/fs2/core/jvm/target/scala-3.1.2-RC1/classes ...
[warn] -- [E030] Match case Unreachable Warning: /tmp/fs2/core/shared/src/main/scala/fs2/Stream.scala:2116:21 
[warn] 2116 |                case `concurrency` => channel.close *> end.complete(()).void
[warn]      |                     ^^^^^^^^^^^^^
[warn]      |                     Unreachable case
[warn] one warning found

where the relevant code is:

val concurrency = if (maxConcurrent == Int.MaxValue) Int.MaxValue else maxConcurrent + 1
val action =
  (
    Semaphore[F2](concurrency.toLong),
    Channel.bounded[F2, F2[Either[Throwable, O2]]](concurrency),
    Deferred[F2, Unit],
    Deferred[F2, Unit]
  ).mapN { (semaphore, channel, stop, end) =>
    val releaseAndCheckCompletion =
      semaphore.release *>
        semaphore.available.flatMap {
          case `concurrency` => channel.close *> end.complete(()).void
          case _             => ().pure[F2]
        }

Note that here the type of concurrency is Int and semaphore.available is F2[Long].

Works as expected on 3.1.1 and earlier

Expectation

No warning.

@griggt griggt added itype:bug area:pattern-matching regression This worked in a previous version but doesn't anymore labels Feb 3, 2022
@smarter smarter added this to the 3.1.2 milestone Feb 5, 2022
@SethTisue
Copy link
Member

SethTisue commented Feb 23, 2022

Relevant: #13290 and its sequel #13485

#13290 fixed the case 42 version of this but the fix needs to be broadened to kick in here too.

Currently Space calls provablyDisjoint, and then the types get widened to Int and Long inside there, and then are rightly considered disjoint, and we're not free to change the behavior of provablyDisjoint, since it's used elsewhere.

Maybe we just need to modify adaptType again (it's used by isSubtype which is used by intersect — all of these methods are inside Space.scala so we're free to modify them). (In #13290 we added convertConstantType.)

But we have to be careful about changing isSubType because it's also used for other purposes within the matcher. We actually already wrote some wip code that does some widening and that fixes the test case, but then breaks a ton of other stuff. For example if we have case 0 => ..., for reachability purposes we definitely can't allow that 0 to be widened to Int — we didn't cover Int, we only covered 0: Int.

@SethTisue
Copy link
Member

Also looking into an alternate solution path where we leave isSubType alone and alter intersectUnrelatedAtomicTypes not to always believe provablyDisjoint's false result if both types being intersected are numeric.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area:pattern-matching itype:bug regression This worked in a previous version but doesn't anymore
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants