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

@asterite

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:

Being able to compare anything to anything, returning false
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.

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

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

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.

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:

At every place where def == exists, add the same def eql? which uses .eql? instead of ==
Then, remove the default implementations of == which return false
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.

@docelic

@docelic Did you get anywhere with your trials?

@straight-shoota

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

Renamed default implementations of def ==(other) in src/value.cr and src/reference.cr to def eql?
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.

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?

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.

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

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

I've been investigating this today as it is an issue I and others run into frequently.

comparing anything to anything (defaulting to returning false) was allowed because otherwise comparing union types would be problematic

I have some thoughts on this from the experimentation I've done:

Comparison between nil or things that are solely a nilable union (String?) should be left as-is
Comparison between two unions where some of the types match (excluding Nil) should be left as-is
Comparison between two unions where none of the types match (excluding Nil) should give a warning (or hopefully in the future, raise an error)

For example:

# This is permitted
1 == nil

# This gives a warning
1 == "string"

# This gives a warning - `String#[]?` returns `Char | Nil`, which doesn't include `String`
string[i]? == "j"

case JSON::Any.new(something).raw
# This is permitted - `#raw` returns `Array(JSON::Any) | Bool | Float64 | Hash(String, JSON::Any) | Int64 | String | Nil`, which includes `String | Nil`
when string[i]?
end

Whether this is done with the addition of an ==? or ==! operator, I'm not sure. I have a preference for the former for a safe default that has an unsafe fallback if one needs it, but I understand this is a massive breaking change. There is also the argument to be made that this kind of thing be left up to a linter with semantic analysis, which is also something that can be looked into.

I'm not sure how feasible this behavior would be to implement, and may require special handling specifically for the equality operator (and maybe other comparison operators too). Someone could define their own MyClass#==(other : String) and even if the types don't match, the method wouldn't always return false, and we'd need to be able to handle that.

Edit:

Put more work into this and came up with this (https://gist.github.com/nobodywasishere/6c1d42eda30d469346b21516b6294710), which allows comparisons between exact types and nilable unions, but disallows everything else. Has already found 2 bugs in my code, thought it may be useful to others as well, just as a separate checker.

@nobodywasishere

@nobodywasishere Do you think this could make it into an Ameba rule?

It definitely can but will require semantic analysis to implement, probably full semantic analysis to catch all cases. How exactly to do that is still in progress.

This issue has been mentioned on Crystal Forum. There might be relevant details there:

https://forum.crystal-lang.org/t/sysrift-v0-1-0-release/8835/5

Navigation Menu

Repository navigation

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

Metadata

Assignees

Labels

Type

Projects

Milestone

Relationships

Development

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

Description

Activity

asterite commented on Jan 21, 2021

straight-shoota commented on Jan 21, 2021

asterite commented on Jan 22, 2021

docelic commented on Jan 22, 2021

straight-shoota commented on Sep 7, 2021

docelic commented on Sep 7, 2021

docelic commented on Sep 7, 2021

straight-shoota commented on Sep 28, 2021

straight-shoota commented on Dec 12, 2022

Sija commented on Dec 12, 2022

nobodywasishere commented on Jan 2, 2025

ysbaddaden commented on Feb 9, 2025

nobodywasishere commented on Feb 9, 2025

crysbot commented on Apr 7, 2026

Metadata

Metadata

Assignees

Labels

Type

Projects

Milestone

Relationships

Development

Issue actions