AnySetValue - a set of with multiple types of values

I would like a review for this code which is used apparently successfully in my current project. More info are available in this gist but I'm pasting here the main info:


The project use C++11/14 as provided by VS2013 (no CTP) and Boost. I
didn't try to compile it in other compilers yet (lack of time and
resources). The tests are provided in another other file, it compiles
and run for me but the tests themselves might require review too.
Tests are using GTest. I use custom assert macros which I will not
explain here as they are obvious to understand.


AnyValueSet: I use this container more or less as a message board, the
values being the messages. Sometime, I use types which are compared
using only part of their value and contain more data that are not used
in comparison, which is why there are overwritting functions.

I would like reviews in particular concerning correctness (including the tests), performance and security.
I expose this code here because I am alone on my (big) project and I fear the lack multiple eyes on the code, in particular for this part of code which is reused several time in the project.

Note that this container isn't supposed to be as generic as a standard or boost container but is specific to my use (which I think should be clear from the tests).

The full code, extracted from my project (which explains the namespace):

```
#ifndef HGUARD_NETRUSH_INPUT_ANYVALUESET_HPP__
#define HGUARD_NETRUSH_INPUT_ANYVALUESET_HPP__

#include
#include
#include

#include
#include
#include

#include

namespace netrush {
namespace input {

/** Set that can contain multiple value types.
The contained types must be Comparable, Copyable and CopyConstructible.
The identity of values is determined by comparison (operator
bool add( ValueType&& value )
{
auto& value_set = find_or_create_set();
bool was_added = value_set.add( std::forward(value) );
if( was_added )

I don't see any real problem, only tidbits and things that could potentially be done differently:

• Your header guard name is actually reserved to the implementation since it uses a double underscore.

• Moving integer types seems useless. You will probably get even perfomance if you simply copy them.

-
You can use list initialization in the constructor initialization lists to prevent implicit narrowing type conversions. Be careful, it does not work with some special types (references for example).

AnyValueSet( AnyValueSet&& other )
    : m_size_changed{ std::move(other.m_size_changed) }
    , m_size{ std::move(other.m_size) }
    , m_set_index{ std::move(other.m_set_index) }
{}

But overall, your code seems pretty good: the style is good (indentation, variable names, redability, proper use of override to prevent surprises, etc...) and the internal logic also seems good (type erasure, use of mutable to cache values, efficient use of iterators, etc...). Therefore, I don't think that you have to worry :)

Ok, I tried to compile your code with GCC, and it seems that you actually have errors that MSVC is not able to catch:

• typename ValueType = std::decay::type actually produces an error. It seems that MSVC does not properly handles the dependant names. You need to add typename before std::decay<>.

-
I also got a warning concerning the order of initialization of the class members in your move constructor: in idiomatic code, the members should be initialized in the order they were declared. The order of initialization should be as follows:

AnyValueSet( AnyValueSet&& other )
    : m_set_index( std::move(other.m_set_index) )
    , m_size_changed( std::move(other.m_size_changed) )
    , m_size( std::move(other.m_size) )
{}

StackExchange Code Review Q#47152, answer score: 5