Search code examples
c++msvc-2017

C2665 boost::variant woes


I'm porting a C++ application that was originally built with MSVC2008 and an older version of Boost. I'm porting it to MSVC2017 with Boost 1.73.

I am getting the error:

Severity    Code    Description Project File    Line    Suppression State
Error   C2665   'boost::variant<boost::detail::variant::recursive_flag<T0>,uint16_t,uint32_t,uint64_t,int8_t,int16_t,int32_t,int64_t,boost::uuids::uuid,float,double,bool,char,std::string,std::wstring,Fraenkel::ByteArray,std::vector<boost::recursive_variant_,std::allocator<_Ty>>>::variant': none of the 3 overloads could convert all the argument types Master Communications Service Group c:\fraenkelsoftware-millikan\core\service groups\master communications\code\commanddatafield.h  317 

This is the class prototype:

#if !defined(COMMANDDATAFIELD_H)
#define COMMANDDATAFIELD_H ///< Header guard

#if (_MSC_VER >= 1915)
#define no_init_all deprecated
#endif

#include "fraenkel.h"

#include <boost/mpl/at.hpp>
#include <boost/uuid/uuid.hpp>
#include <boost/variant.hpp>

#include <algorithm>
#include <vector>

/// Fraenkel namespace contains miscellaneous definitions
namespace Fraenkel
{

    class CommandDataField;

    /// For holding binary data
    struct ByteArray
    {
        /// The data.
        std::vector<uint8_t> m_data;
        /// Default constructor: construct an empty byte array. Note that this will
        /// allocate a zero-sized array, so the pointer will always be valid for a
        /// valid ByteArray; you just can't dereference it.
        ByteArray() {}
        /// Construct a byte array with a given size (and undefined contents).
        /// \param  size    Size in bytes of data array.
        explicit ByteArray(size_t size) : m_data(size) {}
        /// Construct a byte array from a given vector of uint8's
        /// (and defined contents, i.e. copy of the data)
        /// \param  source  Data to copy.
        explicit ByteArray(const std::vector<uint8_t>& source) : m_data(source) {}
        /// Move constructor from vector.
        explicit ByteArray(std::vector<uint8_t>&& source)
        {
            std::swap(m_data, source);
        }
        /// Copy constructor.
        ByteArray(const ByteArray& src) : m_data(src.m_data) {}
        /// Move constructor.
        ByteArray(ByteArray&& src)
        {
            std::swap(m_data, src.m_data);
        }
        /// Assignment operator.
        ByteArray& operator=(const ByteArray& src) { m_data = src.m_data; return *this; }
        /// Move assignment.
        ByteArray& operator=(ByteArray&& src) { std::swap(m_data, src.m_data); return *this; }
        /// Proxy for get() (there's a lot of it in the code, since ByteArray used
        /// to be just a typedef for shared_array).
        /// \return Raw pointer to the data.
        const uint8_t* get() const { return m_data.data(); }
        /// Proxy for size() likewise.
        size_t size() const { return m_data.size(); }
    };

    /// Equality comparison for byte arrays.
    inline bool operator==(const ByteArray& a1, const ByteArray& a2)
    {
        return (a1.m_data == a2.m_data);
    }

    /// Uses Boost::Variant to define a variant type which can contain any of the basic data types,
    /// plus strings, wide strings, or another variant (used for lists).
    /// This means a variant may hold data, or another variant, or a list of variants, and so on.
    /// Adding a new type will mean a new core, and probably will mean all components which use this need recompiling.
    /// \remark The CDFVariant should NOT be used directly. Use the CommandDataField wrapper.
    typedef boost::make_recursive_variant<uint8_t, uint16_t, uint32_t, uint64_t, int8_t, int16_t, int32_t, int64_t,
        boost::uuids::uuid, float, double, bool, char,
        std::string, std::wstring, ByteArray,
        std::vector<boost::recursive_variant_>>::type CDFVariant;
    /// Defines a list of variants
    typedef std::vector<CDFVariant> CDFVariantList;

