Closed
Bug 1184839
Opened 10 years ago
Closed 10 years ago
Provide an overloaded == operator for mozilla::Variant
Categories
(Core :: MFBT, defect)
Core
MFBT
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)
3.87 KB,
patch
|
fitzgen
:
review+
|
Details | Diff | Splinter Review |
No description provided.
Assignee | ||
Comment 1•10 years ago
|
||
Attachment #8635111 -
Flags: review?(nfroyd)
Comment 2•10 years ago
|
||
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 | ||
Updated•10 years ago
|
Assignee: nobody → nfitzgerald
Status: NEW → ASSIGNED
Assignee | ||
Comment 3•10 years ago
|
||
(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.
Assignee | ||
Comment 4•10 years ago
|
||
Attachment #8635111 -
Attachment is obsolete: true
Attachment #8635111 -
Flags: review?(nfroyd)
Attachment #8635481 -
Flags: review?(nfroyd)
Comment 5•10 years ago
|
||
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+
Assignee | ||
Comment 6•10 years ago
|
||
Attachment #8635481 -
Attachment is obsolete: true
Attachment #8635840 -
Flags: review+
Assignee | ||
Comment 7•10 years ago
|
||
Assignee | ||
Updated•10 years ago
|
Keywords: checkin-needed
Keywords: checkin-needed
Comment 9•10 years ago
|
||
Status: ASSIGNED → RESOLVED
Closed: 10 years ago
status-firefox42:
--- → fixed
Flags: in-testsuite+
Resolution: --- → FIXED
Target Milestone: --- → mozilla42
You need to log in
before you can comment on or make changes to this bug.
Description
•