mapbox / mapbox/variant

Implement swap.

Open
#80 1 comment 1 reaction 0 assignees View on GitHub

Nobody has claimed this yet.

enhancement
Dominant language
C++
Stars
384
Forks
96
PR merge metrics
No merged PRs in 30d

Description

In the current code there is no implementation for `swap()` which means `std::swap()` will be used. This should do the right thing (as long as the move constructor and move assignment do the right thing) and the [tests](https://github.com/mapbox/variant/blob/861faa8125ae7c2d9e41e33b3d5d97a17213c415/test/t/variant.cpp#L268-L300) show this.

But this solution is not optimal, because the default `std::swap()` will always do one move construction and two move assignments (in this case of the variant). If there is a more efficient `swap` for the type in the variant, it will not be used. The upcoming standard [P0088R0](http://www.open-std.org/jtc1/sc22/wg21/docs/papers/2015/p0088r0.pdf) describes the effects of swap as: "if `index() == rhs.index()`, calls `swap(get(*this), get(hrs))` with `i` being `index()`. Else calls `swap(*this, hrs)`."

We might want to implement our own `swap` in the future.

Contributor guide

Open the contributing guide

First steps

  1. Read the whole issue, then the project's contributing guide.
  2. Comment on the issue to say you are picking it up — it saves two people doing the same work.
  3. Fork the repository and make your change on a branch.
  4. Open a pull request that references the issue number.

Research direction

Start by reading the existing variant move and swap-related code, then run the tests in test/t/variant.cpp around lines 268-300. Compare the current behavior with the issue's same-index and different-index swap requirements. Done means the tests pass and swap can use the contained type's more efficient operation when appropriate.

Written by the indexing model from the issue text.

Assessment

Tech stack
cpp
Domain
backend
Issue type
Feature
Difficulty
3/5
Estimated time
1-2 days
Activity status
Stale
Clarity
Mostly clear
Newbie friendliness
35/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.