    /// This gives the index of each type in the allowed types list, as returned by
    /// the which() method. There should be a way of getting this directly from the
    /// variant itself using Boost's meta-programming facilities, but I haven't
    /// figured out how to do it.
    template<typename T> struct CDFIndexOf {};
    /// This macro generates a specialisation for a given type at a given index. The
    /// hairy template line is a Boost compile-time assert that the index does in
    /// fact correspond to the correct type.
#define CDF_DECLARE_INDEX(T,i)      \
template<> struct CDFIndexOf<T> {   \
    BOOST_MPL_ASSERT((boost::is_same<T,boost::mpl::at<CDFVariant::types, boost::mpl::integral_c<int,i> >::type>));  \
    static const int value = i;     \
}
    CDF_DECLARE_INDEX(boost::uint8_t,       0); ///< 8-bit unsigned integer types
    CDF_DECLARE_INDEX(boost::uint16_t,      1); ///< 16-bit unsigned integer types
    CDF_DECLARE_INDEX(boost::uint32_t,      2); ///< 32-bit unsigned integer types
    CDF_DECLARE_INDEX(boost::uint64_t,      3); ///< 64-bit unsigned integer types
    CDF_DECLARE_INDEX(boost::int8_t,        4); ///< 8-bit integer types
    CDF_DECLARE_INDEX(boost::int16_t,       5); ///< 16-bit integer types
    CDF_DECLARE_INDEX(boost::int32_t,       6); ///< 32-bit integer types
    CDF_DECLARE_INDEX(boost::int64_t,       7); ///< 64-bit integer types
    CDF_DECLARE_INDEX(boost::uuids::uuid,   8); ///< 128-bit uuid
    CDF_DECLARE_INDEX(float,                9); ///< single-precision float types
    CDF_DECLARE_INDEX(double,               10);///< double-precision float types
    CDF_DECLARE_INDEX(bool,                 11);///< boolean types
    CDF_DECLARE_INDEX(char,                 12);///< character types
    CDF_DECLARE_INDEX(std::string,          13);///< 8-bit string types
    CDF_DECLARE_INDEX(std::wstring,         14);///< wide string types
    CDF_DECLARE_INDEX(ByteArray,            15);///< uint8_t array (for binary data)
    CDF_DECLARE_INDEX(CDFVariantList,       16);///< recursive lists

    /// A simple vector of CommandDataField
    typedef std::vector<CommandDataField> CommandDataFieldList;

    /// \brief Used to identify a position in a CommandDataFieldList
    /// This is an unsigned type according to the standard; no need to compare it against zero.
    typedef CommandDataFieldList::size_type CommandDataFieldIndex;

    ////////////////////////////////////////////////////////////////////////////////
    /// \class CommandDataField
    /// \brief A class that can be used to describe command parameter(s) or reply field(s).
    /// Each CommandDataField contains a CDFVariant member, and it provides a means
    /// of setting and getting this variant.
    class CommandDataField
    {
    public:
        /// Default c'tor
        CommandDataField(void) {}
        /// Default d'tor. No clean up is required.
        ~CommandDataField(void) {}
/**Commented out to prevent C2280 in MSVC2017
        /// Copy constructor. Forwards to the universal constructor.
        CommandDataField(const CommandDataField& source) : CommandDataField(source.m_val) {}
        /// Move constructor. Forwards to the universal constructor.
        CommandDataField(CommandDataField&& source) : CommandDataField(std::move(source.m_val)) {}
*/
        /// Constructor from byte vector. Forwards to ByteArray.
        CommandDataField(std::vector<uint8_t> source) : CommandDataField(ByteArray(std::move(source))) {}
        /// Templated c'tor. Sets the variant with the given value.
        /// Note that T&& here is what Scott Myers calls a "universal reference".
        /// You can't reliably overload a function once you've got a universal
        /// reference in the mix, for reasons which are too involved to go into
        /// here. So following a suggestion of Myers, we forward rvalue and lvalue
        /// references separately to private constructors and do the specialisation
        /// there.
        template<typename T> explicit CommandDataField(T&& t) : CommandDataField(std::forward<T>(t), std::is_rvalue_reference<T&&>()) {}

