Closed Bug 1184839 Opened 5 years ago Closed 5 years ago

Provide an overloaded == operator for mozilla::Variant

Categories

(Core :: MFBT, defect)

defect
Not set
normal

Tracking

()

RESOLVED FIXED
mozilla42
Tracking Status
firefox42 --- fixed

People

(Reporter: fitzgen, Assigned: fitzgen)

References

(Blocks 1 open bug)

Details

Attachments

(1 file, 2 obsolete files)

No description provided.
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.
Assignee: nobody → nfitzgerald
Status: NEW → ASSIGNED
(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.
Attachment #8635111 - Attachment is obsolete: true
Attachment #8635111 - Flags: review?(nfroyd)
Attachment #8635481 - Flags: review?(nfroyd)
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.
Attachment #8635481 - Flags: review?(nfroyd) → review+
https://hg.mozilla.org/mozilla-central/rev/53d4da7e97f1
Status: ASSIGNED → RESOLVED
Closed: 5 years ago
Flags: in-testsuite+
Resolution: --- → FIXED
Target Milestone: --- → mozilla42
You need to log in before you can comment on or make changes to this bug.