-
Notifications
You must be signed in to change notification settings - Fork 1.1k
Implement SIP-67: strictEquality pattern matching (fixes #22732) #23803
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
Changes from 9 commits
5778bea
0a565d0
d433ebe
4f7a9a3
56f11c2
6570fb2
51b34cb
79fc91d
45e188c
f23deba
341e927
684663a
5f62655
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -84,6 +84,9 @@ object Implicits: | |
def strictEquality(using Context): Boolean = | ||
ctx.mode.is(Mode.StrictEquality) || Feature.enabled(nme.strictEquality) | ||
|
||
def strictEqualityPatternMatching(using Context): Boolean = | ||
Feature.enabled(Feature.strictEqualityPatternMatching) | ||
|
||
|
||
/** A common base class of contextual implicits and of-type implicits which | ||
* represents a set of references to implicit definitions. | ||
|
@@ -1039,7 +1042,7 @@ trait Implicits: | |
* - if one of T, U is a subtype of the lifted version of the other, | ||
* unless strict equality is set. | ||
*/ | ||
def assumedCanEqual(ltp: Type, rtp: Type)(using Context) = { | ||
def assumedCanEqual(leftTreeOption: Option[Tree], ltp: Type, rtp: Type)(using Context): Boolean = { | ||
// Map all non-opaque abstract types to their upper bound. | ||
// This is done to check whether such types might plausibly be comparable to each other. | ||
val lift = new TypeMap { | ||
|
@@ -1062,15 +1065,20 @@ trait Implicits: | |
|
||
ltp.isError | ||
|| rtp.isError | ||
|| !strictEquality && (ltp <:< lift(rtp) || rtp <:< lift(ltp)) | ||
|| locally: | ||
if strictEquality then | ||
strictEqualityPatternMatching && leftTreeOption.exists: leftTree => | ||
ltp <:< lift(rtp) && (leftTree.symbol.flags.isAllOf(Flags.EnumValue) || leftTree.symbol.flags.isAllOf(Flags.Module | Flags.Case)) | ||
|
||
else | ||
(ltp <:< lift(rtp) || rtp <:< lift(ltp)) | ||
} | ||
|
||
/** Check that equality tests between types `ltp` and `rtp` make sense */ | ||
def checkCanEqual(ltp: Type, rtp: Type, span: Span)(using Context): Unit = | ||
if (!ctx.isAfterTyper && !assumedCanEqual(ltp, rtp)) { | ||
def checkCanEqual(left: Tree, rtp: Type, span: Span)(using Context): Unit = | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. ltp is not part of the API anymore, so the doc comment needs to be updated. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Thanks, I've updated the comment |
||
val ltp = left.tpe.widen | ||
if !ctx.isAfterTyper && !assumedCanEqual(Some(left), ltp, rtp) then | ||
val res = implicitArgTree(defn.CanEqualClass.typeRef.appliedTo(ltp, rtp), span) | ||
implicits.println(i"CanEqual witness found for $ltp / $rtp: $res: ${res.tpe}") | ||
} | ||
|
||
object hasSkolem extends TreeAccumulator[Boolean]: | ||
def apply(x: Boolean, tree: Tree)(using Context): Boolean = | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,44 @@ | ||
package dotty.tools.dotc.typer | ||
|
||
import dotty.tools.DottyTest | ||
import dotty.tools.dotc.core.Contexts.* | ||
|
||
import org.junit.Test | ||
import org.junit.Assert.fail | ||
|
||
class SIP67Tests extends DottyTest: | ||
|
||
private def checkNoErrors(source: String): Unit = | ||
val ctx = checkCompile("typer", source)((_, _) => ()) | ||
if ctx.reporter.hasErrors then | ||
fail("Unexpected compilation errors were reported") | ||
|
||
@Test | ||
def sip67test1: Unit = | ||
checkNoErrors: | ||
""" | ||
import scala.language.strictEquality | ||
import scala.language.experimental.strictEqualityPatternMatching | ||
enum Foo: | ||
case Bar | ||
|
||
val _ = | ||
(??? : Foo) match | ||
case Foo.Bar => | ||
""" | ||
@Test | ||
def sip67test2: Unit = | ||
checkNoErrors: | ||
""" | ||
import scala.language.strictEquality | ||
import scala.language.experimental.strictEqualityPatternMatching | ||
|
||
sealed trait Foo | ||
|
||
object Foo: | ||
case object Bar extends Foo | ||
|
||
val _ = | ||
(??? : Foo) match | ||
case Foo.Bar => | ||
""" |
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Please add this object in There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Thanks for pointing this out Hamza, I've now added it. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We usually use EmptyTree for missing trees instead of an option type. Also, leftTree should be the last argument. Suggestion:
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Done, thanks 👍🏻