        /// Test whether the command data field is of a given type.
        /// \return True if held variant's type matches the template argument
        template<typename T>
        bool isType(void) const
        {
            return (which() == CDFIndexOf<T>::value);
        }

        /// Test whether the command data field is of the same type as some other.
        /// \param other Another CommandDataField to check
        /// \return True if the two command data fields contain the same type
        bool isType(const CommandDataField& other) const
        {
            return (which() == other.which());
        }

        /// Array and scalar conversion are sufficiently different that we can't
        /// specialise directly, but we can dispatch through a mini-traits class.
        template<typename T> struct Getter
        {
            static T get(const CommandDataField& cdf) { return cdf.get_s<T>(); }
        };
        /// It is sometimes convenient to get the CommandDataField itself as a
        //  no-op.
        template<> struct Getter<CommandDataField>
        {
            static CommandDataField get(const CommandDataField& cdf) { return cdf; }
        };
        /// Variant list is really a scalar, since it corresponds to the value
        /// stored directly in the data field.
        template<> struct Getter<CDFVariantList>
        {
            static CDFVariantList get(const CommandDataField& cdf) { return cdf.get_s<CDFVariantList>(); }
        };
        /// And the vector version.
        template<typename T> struct Getter<std::vector<T> >
        {
            static std::vector<T> get(const CommandDataField& cdf) { return cdf.get_v<T>(); }
        };
        /// Generic get method that works with array or scalar parameters.
        template<typename T> T get(void) const { return Getter<T>::get(*this); }

        /// get template method returns the value of the variant.
        /// \return The value of the variant
        template <typename T>
        T get_s(void) const
        {
            if (typeid(T) != m_val.type())
            {
                throw std::logic_error("CommandDataField: wrong type for get");
            }
            const T *pX = boost::get<T>(&m_val);
            return *pX;
        }

        /// Specialisation of get() for array types. WARNING slow for large arrays.
        template<typename T> std::vector<T> get_v(void) const
        {
            const CDFVariantList* vs = boost::get<CDFVariantList>(&m_val);
            assert(vs != nullptr);
            std::vector<T> result;
            result.reserve(vs->size());
            for (auto const& v : *vs)
            {
                const T* val = boost::get<T>(&v);
                assert(val != nullptr);
                result.push_back(*val);
            }
            return result;
        }


        /// Set template method changes the value of the variant.
        /// \param t Data to set. Must be one of the variant's supported types.
        template <typename T> void set(const T& t) { m_val = t; }

        /// returns the size of the stored array (and will assert if there is no array stored)
        /// \return size
        size_t getArraySize(void) const
        {
            assert(m_val.type() == typeid(ByteArray));
            return get_s<ByteArray>().size();
        }

        /// Overload of set for handling lumps of binary data
        /// which we will treat as lists of uint8_t's
        /// \param pData Pointer to binary data
        /// \param size Size in bytes of pData
        void set(const uint8_t* pData, const size_t size)
        {
            ByteArray temp(size);
            memcpy(temp.m_data.data(), pData, size);
            m_val = std::move(temp);
        }

        /// Specialisation for array types (slow for large arrays)
        template<typename T> void set(const std::vector<T>& t)
        {
            CDFVariantList temp;
            temp.reserve(t.size());
            std::copy(t.begin(), t.end(), std::back_inserter(temp));
            m_val = temp;
        }

        /// Uber-specialisation for CDFVariantList, which can be set directly.
        void set(const CDFVariantList& t) { m_val = t; }
        /// Move version of the variant-list setter.
        void set(CDFVariantList&& t) { m_val = std::move(t); }

