strictEqualityPatternMatching: does it work for you?

Scala Improvement Process
1

Hey,

the strictEqualityPatternMatching feature was recently released in Scala 3.8

As the author of this feature, I would now like to collect some feedback about it in order to ascertain whether it succeeded in its goal, a significant reduction of the friction encountered when enabling strictEquality. If so, hopefully it can be turned on by default in the future.
I would much appreciate it if you could take a minute to answer some or all of these questions.

  • How many errors do you get when you add -language:strictEquality to your scala compiler flags?
  • How many of these disappear when you additionally enable -language:experimental.strictEqualityPatternMatching?
  • are there any cases of pattern matching that you feel should compile but don’t (because of strictEquality)?
  • are there any cases of pattern matching that now compile but in your opinion shouldn’t? If so, why?
  • anything else you want to add?

Thanks for reading and commenting!

3 Likes
2

There is a typo in the flag, should be:

-language:experimental.strictEqualityPatternMatching (with a dot)

below are the number of compiler errors for two of the submodules in a project I work with

submodule LOC strict equality pattern matching errors without flag after fixing errors with flag
R 3000 45 34 0
A 3700 124 26 7

Types of errors that still remain after the pattern matching flag:

def empty: A
extension (value: A) def isEmpty: Boolean = value == empty

Values of types A and A cannot be compared with == or != [25:47]

val btm = bottoms.productElement(i)
if btm == null

Values of types Any and Null cannot be compared with == or !=

enum Permission:
    case ALLOW
    case PARTIAL

if permission == ALLOW 


Values of types rdts.filters.Permission and rdts.filters.Permission cannot be compared with == or !=

      (transaction: A) match
          case Some => 

Values of types object Some and A cannot be compared with == or !=

(This code is just WTF and an actual bug found by strict equals)

.find(_.proposal == p)

Values of types rdts.protocols.old.simplified.ProposalNum and rdts.protocols.old.simplified.ProposalNum cannot be compared with == or !=

Is strict equality supposed to prevent == between non-generic instances of the same case class? The ProposalNum case class does have a parameter with a CanEquals instance.

if observer != null then

Values of types (reactives.core.Tracing.Data => Unit) | Null and Null cannot be compared with == or !=


            tree.asExpr match
                case '{ (${ x }: MacroAccess[?]).value }

No given instance of type scala.quoted.Quotes was found.
I found:

    MacroLego.this.quotes

But given instance quotes in class MacroLego does not match type scala.quoted.Quotes.

