Skip to content

Avoid trivially true branch for the last arm of an exhaustive match - #27070

Draft
SolalPirelli wants to merge 1 commit into
scala:mainfrom
dotty-staging:solal/better-patmat-last-arm
Draft

SolalPirelli wants to merge 1 commit into
scala:mainfrom
dotty-staging:solal/better-patmat-last-arm

Conversation

@SolalPirelli

@SolalPirelli SolalPirelli commented Sep 14, 2026

Copy link
Copy Markdown
Contributor

Inspired by a commit in Haoyi's #26365

We can already prove a match is exhaustive thanks to the space engine, so let's use that fact.

Have you relied on LLM-based tools in this contribution?

No

How was the solution tested?

New automated tests (including the issue's reproducer, if applicable)

Comment thread tests/warn/i16899.scala

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

File moved since this was not a deep-subtype test, and it was using -Werror to pretend to be a pos test, but it's a warn test.

Comment thread tests/warn/i25100.scala
i match {
case Bar => println("i is a Bar")
case i: Foo => println("i is a Foo")
case Bar => println("i is a Long")

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This file checks that the CanEqual import for Bar and Foo is used (i.e., doesn't warn as unused), so changing the order of the comparison is necessary to make sure it's actually used here.

Comment thread tests/neg/i24789.scala
@@ -1,10 +1,8 @@
//> using options -Werror -explain

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

These tests already produced errors, having nopos-errors with -Werror in addition to that is just odd.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The ticket was something about there being two diagnostics, so this documents the warning.

It looks odd because the test rig doesn't have a way to do both warns and errors.


def f2(o: Option[O.X]) = o match
case None => 0
case Some(x: O.X) => 1 // ditto in reverse order

@SolalPirelli SolalPirelli Sep 14, 2026

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

regression test for a bug introduced while developing this (found in benchmarks)

def main(args: Array[String]): Unit =
f(E.Left(42))
f(E.Right(42))
f(E.Both(42, 0))

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

regression test for a bug introduced while developing this (found in cats)

@SolalPirelli
SolalPirelli force-pushed the solal/better-patmat-last-arm branch from 752252d to 218b497 Compare September 14, 2026 14:47
toRemove += prod
handleInputs(prod, prod.asInstanceOf[MultiANewArrayInsnNode].dims)

// Remove "is instance of j.l.Object", a leftover after some optimizations

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

See LocalOptimizationBytecodeTests changes below to understand what this PR achieves; this change here is needed so that some temporary type tests get eliminated properly in our motivating examples.

Comment thread tests/neg/i8681.scala
case B(_) => "OK"
case C(_) => "NOT OK" // error
}
// nopos-error: No warnings can be incurred under -Werror (or -Xfatal-warnings)

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Same comment about the comment.

@dottybot

Copy link
Copy Markdown
Member

Benchmarks started. Workflow run.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants