Container which iterates a range or a list

I have made some composited container Range which accepts either a min/max range as a std::pair or a set of integers as a std::set.

internally it saves a copy of the input by as void *

This container supports iterators which is useful.
I am not sure if there is a better implementation to mimic an iteration for both, a range or a list of numbers, also with respect to move semantics.
I could have used boost::variant types, but that only bloats the whole class, and for the iterator I need another boost::variant filled with iterators ...?

Have I used the move semantics correctly?

I added also some performance test in the MWE: MWE LINK

```
template
class Range{
public:
typedef std::pair Pair;
typedef std::set Set;

Range(const Pair & pair){
m_ptr = static_cast( new Pair(pair) );
m_which = 0;
}
Range(Pair && pair){
m_ptr = static_cast( new Pair( std::move(pair) ) );
m_which = 0;
}

Range(const Set & set){
m_which = 1;
m_ptr = static_cast( new Set(set) );
}
Range(Set && set){
m_which = 1;
m_ptr = static_cast( new Set( std::move(set) ) );
}

Range(const Range & r){
*this = r;
}

// Move Constructor
Range(Range && r){
*this = std::move(r);
}

// Move Assigment
Range & operator=(Range && r){
assert(r.m_ptr);
if(m_ptr != r.m_ptr && this != &r ){ // Prevent self-assignment
~Range(); // delete resources
m_ptr = std::move(r.m_ptr);
m_which = std::move(r.m_which);
r.m_ptr = nullptr;
r.m_which = 0;
}
}

// Assigment
Range & operator=(Range & r){
if(m_ptr != r.m_ptr && this != &r ){ // Prevent self-assignment
if(r.m_which == 0){
if(m_which == 0 && m_ptr){
static_cast(m_ptr) = static_cast(r.m_ptr); // Copy
}
m_ptr = static_cast( n

You should use template specialization rather than casting to void* here

// Pair or set depends on m_which.
// Very easy to get that screwed up.
// Especially since it is not const.

Range(const Pair & pair){
    m_ptr = static_cast( new Pair(pair) );
    m_which = 0;
}
Range(const Set & set){
    m_which = 1;
    m_ptr = static_cast( new Set(set) );
}

Your technique is very error prone as the compiler can no longer validate the types you are using. By using template specialization you remove the problem.

This is the wrong way around.

Range(const Range & r){
    *this = r;
}

You should define the copy constructor like normal. Then define the assignment operator in terms of the copy constructor. It is known as the copy and swap idiom.

Also you don't set up the members. This means unless the assignment operator only sets the values you are looking at undefined behavior when they are read.

// As I suspected the first thing you try and do is read from `m_ptr`.
// This value has not been set in the copy constructor and thus you have
// undefined behavior.
Range & operator=(Range & r){
    if(m_ptr != r.m_ptr && this != &r ){

This is very hard to understand.

Comes from casting around void*. Would not do this.

if(m_which == 0 && m_ptr){
                *static_cast(m_ptr) = *static_cast(r.m_ptr); // Copy
            }

Now you leak the original data and replace it with the a copy of the source.

m_ptr = static_cast( new Pair(*static_cast(r.m_ptr))); // Make new and Copy
        }

Copy and Swap idiom looks like this:

Range(Range const& r)
  : m_which(r.m_which)
  , m_ptr(r.m_which == 0
            ? static_cast(new Pair(*static_cast(r.m_ptr)))
            : static_cast(new Set(*static_cast(r.m_ptr)))
         )
{}

Range& operator=(Range value) // Pass by value to generate copy.
{
    value.swap(*this);        // Swap value with the copy.
    return *this;
}                             // Destructor of `value` cleans up the old value.

void swap(Range& other) noexcept
{
    std::swap(m_which, other.m_witch);
    std::swap(m_ptr,   other.m_ptr);
}

Same problem with the move constructor.

// Move Constructor
Range(Range && r){
    *this = std::move(r);
}

You don't initialize the members. Since the members are POD they have indeterminate values and thus reading them is undefined behavior (unless you assign something to them).

So the move assignment operator has the same problems as the assignment operator as it reads the value of m_ptr (only a problem when called from the move constructor).

// Move Assigment
Range & operator=(Range && r){
    assert(r.m_ptr);

    // Reading m_ptr here.
    // But we have just been called from the move constructor
    // thus m_ptr is not defined.
    if(m_ptr != r.m_ptr && this != &r ){

You are allowed to call the destructor.

~Range(); // delete resources

But this stops the object from being an object. The only thing you are allowed to do at this point is call the call placement new to make it a real object again (or pass it around as void*). So calling the destructor is not really a good idea. Move the code for releasing resource into another function and call that from here and the destructor.

OK. So after a move the source should be in a valid state (but can be indeterminate state).

r.m_ptr = nullptr;
        r.m_which = 0;

I suppose this does that. But a lot of code assumes that m_which of 0 means that it contains a pointer to a pair. You may want to have another state ie 2 (or -1) that indicates nothing is stored in the object.

Personally I would simplify the above:

Range(Range && r)
    : m_which(r.m_wich)
    , m_ptr(r.m_ptr)
{
    // r.m_which = ??   not sure you may want to set this.
    r.m_ptr   = nullptr;
}
Range& operator=(Range&& r)
{
    r.swap(*this);      // Just swap the objects on assignment.
    return *this;
}

StackExchange Code Review Q#52507, answer score: 5