(MacroLego.this.quotes is explicitly typed as scala.quoted.Quotes)

  sealed private trait Result[+R]
  private object Await             extends Result[Nothing]

    res match {
      case Await =>

Values of types object reactives.locking.Keychains.Retry and reactives.locking.Keychains.Result[reactives.locking.Key[ParRPInterTurn]] cannot be compared with == or !=

I guess the change only works for enums, not sealed hierarchies?

current != Symbol.noSymbol && current != defn.RootPackage && current != defn.RootClass

Values of types quotes.reflect.Symbol and quotes.reflect.Symbol cannot be compared with == or !=

I guess this is more of the “these are the same type” but now in the standard library.

I then went ahead and fixed/worked around the above problems in subproject A by adding CanEquals either to algebraic datatypes, or just importing reflexive CanEquals for local types. This left me with 7 errors that are addressed by strictEqualityPatternMatching.
Fixing those required adding 3 more “derives CanEqual” to some ADT definitions.

So for this project, I think that the primary issue is that it prevents comparisons of values that have the exact same static type as far as I can tell.
The strictEqualityPatternMatching seems to prevent these errors for pattern matching, but any use of == instead of pattern matching (which we do use for all but 3 relevant types) requires the CanEquals instance anyway, and then the added value of the flag is lost.

Addendum. I now also fixed the remaining errors in subproject R. Those were also all either A | Null == Null or comparing two things that have the exact same type (particularly annoying for those in the scala.reflect.Quotes API). And adding the CanEquals instances where possible did also address all additional errors prevented by strictEqualityPatternMatching.

Overall, strict equality still seems half baked, the pattern matching changes feels like it improves usability, for one case. But I would really need most of the cases addressed for strict equality to feel like its worth it.

2 Likes
3

Hi Ragnar,

thank you for your response, this is very useful information.

Do I read this correctly that strictEqualityPatternMatchingsolved 11 out of 45 error messages (24%) in submodule R and 98 out of 124 (79%) in module A? That is 64% overall, which is not a bad result IMO.

Regarding comparisons of singletons/enum cases with ==: this was observed before in this thread, shoutout to @dontgitit. Since you’re already the second person to point this out, it does seem to be a real issue, and it should be fairly simple to implement. I support that change, but I think it can be dealt with in a separate SIP to avoid slowing down strictEqualityPatternMatching (which I hope to get approved for 3.9).

The problem is that just checking if two types have the same static type is not very safe

42 == "foo" // obviously shouldn't compile
val a: Any = 42
val b: Any = "foo"
a == b // would compile with the "same static type" rule; I don't think it should

And the compiler will eagerly “upcast” values if that helps making things compile. So it was decided that the clearest and safest approach is to just make things explicit: if you want to make comparisons, add derives CanEqual.
It has been argued that a CanEqual instance should be available by default for case classes and/or enums, but there are good reasons to be sceptical about that. For one thing, you can have things in your case classes that cannot be meaningfully compared, like functions. And having some kind of fully automatic typeclass derivation is a whole other can of worms, it’s unfortunately much messier than it seems.

Actually it works for both. The feature was originally designed to allow pattern matching on ADTs. Since Scala 2 doesn’t have ADTs but encodes them as sealed hierarchies of case classes and case objects, that’s what I designed the feature to support: case objects – add a case modifier and you’re good to go.

Arguably we could just drop that requirement because it doesn’t really matter if it has a case modifier. This would be in line with the fact that for instance Mirror.SumOf doesn’t care if the objects in your ADT have a case modifier or not. I think I’ll amend the SIP accordingly, thanks for pointing it out!

Yes, the standard library definitely needs more CanEqual instances! Fortunately, it was recently opened for improvements and I will definitely push for more CanEqual instances.

Given that null is not considered idiomatic Scala, I’m not convinced this needs to be addressed at the language level, also because we have alternatives:

  • use an unboxed Option type that uses null under the hood
    • in fact I think one should be added to the standard library
  • use eq to perform comparisons against null
    • we might want to add a compiler hint/quickfix to remind people it’s there

The only way to get there is one step at a time, and strictEqualityPatternMatching is one of the steps.

2 Likes
4

Yes. However, after working around all the things that still need to be worked around, its only the 0 and 7 errors that remain.
Essentially, of the 98 errors that are fixed in subproject A, all of them are a couple of ADTs that need a CanEquals instance, which is addressed by adding a derives CanEqual.

Overall, I think this SIP makes sense and is an improvement. Especially in a codebase without == checks on the same ADTs that you pattern match, this will avoid the derives CanEqual boilerplate.

Adding the == handling would improve the value of this SIP a lot, but as you say, thats completely fine to do step by step.

I think, all other cases I mention are more general issues with strict equality that are somewhat orthogonal to this SIP.

Some comments on the other points though.

I think the issue is that I don’t have a convenient enough workaround. The best I thought of was adding a given CanEqual[T, T] = CanEqual.derived in front of each such comparison.
I guess adding a standard library function to be used instead would be sufficient (or maybe there is something better I am not aware of).

I quite like T | Null as an unboxed option (this is with safe nulls).
Ironically, some of these checks are also specifically to interact with some APIs that may return null, or to guard against it.

eq does not work with safe null, because it no longer is an AnyRef neither is T | Null so the compiler complains … but that might be a separate bug. Though, I feel eq is even less idiomatic than null :neutral_face:

5

I supposed it’s not intended to help with case null =>, which I noticed because of a bug report about unused imports of CanEqual.