Closed Bug 1142366 Opened 5 years ago Closed 5 years ago

Add an equivalent of std::make_pair for mozilla::Pair

Categories

(Core :: MFBT, defect)

defect
Not set

Tracking

()

RESOLVED FIXED
mozilla39
Tracking Status
firefox39 --- fixed

People

(Reporter: seth, Assigned: seth)

References

Details

Attachments

(1 file, 2 obsolete files)

Using mozilla::Pair is a bit aggravating right now because you have to type out the all of the template parameters (which can be verbose in some cases) to construct an instance. The STL fixes that with std::make_pair, which uses type inference to build a std::pair object of the correct type. Let's add the same facility for mozilla::Pair.
Here's the patch. This adds MakePair, which should behave equivalently to
std::make_pair. (Except that std::make_pair applies std::decay to its type
arguments, but we haven't implemented that, so I just removed references and
CV-qualifiers.)

The "Pair(Pair&& aOther) = default;" line might seem like a nonsequitor, but
MakePair is completely useless if Pair is neither copyable nor movable. I'm not
convinced that copyability is a bad idea, but movability should be pretty
uncontroversial, so I added that here. By using "= default", there shouldn't be
any issues if the underlying types aren't movable - you just won't be able to
use MakePair in that case.

A small smattering of new tests are also included.
Attachment #8576385 - Flags: review?(jwalden+bmo)
Tweaked the test a bit.
Attachment #8576404 - Flags: review?(jwalden+bmo)
Attachment #8576385 - Attachment is obsolete: true
Attachment #8576385 - Flags: review?(jwalden+bmo)
Blocks: 1142376
Blocks: 1125055
OK, MSVC 2013 doesn't support "T(T&&) = default", so we can't use that. Here's a
new version of the patch with a workaround.

Here's a new try job:

https://treeherder.mozilla.org/#/jobs?repo=try&revision=5e5232b906eb
Attachment #8576930 - Flags: review?(jwalden+bmo)
Attachment #8576404 - Attachment is obsolete: true
Attachment #8576404 - Flags: review?(jwalden+bmo)
Comment on attachment 8576930 [details] [diff] [review]
Add an equivalent of std::make_pair for mozilla::Pair

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

::: mfbt/Pair.h
@@ +188,5 @@
> + * like this:
> + *
> + *   MakePair(Foo(), Bar())
> + *
> + * will return a Pair<Foo, Bar>.

Add a comment noting that without -> decltype(...) support that we won't have until which particular feature on the C++11/14 feature list pagee sayp we can, we can't actually write out the type of the return value, and so we can't have a MakePair that works on anything and everything including movable-only types.
Attachment #8576930 - Flags: review?(jwalden+bmo) → review+
Thanks for the review!

We discussed the review comments further on IRC, and concluded that what's really wanted here is to implement an equivalent of std::decay, so I filed bug 1142823 about that.
https://hg.mozilla.org/mozilla-central/rev/70bccab35a02
Status: NEW → RESOLVED
Closed: 5 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla39
You need to log in before you can comment on or make changes to this bug.