Skip to content

Allow equality operator (== and !=) for ?T and T when equality operator is supported for T and T #1332

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

Closed
mdsteele opened this issue Aug 5, 2018 · 13 comments
Labels
accepted This proposal is planned. proposal This issue suggests modifications. If it also has the "accepted" label then it is planned.
Milestone

Comments

@mdsteele
Copy link
Contributor

mdsteele commented Aug 5, 2018

This test passes just fine:

test "compare optional with non-optional" {
    const foo: *i32 = @intToPtr(*i32, 4);
    const bar: ?*i32 = @intToPtr(*i32, 4);
    assert(foo == bar);
}

While this one fails to compile, with the error message "error: operator not allowed for type '?i32'":

test "compare optional with non-optional" {
    const foo: i32 = 4;
    const bar: ?i32 = 4;
    assert(foo == bar);
}

At first, I assumed this was a bug, but reading #658, it sounds like this was a deliberate decision?
(Because ?i32 == i32 would have to do two comparisons behind the scenes, while ?*i32 == *i32 can just do one.)

Still, this seems like a surprising inconsistency -- in every other language I can think of with a generic option type, either (1) Option<T> supports == iff T supports ==, or (2) Option<T> doesn't support == at all. Here, we have ?T supporting == for one primitive type (pointers) but not for an even more primitive type (i32).

mdsteele added a commit to mdsteele/zig that referenced this issue Aug 5, 2018
@andrewrk andrewrk added the proposal This issue suggests modifications. If it also has the "accepted" label then it is planned. label Aug 6, 2018
@andrewrk andrewrk added this to the 0.4.0 milestone Aug 6, 2018
@andrewrk andrewrk changed the title Comparing ?T == T, e.g. "error: operator not allowed for type '?i32'" Allow equality operator (== and !=) for ?T and T when equality operator is supported for T and T Aug 6, 2018
@andrewrk andrewrk modified the milestones: 0.4.0, 0.5.0 Feb 15, 2019
@andrewrk andrewrk added the accepted This proposal is planned. label Feb 15, 2019
@andrewrk
Copy link
Member

andrewrk commented Feb 15, 2019

I think this makes sense especially considering #1059 and #1953 which create a scenarios where pointer types are in the second category.

@andrewrk andrewrk modified the milestones: 0.5.0, 0.6.0 Aug 28, 2019
@thejoshwolfe
Copy link
Contributor

@andrewrk andrewrk modified the milestones: 0.6.0, 0.7.0 Jan 3, 2020
@foobles
Copy link
Contributor

foobles commented Apr 11, 2020

What should happen if one side is T, while the other is ?T ?

Should fn_returning_i32_not_an_option() == null be allowed?

Edit: I suppose the i32 could implicitly coerce to a ?i32

@thejoshwolfe
Copy link
Contributor

Should fn_returning_i32_not_an_option() == null be allowed?

A similar question is whether 0 == null should be a compile error. I think there's no good reason for 0 == null, but @as(?i32, 0) == null should be allowed.

@andrewrk
Copy link
Member

A similar question is whether 0 == null should be a compile error.

It's a bug if not.

@foobles
Copy link
Contributor

foobles commented Apr 20, 2020

For example, when comparing two distinct optional values:

const x: ?i32 = createAnOptional();
const y: ?i32 = createAnOptional();
if (x == y) { 
    debug.warn("equal", .{});
}

It would be necessary to turn the x == y into something like the following (under the hood):

((x == null) == (y == null)) and (x == null or x.? == y.?)

But if one were to write this instead:

if (x == 10) { ... }

What should that expand to? Should the 10 be implicitly cast to an optional, or should comparing ?T with T be a special case, where it turns into something closer to this:

(lhs != null) and (lhs.? == rhs)

Which is likely a lot more efficient.
Would it be worth it to make this optimization? Or could LLVM just do this on its own?

@thejoshwolfe
Copy link
Contributor

@theInkSquid this proposal is for comparing ?T and T. If we want to support comparing ?T and ?T, that's a separate proposal.

This proposal is to expand @as(?T, foo) == @as(T, bar) to foo != null and foo.? == bar.

What you're proposing for ?T and ?T might also be valuable, but it needs its own proposal.

@foobles
Copy link
Contributor

foobles commented Apr 20, 2020

I see, thank you. Would it be appropriate to short-circuit evaluate the non-optional parameter? For example:

const x: ?i32 = null;
if (x == @compileError("err")) {...}

Should this be a compile error? Or, since it can be determined that because x is null (and in no case will it be equal to the right hand side), should it just evaluate to false?

I am guessing this should not short-circuit, since that would be a non-keyword operator affecting control flow. Also, it seems a bit useless and confusing, but I just wanted to check.
@andrewrk

@thejoshwolfe
Copy link
Contributor

thejoshwolfe commented Apr 20, 2020

@theInkSquid that's an insightful question. There are two options.

For the expression lhs == rhs where lhs is of type ?T and rhs is of type T, the semantics should either be:

Option 1: no short circuit.

$lhs_value = evaluate lhs;
$rhs_value = evaluate rhs;
if $lhs_value == null {
    result = false;
} else {
    result = $lhs_value.? == $rhs_value;
}

Option 2: short circuit:

$lhs_value = evaluate lhs;
if $lhs_value == null {
    result = false;
} else {
    $rhs_value = evaluate rhs;
    result = $lhs_value.? == $rhs_value;
}

The OP posted an important motivation for this issue: the inconsistency of the == operator for optional pointers and optional everything-else. Following this logic, we should do option 1 with no short circuit, since we don't do that for pointer types.

EDIT: An even stronger argument for no short circuiting is that the == operator is supposed to be commutative after operand evaluation. Consider instead of ?T == T you had T == ?T. The == operator semantics require that the left hand side be evaluated before the right-hand-side. It would be very surprising if there was short circuiting depending on the ordering of the parameters and depending on whether T was a pointer type.

@mikdusan
Copy link
Member

mikdusan commented Apr 20, 2020

this proposal is for comparing ?T and T.

to clarify, ??T == T is to be a compile-error?

@andrewrk
Copy link
Member

Yes

@foobles
Copy link
Contributor

foobles commented Apr 22, 2020

How about when ?T == U when T and U coerce to the same type?

Like could I do this?

var x: ?i32 = ...;
var y: i16 = ...;
x == y

@foobles
Copy link
Contributor

foobles commented Apr 23, 2020

Also, what about for other special cases where a type is comparable with some different other type? For example, you can compare a tagged union with an enum literal to check if that is the current value. Should an optional tagged union be comparable with an enum literal? That doesn't fall into the rigid ?T ==/!= T that was specified, but would make a lot of sense to have.

If something like opt_tagged_union == .enum_literal is allowed, then what mikdusan asked about would indeed by allowed by extension:

If we allow ?T == U if T and U can be compared, then in ??i32 == i32, T is ?i32 and U is i32, which are also comparable under this proposal.

Even more powerful, if we allow ?T == U, is that it could compare ?T == ?T, with this chain of logic:

?i32 == ?i32
T = i32, U = ?i32

now we check if i32 is comparable with ?i32, which it also is under this proposal.

@andrewrk

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
accepted This proposal is planned. proposal This issue suggests modifications. If it also has the "accepted" label then it is planned.
Projects
None yet
5 participants