1184839 - Provide an overloaded == operator for mozilla::Variant

Status: RESOLVED FIXED

(Core :: MFBT, defect)

Comment 1

[Nick Fitzgerald [:fitzgen] [⏰PST; UTC-8]]

Attachment: Provide an overloaded == operator for mozilla::Variant

Comment 2

[Jeff Walden [:Waldo]]

Comment on attachment 8635111 [details] [diff] [review]
Provide an overloaded == operator for mozilla::Variant

Add an operator!= returning the negation of equality, please.

And I guess there's no need yet for T == Variant<..., T, ...> or vice versa yet, right?  So no need to try to futz that into place.  Seems like if comparing two variants is desirable, where the stored type of each is unknown, the more-precise comparison where a variant is compared against (in effect) a variant containing a *known* type is a fortiori desirable.

Comment 3

[Nick Fitzgerald [:fitzgen] [⏰PST; UTC-8]]

(In reply to Jeff Walden [:Waldo] (remove +bmo to email) from comment #2)
> Comment on attachment 8635111 [details] [diff] [review]
> Provide an overloaded == operator for mozilla::Variant
> 
> Add an operator!= returning the negation of equality, please.
> 
> And I guess there's no need yet for T == Variant<..., T, ...> or vice versa
> yet, right?  So no need to try to futz that into place.  Seems like if
> comparing two variants is desirable, where the stored type of each is
> unknown, the more-precise comparison where a variant is compared against (in
> effect) a variant containing a *known* type is a fortiori desirable.

Yeah that certainly seems like a good thing to support, but yeah my approach so far has been to add things only as I need them. Filed bug 1185050 for this.

Comment 4

[Nick Fitzgerald [:fitzgen] [⏰PST; UTC-8]]

Attachment: Provide an overloaded == operator for mozilla::Variant

Comment 5

[Jeff Walden [:Waldo]]

Comment on attachment 8635481 [details] [diff] [review]
Provide an overloaded == operator for mozilla::Variant

Review of attachment 8635481 [details] [diff] [review]:
-----------------------------------------------------------------

::: mfbt/tests/TestVariant.cpp
@@ +109,5 @@
> +  MOZ_RELEASE_ASSERT(v0 != v1);
> +  MOZ_RELEASE_ASSERT(v1 == v2);
> +  MOZ_RELEASE_ASSERT(v2 != v3);
> +  MOZ_RELEASE_ASSERT(v3 != v4);
> +  MOZ_RELEASE_ASSERT(v4 == v5);

Also seems worth having

  V v6(int('b'));

and perhaps others, comparing that against V('b') and verifying they're not equal.  You've tested the different-tags-different-values case, the same-tags-but-different-values case, and the same-tags-values cases.  That leaves the different-tags-same-values case worth testing.

Comment 6

[Nick Fitzgerald [:fitzgen] [⏰PST; UTC-8]]

Attachment: Provide an overloaded == operator for mozilla::Variant

Comment 7

[Nick Fitzgerald [:fitzgen] [⏰PST; UTC-8]]

Try push: https://treeherder.mozilla.org/#/jobs?repo=try&revision=002aa726253a

Comment 8

[Pulsebot]

https://hg.mozilla.org/integration/mozilla-inbound/rev/53d4da7e97f1

Comment 9

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/mozilla-central/rev/53d4da7e97f1