The issue with Value/Ref#== having a default implementation

[docelic]

I realized that Crystal currently allows comparisons that always fail:

"a" == 'a'  # Not comparable, but accepted by compiler and always returns false
SomeEnum::Member == 10 # Same here (`SomeEnum != Int32` but again accepted and returns false)

I asked on Crystal chat and @asterite and @straight-shoota report that comparing anything to anything (defaulting to returning false) was allowed because otherwise comparing union types would be problematic, and the language would be too rigid. Previous or similar tickets about this issue, which explain it, are #2996, #6154, and #8893. (And #1730 which asks about the same issue.)

(For a very brief example, if such comparisons were prevented, they would have prevented some very reasonably-looking code from working, such as: {1, "a", nil}.includes? 'x')

In any case, the reason why these comparisons are allowed (and always returning false) is this fallback implementation of #== in types Value and Reference:

  # Returns `false`.
  def ==(other)
    false
  end

This seems to me as undesirable because it (1) removes type safety from arguably the most basic operator in a language (==), and also it partially masks Comparable.

I think that both arguments:

1. Being able to compare anything to anything, returning false
2. Asking for type safety in a comparison operator

Are completely valid, and in fact they represent two different use cases which shouldn't be hidden behind a single #== operator.

My suggestion is that we add a new operator which implements the other case.

Option 1:
The default implementation of #== is removed, so that the only working #== comes from Comparable or a specific implementation of #==, and is type safe.
Another operator (for example ==?) is added which does what #== currently does (returns false as a fallback).

Option 2:
#== is left as-is, and another operator is added, which enforces type checks.

I think option 1 would be clean and consistent, certainly preferrable, and option 2 would maybe be easier/more viable, all things considered.

@straight-shoota mentioned that he was preparing an RFC about this issue.
@asterite said: "It would be nice if equality was type safe. Someone can try it and see how hard it is to make the std specs pass with that change, and how the code changes."

Comments welcome.

[asterite]

You can send a PR for the first option and see how it goes. Without trying this there's no way to weight this.

[straight-shoota]

I don't think option 1 makes any sense, honestly. While there are use cases for stricter and type-safe equality, I doubt it's the best choice for the omnipresent == operator. Also it's a huge breaking change. If we want to introduce a new, stricter equality concept, it should use a new method, not replace an existing one which then gets a different replacement. Just way less friction that way. So change management definitely speaks for option 2.

We should probably add something like Ruby's #eql? method. In Ruby it's terrible naming btw. because there's also #equal? (in Crystal that's called #same? so we're safe here 😆).

[asterite]

I just want to add that in some languages equality between different types is a compile error. Haskell is one such example. In C# I think they disallow comparing intw to strings, but then you can compare any reference type to any other reference type.

I really commend someone to see if this is possible. I'm almost sure the friction will be too great, but at least we'll have these reasons documented.

[docelic]

I can try it. I will try option 1 which is more interesting. Option 2 (which just adds a new method) could be done even now by anyone who reopens a few types and defines that method in there. They don't even need official help from the language.

I agree with everything said about friction, breaking change, etc., but I think it would be quite strange if a type-safe language had to use a non-standard operator (i.e. something other than ==) to turn on type safe checks. Also, it would look pretty unusual if any other operator than == became the "normal" equality operator.

So option 1 is certainly worth trying, at least so that we know where we stand as Ary says.

So my plan is, make == stricter by default, and make eql? (just temporary name, doesn't matter) be the current relaxed check, using the following procedure:

1. At every place where def == exists, add the same def eql? which uses .eql? instead of ==
2. Then, remove the default implementations of == which return false
3. Then, go through compile errors of std_spec one by one, examining on a case by case basis what's actually going on there (in simplest cases the usage of == would just be replaced with eql?, but I guess there will be non-trivial issues as well)

If you have better instructions than this, sure let me know.

I already did some of this yesterday. Will update when there are news. Thanks.

[straight-shoota]

@docelic Did you get anywhere with your trials?

[docelic]

Hey @straight-shoota , thanks for checking. I did some basic testing:

1. Renamed default implementations of def ==(other) in src/value.cr and src/reference.cr to def eql?
2. Tried to run make spec

This revealed that the first/simplest cases where the fallback to default == happens are comparisons of type Nil == NonNil and NonNil == Nil.

I took care of those two types of cases globally by defining a specific def ==(other : Nil). Through further cases that errored, I realized that the issue (same as with Nil) happens primarily with type unions, where only some of the possible types in a union don't have a specific == defined. For example:

In src/time/location.cr:140:64

 140 | if with_seconds == true || (seconds != 0 && with_seconds == :auto)
                                                                ^
Error: no overload matches 'Bool#==' with type Symbol

(In that example, with_seconds is Bool | Symbol, and if it happens that it's a bool then the default == is called.)

From that I figured I couldn't go through code and replace all errors with eql?, because that circumvents all existing ==s and goes straight to the default implementation of eql? for all types.

My next attempt will be to leave the default/fallback == in place, and add code to it which will count which types (and how many times) are compared using the fallback ==s.

[docelic]

But, apart from any further testing/investigation, a more interesting question is -- do you see any way in which this ticket could move or these are just last comments before closing it?

From what I could tell, it is too late to change the behavior of ==. So our only choice (if anything is to be done here) would be to define a new operator such as ==!, which behaves identically to == (ideally it would use == automatically), except for the lowest level in Value and Reference where it would not have a default implementation, and would instead allow the code to cause an error rather than assume false.

Your thoughts/suggestions?

[straight-shoota]

At this point we're really just at an exploration stage. It's impossible to say how it might end up some day.
But I certainly wouldn't outright dismiss this. If there's a chance to improve type safety of equality checks, I think that's a reasonable goal.

#8893 is a very similar matter. It's a bit more advanced already and has shown that stricter type restrictions help identify (and subsequently avoid) bugs.

We can probably get useful information from collecting statistics of == calls and their types.

[straight-shoota]

Discussion about the same problem in Julia: JuliaLang/julia#40717

[Sija]

In the last ameba release there was added Lint/LiteralsComparison rule which reports cases like the one mentioned by the OP.