        /// Comparison operator: compares the underlying variants.
        /// Boost will provide operator!= automatically thanks to the
        /// equality_comparable template.
        /// \param  rhs Comparand.
        /// \return true if the two fields are equal (type and value).
        bool operator==(const CommandDataField& rhs) const
        {
            return (m_val == rhs.m_val);
        }

        /// Provides a means of accessing the underlying variant so that the visitor
        /// pattern can be used. See boost::apply_visitor.
        /// \param visitor A boost::static_visitor
        /// \return Visitor-appropriate return value
        template <typename Visitor>
        typename Visitor::result_type applyVisitor(Visitor& visitor) const
        {
            return boost::apply_visitor(visitor, m_val);
        }
        /// Visitor, const version.
        /// \param visitor A boost::static_visitor
        /// \return Visitor-appropriate return value
        template<typename Visitor>
        typename Visitor::result_type applyVisitor(const Visitor& visitor) const
        {
            return boost::apply_visitor(visitor, m_val);
        }

    private:
        /// Generic constructor for lvalue references; copies into the underlying
        /// variant.
        template<typename T> explicit CommandDataField(const T& t, std::false_type) : m_val(t) {}
        /// Generic constructor for rvalue references; moves into the underlying
        /// variant.
        template<typename T> explicit CommandDataField(T&& t, std::true_type) : m_val(std::forward<T>(t)) {}
        /// Vector constructor; forwards to set().
        template<typename T> explicit CommandDataField(const std::vector<T>& v, std::false_type) { set(v); }
        /// Rvalue vector constructor; forwards to set() (this won't be any
        /// different from the lvalue version except in the case of a variant list,
        /// which can be set directly).
        template<typename T> explicit CommandDataField(std::vector<T>&& v, std::true_type) { set(std::move(v)); }
        /// Alternative copy constructor for copy operations that were erroneously
        /// caught by the universal constructor, because C++.
        explicit CommandDataField(const CommandDataField& rhs, std::false_type) : m_val(rhs.m_val) {}
        /// Get the type as an index into the list. Not for user consumption: is
        /// used for type checking.
        /// \return The list index of the given variant type
        int which(void) const { return m_val.which(); }

        CDFVariant m_val; ///< The variant that this class wraps.
    };

    /// This visitor extracts the variant corresponding to the calling field.
    class IdentityVisitor
    {
    public:
        typedef CDFVariant result_type; ///< Returns a CDFVariant
        template<typename T> CDFVariant operator()(const T& val) const { return val; }
    };

    ////////////////////////////////////////////////////////////////////////////////
    /// Pack a CommandDataFieldList into a single CommandDataField of vector type.
    /// \param cdfs List of command data fields to pack
    /// \return resulting single command data field
    inline CommandDataField pack(const CommandDataFieldList& cdfs)
    {
        // A functor to transform a CommandDataField into its underlying variant.
        // I'm sure you ought to be able to do this with lambda functions, but I
        //  haven't been able to get it to compile.
        class Transform
        {
        public:
            CDFVariant operator()(const CommandDataField& cdf) const { return cdf.applyVisitor(IdentityVisitor()); }
        };
        // Variant list for the result.
        CDFVariantList vs;
        std::transform(cdfs.begin(), cdfs.end(), std::back_inserter(vs), Transform());
        return CommandDataField(std::move(vs));
    }

    ////////////////////////////////////////////////////////////////////////////////
    /// Template function to convert an arbitrary data type to a CDF. Useful in
    /// functional algorithms manipulating containers.
    ///
    /// \param  t   Incoming data to convert.
    /// \return Command data field containing the data.
    ///
    template<typename T> Fraenkel::CommandDataField makeCDF(const T& t)
    {
        return Fraenkel::CommandDataField(t);
    }

} //end Fraenkel namespace

#endif //!defined(COMMANDDATAFIELD_H)

Line 317 is this in the class:

template<typename T> explicit CommandDataField(T&& t, std::true_type) : m_val(std::forward<T>(t)) {}

