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

Jump to bottom

Open

mberndt123 wants to merge 9 commits into scala:main

from mberndt123:sip-67-strict-equality-improvements

Open

Implement SIP-67: strictEquality pattern matching (fixes #22732) #23803

mberndt123 wants to merge 9 commits into scala:main from mberndt123:sip-67-strict-equality-improvements

+76 −8

Conversation

mberndt123

Copy link

@mberndt123 mberndt123 commented Aug 24, 2025 •

edited

Loading

Hi,

this is my attempt at implementing the SIP-67 specification. Thanks to @noti0na1 and @dwijnand for the hints you gave in #22732. Sorry it took so long, please let me know what you think.

@mberndt123 mberndt123 force-pushed the sip-67-strict-equality-improvements branch from 3bf5df8 to e3d22ab Compare

August 26, 2025 21:14

mberndt123 added 2 commits

August 26, 2025 23:14

@mberndt123


 Implement SIP-67: strictEquality pattern matching (Github issue scala...

5778bea

...#22732)

@mberndt123


 use braceless syntax in SIP67Tests

0a565d0

@mberndt123 mberndt123 force-pushed the sip-67-strict-equality-improvements branch from e3d22ab to 0a565d0 Compare

August 26, 2025 21:14

@Gedochao Gedochao requested review from odersky and noti0na1

August 27, 2025 11:19

@Gedochao Gedochao assigned odersky and noti0na1

Aug 27, 2025

odersky

odersky requested changes

Aug 30, 2025

View reviewed changes

compiler/src/dotty/tools/dotc/typer/Typer.scala Outdated

untpd.Select(untpd.TypedSplice(tree), nme.EQ),

untpd.TypedSplice(dummyTreeOfType(pt)))

typedExpr(cmp, defn.BooleanType)

if ! (tree.tpe <:< pt && (tree.symbol.flags.isAllOf(Flags.EnumValue) || (tree.symbol.flags.isAllOf(Flags.Module | Flags.Case)))) then

Copy link

Contributor

@odersky odersky Aug 30, 2025 •

edited

Loading

Choose a reason for hiding this comment

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

Some remarks:

The motivation for the SIP was that it makes strictEquality easier to use, but this also changes the pattern matching behavior of patterns without that import. And it breaks the law that pattern matching values is done with an == check. I think this goes too far, even though this is what's specified in the SIP.
We cannot implement a SIP change without going through an experimenal language import.
I think it would be better to add code to assumedCanEqual instead. Under strictEquality and the experimental import, if the left operand (or either one) is an enum constant, and the other operand is a supertype, assume the two CanEqual.

Copy link

Author

@mberndt123 mberndt123 Aug 31, 2025 •

edited

Loading

Choose a reason for hiding this comment

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

Hi @odersky,

Thank you for your review.

Regarding 1: 🤔 I'm not sure I understand. The SIP says "The semantics of pattern matching against a constant are otherwise unchanged", meaning that it will still be equivalent to ==, except it doesn't require CanEqual. If that's not what it does, then it's a bug in my implementation. But I think that's solved by moving it to assumeCanEqual as you suggested in point 3, right?

Regarding 2: ✅ I've added an experimental language import, strictEqualityPatternMatching, to enable this behaviour. I also tried adding a Mode for this so it can be enabled with a command line flag similar to -language:strictEquality, but unless I'm misreading the code, the modes are stored in an Int bitmask and all the bits are already taken 😒

Regarding 3: ✅ I've moved the implementation to assumedCanEqual, which was a bit tricky because in that function the Tree isn't available, but I need it to know if it's a case object or enum case. It's also called from Synthesizer where there is no Tree, so I'm passing it as an Option[Tree] now.

@odersky odersky assigned mberndt123 and unassigned odersky

Aug 30, 2025

@mberndt123 mberndt123 requested a review from a team as a code owner

August 31, 2025 20:54

@mberndt123 mberndt123 force-pushed the sip-67-strict-equality-improvements branch 3 times, most recently from 2bb5197 to e9fb60b Compare

August 31, 2025 21:17

mberndt123 added 2 commits

August 31, 2025 23:45

@mberndt123


 change SIP-67 implementation based on @odersky 's review

d433ebe

@mberndt123


 hide SIP-67 behaviour behind experimental feature flag

4f7a9a3

@mberndt123 mberndt123 force-pushed the sip-67-strict-equality-improvements branch from e9fb60b to 4f7a9a3 Compare

August 31, 2025 21:46

@mberndt123 mberndt123 requested a review from odersky

August 31, 2025 22:06

mberndt123 added 3 commits

September 1, 2025 01:00

@mberndt123


 remove excess parens

56f11c2

@mberndt123


 minor refactoring of SIP67Tests

6570fb2

@mberndt123


 whitespace fix

51b34cb

hamzaremmal

hamzaremmal requested changes

Sep 15, 2025

View reviewed changes

library/src/scala/runtime/stdLibPatches/language.scala

Copy link

Member

@hamzaremmal hamzaremmal Sep 15, 2025

Choose a reason for hiding this comment

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

Please add this object in scala.language.experimental too (Not just in the stdLibPatches).

Copy link

Author

@mberndt123 mberndt123 Sep 15, 2025 •

edited

Loading

Choose a reason for hiding this comment

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

Thanks for pointing this out Hamza, I've now added it.

@mberndt123 mberndt123 requested a review from hamzaremmal

September 15, 2025 21:26

@mberndt123


 add strictEqualityPatternMatching in scala.language.experimental

79fc91d

@mberndt123 mberndt123 force-pushed the sip-67-strict-equality-improvements branch from a82ddbd to 79fc91d Compare

September 15, 2025 21:28

@mberndt123


 Merge branch 'main' into sip-67-strict-equality-improvements

45e188c

@mberndt123

Copy link

Author

mberndt123 commented Sep 23, 2025

Hey @Gedochao ,

It's been a while since you assigned this to Martin, and I addressed his feedback as well as @hamzaremmal's. But I have the impression that it wasn't noticed yet, perhaps you could assign it back to @odersky?

@soronpo

Copy link

Contributor

soronpo commented Sep 23, 2025

I will mention the implementation is ready in the upcoming SIP meeting this Friday. It's already on the agenda.

@mberndt123

Copy link

Author

mberndt123 commented Sep 23, 2025

Thanks @soronpo!

@SethTisue SethTisue added this to the 3.8.0 milestone

Sep 24, 2025

@SethTisue

Copy link

Member

SethTisue commented Sep 24, 2025

tentatively milestoning for 3.8.0 since it should least be evaluated for inclusion

odersky

odersky requested changes

Sep 24, 2025

View reviewed changes

Copy link

Contributor

@odersky odersky left a comment

Choose a reason for hiding this comment

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

Otherwise LGTM in the sense that it implements SIP 67 as specced and I believe after the suggested changes are implemented it will fit well with the rest of the compiler.

compiler/src/dotty/tools/dotc/typer/Implicits.scala

* 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 = {

Copy link

Contributor

@odersky odersky Sep 24, 2025

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:

def assumedCanEqual(ltp: Type, rtp: Type, leftTree: Tree = EmptyTree)

compiler/src/dotty/tools/dotc/typer/Implicits.scala

/** 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 =

Copy link

Contributor

@odersky odersky Sep 24, 2025

Choose a reason for hiding this comment

The 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.

compiler/src/dotty/tools/dotc/typer/Implicits.scala

|| 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))

Copy link

Contributor

@odersky odersky Sep 24, 2025

Choose a reason for hiding this comment

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

.flags is redundant, should be omitted. Suggestion:

 if strictEqualityPatternMatching 
 && (leftTree.symbol.isAllOf(Flags.EnumValue) || leftTree.symbol.isAllOf(Flags.Module | Flags.Case))
 then ltp <:< lift(rtp)
 else false

More pedestrian but much clearer and more efficient.

Labels

None yet

6 participants

@mberndt123 @soronpo @SethTisue @odersky @hamzaremmal @noti0na1

Implement SIP-67: strictEquality pattern matching (fixes #22732) #23803

Are you sure you want to change the base?

Implement SIP-67: strictEquality pattern matching (fixes #22732) #23803

Conversation

@mberndt123 mberndt123 commented Aug 24, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

@odersky odersky Aug 30, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

@mberndt123 mberndt123 Aug 31, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

@hamzaremmal hamzaremmal Sep 15, 2025

Choose a reason for hiding this comment

Uh oh!

@mberndt123 mberndt123 Sep 15, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

mberndt123 commented Sep 23, 2025

Uh oh!

soronpo commented Sep 23, 2025

Uh oh!

mberndt123 commented Sep 23, 2025

Uh oh!

SethTisue commented Sep 24, 2025

Uh oh!

@odersky odersky left a comment

Choose a reason for hiding this comment

Uh oh!

@odersky odersky Sep 24, 2025

Choose a reason for hiding this comment

Uh oh!

@odersky odersky Sep 24, 2025

Choose a reason for hiding this comment

Uh oh!

@odersky odersky Sep 24, 2025

Choose a reason for hiding this comment

Uh oh!

Uh oh!

@mberndt123 mberndt123 commented Aug 24, 2025 •

edited

Loading

@odersky odersky Aug 30, 2025 •

edited

Loading

@mberndt123 mberndt123 Aug 31, 2025 •

edited

Loading

@mberndt123 mberndt123 Sep 15, 2025 •

edited

Loading