Skip to content

proposal: Allow comparison (==/!=) of ?T with U if T and U are comparable #5155

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

Open
foobles opened this issue Apr 24, 2020 · 4 comments
Open
Labels
proposal This issue suggests modifications. If it also has the "accepted" label then it is planned.
Milestone

Comments

@foobles
Copy link
Contributor

foobles commented Apr 24, 2020

Equality comparison (== or !=) should be allowed between ?T and U, as long as T and U can be compared the same way.

This is an extension of issue #1332 , which is accepted to only allow comparison between ?T and T if T is comparable to itself. This proposal aims to extend this to the next logical step, and avoid language inconsistencies. #1332 would allow something like this:

const x: ?i32 = 10;
const y: i32 = 10;
if (x == y) { ... }

but, surprisingly, not this:

const x: ?i32 = 10;
if (x == 10) { ... }

Because x is a ?i32, and 10 is a comptime_int, so the original proposal doesn't cover that case of different (yet still compatible) types. You would need to use an @as or something, which seems wrong in this case. In addition to the ability to utilize peer-type resolution and various special cases (like comparing an enum literal to a tagged union), this proposal solves another useful problem as a side-effect:

const x: ?i32 = 10;
const y: ?i32 = 10;
if (x == y) { ... }

Comparison of two different optionals!

This would work because, in this case, T = i32 (from the lhs), and U = ?i32 from the rhs. Then, the compiler would check if those two types are comparable: yes they are! The original goal of this proposal was to allow ?T to be compared with T, and that's what we have with T and U in this case.

This proposal brings further consistency and ergonomics to the language, that the original proposal missed. It also solves the other issue of comparing two optionals with each other, without heavy overhead or special-casing.

@foobles
Copy link
Contributor Author

foobles commented Apr 24, 2020

Clarification on how optional comparison to optional would work:

// assuming T is the child type of the optional, and U is the type of the other argument

?i32 == ?i32 // optional compared to something else, compiler just picks which one is T
 ^^^ T  ^^^^ U

// are T and U comparable?
i32 == ?i32 // new T and U are determined
^^^ U'  ^^^ T'

// are U' and T' comparable?
i32 == i32 // i32 is comparable with itself, so ?i32 is comparable with ?i32

@SpexGuy
Copy link
Contributor

SpexGuy commented Apr 24, 2020

Would this mean that ?(?T) == T is allowed because ?T == T is allowed? Induction like this would allow unwrapping of any number of optionals on both sides. Or we could special-case this to only allow one optional unwrapping on each side.

@foobles
Copy link
Contributor Author

foobles commented Apr 24, 2020

I suppose that would be a natural result as well. Would that be a problem if it did exist? Multiple-wrapped optionals are very uncommon anyway.

It wouldn't be difficult to special-case though, and I believe andrew said he wouldnt want ??T == T to work. I just don't see why it shouldn't,

The special casing could be as simple as: we select the optional child type T. If T is an optional, throw a compile-time error.

@thejoshwolfe
Copy link
Contributor

I'm not sure the idea of "f T and U are comparable" is well defined in Zig. These kinds of recursive definitions are scary. The pattern matches so many scenarios that it will lead to surprising results that have already been brought up in this discussion, like ??T == T and ?T == ?T.

Your example in the OP is a very good example:

const x: ?i32 = 10;
if (x == 10) { ... } // ERROR
if (x == @as(i32, 10)) { ... } // OK

That's a strong argument that something needs to change, but this proposal is not the only way. Here's another proposal:

  • For t: ?T, u: U, t == u is equivalent to: t == @as(T, u).

This would forbid comparing two optional types ?i32 == ?i32, because you can't cast from ?i32 to i32. Unfortunately, this would also make ?*T == ?*T a compile error, which it currently is not. This could be solved with a priority order of rules, but it's a little bit weird.

Another idea is to restrict the proposal for when U is not an optional type, but that seems unfair and surprising for generic code.

I think the fundamental problem with this is the double-variable pattern matching with T and U. In order to fix the x == 10 problem, we need a more restrictive condition.

@Vexu Vexu added the proposal This issue suggests modifications. If it also has the "accepted" label then it is planned. label Apr 28, 2020
@Vexu Vexu added this to the 0.7.0 milestone Apr 28, 2020
@andrewrk andrewrk modified the milestones: 0.7.0, 0.8.0 Oct 27, 2020
@andrewrk andrewrk modified the milestones: 0.8.0, 0.9.0 May 19, 2021
@andrewrk andrewrk modified the milestones: 0.9.0, 0.10.0 Nov 23, 2021
@andrewrk andrewrk modified the milestones: 0.10.0, 0.11.0 Apr 16, 2022
@andrewrk andrewrk modified the milestones: 0.11.0, 0.12.0 Apr 9, 2023
@andrewrk andrewrk modified the milestones: 0.13.0, 0.12.0 Jul 9, 2023
@andrewrk andrewrk modified the milestones: 0.14.0, 0.15.0 Feb 9, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
proposal This issue suggests modifications. If it also has the "accepted" label then it is planned.
Projects
None yet
Development

No branches or pull requests

5 participants