Solution

  • With way how the call is now , the conversion from CDFVariantList&& to CommandDataField is not possible, which produces that error due to return in pack. It results in call to

    template<typename T> 
    explicit CommandDataField(T&& t) : 
        CommandDataField(std::forward<T>(t), std::is_rvalue_reference<T&&>()) 
    {}
    

    but there is no CommandDataField ctors that take such two parameters.In legacy code it used the non-template move ctor. If template one was still present, it wasn't considered anyway.

    The the oddly overloaded (why not to use SFINAE?)

    template<typename T> explicit CommandDataField(T&& t, std::true_type) 
    : m_val(std::forward<T>(t)) 
    {}
    

    got the problem is that T&& equals to CommandDataField&& there and CDFVariant is attempted to be constructed with CommandDataField&&. Which it cannot be. Perhaps it should be a new definition.

     explicit CommandDataField(CommandDataField&& rhs, std::true_type)
       : m_val(std::move(rhs.m_val))
     {}
    

    The entire implementation of class could be simpler, because it seems that your use-cases fit into default behavior.

        ///////////////////////////////////////////////////////
        /// \class CommandDataField
        /// \brief A class that can be used to describe command parameter(s) or reply field(s).
        /// Each CommandDataField contains a CDFVariant member, and it provides a means
        /// of setting and getting this variant.
        class CommandDataField
        {
        public:
            /// Default c'tor
            CommandDataField(void) = default;
            /// Default d'tor. No clean up is required.
            ~CommandDataField(void) = default;
    
            /// Copy constructor. Forwards to the universal constructor.
            CommandDataField(const CommandDataField& source) : CommandDataField(source.m_val) {}
            /// Move constructor. Forwards to the universal constructor.
            CommandDataField(CommandDataField&& source) : CommandDataField(std::move(source.m_val)) {}
    
            CommandDataField& operator= ( const CommandDataField& source ) = default;
            CommandDataField& operator= ( CommandDataField&& source ) = default;
            
            /// Constructor from byte vector. Forwards to ByteArray.
            CommandDataField(std::vector<uint8_t> source) : CommandDataField(ByteArray(std::move(source))) {}
            /// Templated c'tor. Sets the variant with the given value.
            /// Note that T&& here is what Scott Myers calls a "universal reference".
            /// You can't reliably overload a function once you've got a universal
            /// reference in the mix, for reasons which are too involved to go into
            /// here. So following a suggestion of Myers, we forward rvalue and lvalue
            /// references separately to private constructors and do the specialisation
            /// there.
            template<typename T> 
            explicit CommandDataField(T&& v) : m_val(std::forward<T>(v)) {}
    
            /// Test whether the command data field is of a given type.
            /// \return True if held variant's type matches the template argument
            template<typename T>
            bool isType(void) const
            {
                return (which() == CDFIndexOf<T>::value);
            }
    
            /// Test whether the command data field is of the same type as some other.
            /// \param other Another CommandDataField to check
            /// \return True if the two command data fields contain the same type
            bool isType(const CommandDataField& other) const
            {
                return (which() == other.which());
            }
    
            /// Array and scalar conversion are sufficiently different that we can't
            /// specialise directly, but we can dispatch through a mini-traits class.
            template<typename T> struct Getter
            {
                static T get(const CommandDataField& cdf) { return cdf.get_s<T>(); }
            };
           
            /// And the vector version.
            template<typename T> struct Getter<std::vector<T> >
            {
                static std::vector<T> get(const CommandDataField& cdf) { return cdf.get_v<T>(); }
            };
            /// Generic get method that works with array or scalar parameters.
            template<typename T> T get(void) const { return Getter<T>::get(*this); }
    
            /// get template method returns the value of the variant.
            /// \return The value of the variant
            template <typename T>
            T get_s(void) const
            {
                if (typeid(T) != m_val.type())
                {
                    throw std::logic_error("CommandDataField: wrong type for get");
                }
                const T *pX = boost::get<T>(&m_val);
                return *pX;
            }
    
            /// Specialisation of get() for array types. WARNING slow for large arrays.
            template<typename T> std::vector<T> get_v(void) const
            {
                const CDFVariantList* vs = boost::get<CDFVariantList>(&m_val);
                assert(vs != nullptr);
                std::vector<T> result;
                result.reserve(vs->size());
                for (auto const& v : *vs)
                {
                    const T* val = boost::get<T>(&v);
                    assert(val != nullptr);
                    result.push_back(*val);
                }
                return result;
            }
    
    
            /// Set template method changes the value of the variant.
            /// \param t Data to set. Must be one of the variant's supported types.
            template <typename T> void set(const T& t) { m_val = t; }
    
            /// returns the size of the stored array (and will assert if there is no array stored)
            /// \return size
            size_t getArraySize(void) const
            {
                assert(m_val.type() == typeid(ByteArray));
                return get_s<ByteArray>().size();
            }
    
            /// Overload of set for handling lumps of binary data
            /// which we will treat as lists of uint8_t's
            /// \param pData Pointer to binary data
            /// \param size Size in bytes of pData
            void set(const uint8_t* pData, const size_t size)
            {
                ByteArray temp(size);
                memcpy(temp.m_data.data(), pData, size);
                m_val = std::move(temp);
            }
    
            /// Specialisation for array types (slow for large arrays)
            template<typename T> void set(const std::vector<T>& t)
            {
                CDFVariantList temp;
                temp.reserve(t.size());
                std::copy(t.begin(), t.end(), std::back_inserter(temp));
                m_val = temp;
            }
    
            /// Uber-specialisation for CDFVariantList, which can be set directly.
            void set(const CDFVariantList& t) { m_val = t; }
            /// Move version of the variant-list setter.
            void set(CDFVariantList&& t) { m_val = std::move(t); }
    
            /// Comparison operator: compares the underlying variants.
            /// Boost will provide operator!= automatically thanks to the
            /// equality_comparable template.
            /// \param  rhs Comparand.
            /// \return true if the two fields are equal (type and value).
            bool operator==(const CommandDataField& rhs) const
            {
                return (m_val == rhs.m_val);
            }
    
            /// Provides a means of accessing the underlying variant so that the visitor
            /// pattern can be used. See boost::apply_visitor.
            /// \param visitor A boost::static_visitor
            /// \return Visitor-appropriate return value
            template <typename Visitor>
            typename Visitor::result_type applyVisitor(Visitor& visitor) const
            {
                return boost::apply_visitor(visitor, m_val);
            }
            /// Visitor, const version.
            /// \param visitor A boost::static_visitor
            /// \return Visitor-appropriate return value
            template<typename Visitor>
            typename Visitor::result_type applyVisitor(const Visitor& visitor) const
            {
                return boost::apply_visitor(visitor, m_val);
            }
    
        private:
            int which(void) const { return m_val.which(); }
    
            CDFVariant m_val; ///< The variant that this class wraps.
        };
    
        /// It is sometimes convenient to get the CommandDataField itself as a
        //  no-op.
        template<> 
        struct CommandDataField::Getter<CommandDataField>
        {
           static CommandDataField get(const CommandDataField& cdf) { return cdf; }
        };
        
        /// Variant list is really a scalar, since it corresponds to the value
        /// stored directly in the data field.
        template<> 
        struct CommandDataField::Getter<CDFVariantList>
        {
                static CDFVariantList get(const CommandDataField& cdf) { return cdf.get_s<CDFVariantList>(); }
        };
    

    Note, that I moved Getter specializations out of class. That's compliant notation.

    There is an odd constructor overload that uses move semantic from vector to ByteArray.

    And I don't see (maybe something missing, provided I don't know use cases in your project), why the whole boilerplate can be avoided. That is, leave only explicit template constructor and replace all internalizes in code with universal ones, e.g.

    ByteArray b;
    CDFVariant a {b};
    CommandDataField field{std::vector<CDFVariant>{ a , b }};
    

    would work perfectly