Yet another optimization with rvalue reference &&. Simply rename fields

Question

Let's consider the next code:

struct Channel;  // somewhere declared
using iter_t = std::set<Channel>::iterator;

std::set<Channel> myset;
std::pair<iter_t, bool> emplaced = myset.emplace(arg1,arg2);

Then emplaced.first contains iterator to element, emplaced.second indicates if element was added, or already exist. first and second is not clear as for me. I want to rename this fields:

struct ChannelAddedResult
{
    iter_t channelIter;
    bool wasAdded;
public: // --- Functions, constructors ---
    ChannelAddedResult(std::pair<iter_t, bool> &&pa) {
       this->channel = pa.first;
       this->added = pa.second;
    }
 };

This constructor copies values. And here is no profit from rvalue reference. Right?
But how can I convert std::pair<iter_t, bool> to ChannelAddedResult? This types are equivalent. So C-style conversation could look like:

union CarUni {
  std::pair<iter_t, bool>  pair;
  ChannelAddedResult       car;
}
// Use
CarUni u;
u.pair = myset.emplace(arg1,arg2);
auto ch = *u.car.channelIter;
bool a = u.car.wasAdded;

This allows to achieve rename without additional copies. May be C-casting (ChannelAddedResult)emplaced_pair will do the same work, but it is deprecated in C++. These 2 conversions are dangerous from the point of type safety.

Is there a C++11 way for such conversion?

Where is this "C-style conversation" you're talking about? That's all C++. And C-style casts (which you have *not used* in your example) are not deprecated in C++. Overall, your code seems fine and not in any way "dangerous from the point of type safety". What is your concern, exactly? — Nicol Bolas, Dec 08 '16 at 19:04
`std::pair &emplaced == myset.emplace(arg1,arg2);` should be `std::pair &emplaced = myset.emplace(arg1,arg2);` — badola, Dec 08 '16 at 19:09
`std::pair &emplaced == myset.emplace(arg1,arg2);` please avoid posting code that does not compile for reasons unrelated to your question. I guess there are two typos, but... if there are typos there how many other random features of your code are typos you did not intend? — Yakk - Adam Nevraumont, Dec 08 '16 at 19:26
Second, how expensive do you think copying a std set iterator and a bool is? How hard to optimize do you think it is? — Yakk - Adam Nevraumont, Dec 08 '16 at 19:29
*expensive* - here 2 copies does not meter. But this is only snippet. In some more extended code i.e. emplace 100000000000 channels could slow down application. This example implements **idea of renaming fields**. I can imagine other implementations and variations. — kyb, Dec 08 '16 at 20:30

score 1 · Answer 1 · answered Dec 08 '16 at 19:31

1

If you don't want to copy, move:

ChannelAddedResult(std::pair<iter_t, bool> &&pa)
    : channel(std::move(pa.first))
    , added(std::move(pa.second))
{}

Though for types like bools and iterators there is no real difference.

answered Dec 08 '16 at 19:31

yuri kilochek

12,709
2
32
59

badola · Answer 2 · 2016-12-09T04:03:12.667

I have written a small sample program to show how I might have given it a try. What I get from your question is that you want to rename the first and second of pair with your custom typed name i.e. channelIter and wasAdded.

I took your sample code and made a simpler program out of it.

#include <set>
#include <iostream>

struct Channel
{
    int _a; // Declaring 2 simple args
    int _b;

    Channel(int a, int b) : _a(a), _b(b) {} 

    // Need to provide a < operator for custom datatype
    bool operator<(Channel const & rhs) const {
        return (this->_a < rhs._a) || (this->_b < rhs._b);
    }
};

using iter_t = std::set<Channel>::iterator;

struct ChannelAddedResult
{
    iter_t _channelIter;
    bool   _wasAdded;

    ChannelAddedResult(std::pair<iter_t, bool> && pa) 
                      : _channelIter(pa.first), _wasAdded(pa.second) {}
};

int main()
{
    std::set<Channel> myset;
    auto u  = ChannelAddedResult(myset.emplace(5, 6));
    auto ch = *u._channelIter; // Access pair.first
    bool a  = u._wasAdded;     // Access pair.second

    std::cout << "ch._a => [" << ch._a << "] ch._b => [" << ch._b << "]\n";
    return 0;
}

Compile with -std=c++11

$ g++ -std=c++11 set_sample.cpp -o set_sample

Output

$ ./set_sample
ch._a => [5] ch._b => [6]

Point to be understood here is that, ChannelAddedResult(std::pair<iter_t, bool> && pa) is an rvalue reference and should be passed an rvalue to be used efficiently. So it should be used like ChannelAddedResult(myset.emplace(5, 6)). Here myset.emplace(5,6) is an rvalue. So no copy would be created.

On the contrary if the usage had been as following, a useless copy is being created in between. Passing an lvalue to an rvalue reference defeats the whole purpose.

auto val = myset.emplace(5,6);
auto u  = ChannelAddedResult(val); // val is an lvalue

The above mentioned code won't compile though. To make it work you would need to upgrade the signature as -

ChannelAddedResult(std::pair<iter_t, bool> const & pa) 
                  : _channelIter(pa.first), _wasAdded(pa.second) {}

Now it will work with both rvalue and lvalue. Since we are already copying the data of std::pair into other variables, the usage of rvalue reference doesn't add much to the performance. Better use T const & pa rather than T && pa, in this situation.

Right. Thank you for the full program. I tried to make my post as short as possible. But here as far as I understand `&&pa` does not any meter since it works exactly as `const&pa` — kyb, Dec 08 '16 at 20:36
@kyb : Your understanding is not correct. `T && pa` is an rvalue reference and will directly use the value of `myset.emplace(5, 6)` (since it is an rvalue in my program). On the contrary, making use of `T const & pa` instead of `T && pa` would have created a temporary copy for the same, used ithe temporary, and then later destroy the temporary copy. — badola, Dec 09 '16 at 02:11
I have edited my answer to add more details catering to your doubt. Go through it once again. — badola, Dec 09 '16 at 03:45

Yet another optimization with rvalue reference &&. Simply rename fields

2 Answers2