Multiple copy constructors

[wcassella]

Hi,

I'm building a non-templated weak pointer type, similar to boost::any in that it knows the type of the underlying value it points to, except it does not copy (it only takes the address). I realize that's not the safest thing in the world, but in the limited context I'm using it its fine.

Also, to make things less confusing I'll call my type "Reference" (capital R), and the C++ language version "reference" (like int&).

Anyway, I have two copy constructors for my Reference class: one accepts a const reference (to a Reference), while the other accepts a non-const reference. The reason being is the non-const copy constructor copies the mutability (held as a boolean) of the Reference, while the const copier copies it as an immutable Reference (which causing an error if you try to cast the Reference into a non-const reference). MSVC doesn't like that, and gives me the warning C4521 - "multiple copy constructors specified". Clang doesn't seem to mind though, which is interesting since MSVC has let me get away with more non-standard stuff than Clang in the past.

At the moment I'm considering abandoning the concept of Immutable References, because all it does is enforce const-correctness at runtime which is actually causing more issues than it solves (not only do you have to check the Type and Validity of the Reference, but you also need to check its mutability). I could do this by only allowing creation of References to non-const data, or by using a const_cast.

What should I do?

Thanks,

Will Cassella

[JLBorges]

> Anyway, I have two copy constructors for my Reference class:
> one accepts a const reference (to a Reference), while the other accepts a non-const reference.

That is fine.

[Note: All forms of copy/move constructor may be declared for a class.

[Example:

struct X {
    X(const X&);
    X(X&); // OK
    X(X&&);
    X(const X&&); // OK, but possibly not sensible
};

—end example ]

—end note ] - IS

> What should I do?

Hard to say, without seeing what the class looks like.

[wcassella]

I've included a portion of the class header. There's some stuff missing (the Unpack<T>(Reference) functions are not included), but you get the gist of it. I still can't figure out why exactly MSVC would throw a warning for this, as it seems like perfectly reasonable behavior to me.

/** Weak pointer that holds a reference to an instance of a reflected type */
class UTILITY_API Reference
{
	///////////////////////
	///   Information   ///
public:

	template <typename AnyType> friend struct __unpack__;		

	////////////////////////
	///   Constructors   ///
public:

	/** Construct default (null) reference */
	Reference();

	/** Construct reference if you already know the type */
	Reference(void* value, const Type& type, bool immutable = false);

        /** Copy mutability of copy */
	Reference(Reference& copy);

        /** Copy immutable reference from copy */
	Reference(const Reference& copy);
	Reference(Reference&& move);

        /** Construct mutable reference to value */
	template <typename AnyType>
	Reference(AnyType& value)
	{
		this->_value = &value;
		this->_type = &TypeInfo(value);
		this->_isImmutable = false;
	}

        /** Construct immutable reference to value */
	template <typename AnyType>
	Reference(const AnyType& value)
	{
		this->_value = const_cast<AnyType*>(&value);
		this->_type = &TypeInfo(value);
		this->_isImmutable = true;
	}

	///////////////////
	///   Methods   ///
public:

	/** Formats the state of this reference as a String */
	String ToString() const;

	/** Returns the underlying type of the reference */
	const Type& GetType() const;

	/** Returns whether this reference points to a value */
	bool IsNull() const;

	/** Returns whether this reference is immutable */
	bool IsImmutable() const;

	/////////////////////
	///   Operators   ///
public:

        /** Nullify this reference */
	Reference& operator=(std::nullptr_t);

        /** Assign mutable reference from copy */
        Reference& operator=(Reference& copy);

        /** Assign immutable reference from copy */
	Reference& operator=(const Reference& copy);
	Reference& operator=(Reference&& move);

        /** Assign mutable reference to value */
	template <typename AnyType>
	Reference& operator=(AnyType& value)
	{
		this->_value = &value;
		this->_type = &TypeInfo(value);
		this->_isImmutable = false;

		return This;
	}

        /** Assign immutable reference to value */
	template <typename AnyType>
	Reference& operator=(const AnyType& value)
	{
		this->_value = const_cast<AnyType*>(&value);
		this->_type = &TypeInfo(value);
		this->_isImmutable = true;

		return This;
	}
		
	////////////////
	///   Data   ///
private:

	void* _value;
	const Type* _type;
	bool _isImmutable;
};

Anyway, is ensuring runtime const correctness worth the trouble, strange warnings or no?

Thanks

[JLBorges]

> template <typename AnyType> friend struct __unpack__;

Avoid using names containing a double underscore.

Certain sets of names and function signatures are always reserved to the implementation:
— Each name that contains a double underscore __ or begins with an underscore followed by an uppercase letter is reserved to the implementation for any use.
— Each name that begins with an underscore is reserved to the implementation for use as a name in the global namespace.

Isn't this is a somewhat unusual (unintuitive) interpretation of const-ness?

/** Copy immutable reference from copy */
Reference(const Reference& copy);

That const Reference means not only that the object is const, but also that the object referred to by it is const.

int value = 0 ;
const int cvalue = 0 ;
int* p = std::addressof(value) ; // (non-cost) pointer to (non-cost) int // 1
const int* pc = std::addressof(value) ; // (non-cost) pointer to const int // 2
int* const cp = std::addressof(value) ; // const pointer to (non-cost) int // 3
const int* const cpc = std::addressof(value) ; // const pointer to const int // 4

Reference r(value) ; // (non-cost) Reference to (non-cost) int // 1
Reference rc(cvalue) ; // (non-cost) Reference to const int // 2
const Reference cr(value) ; // const Reference to (non-cost) int // 3
const Reference crc(cvalue) ; // const Reference to const int // 4 

Leaving that aside for the moment,

This is dangerous

        /** Construct immutable reference to value */
	template <typename AnyType>
	Reference(const AnyType& value)
	{
		this->_value = const_cast<AnyType*>(&value);
		this->_type = &TypeInfo(value);
		this->_isImmutable = true;
	}

without also having

template <typename AnyType>
	Reference( AnyType&& value ) = delete ;

Can't you ensure runtime const correctness by storing the type_info or type_index of the pointer?
Though, typeid(int) == typeid( const int ), typeid(int*) != typeid( const int* )

Something like this, perhaps:

struct Reference
{
    Reference() = default ;

    template < typename T > Reference( T& value ) noexcept
        : _type( typeid(T*) ), _value( std::addressof(value) ) {}

    template < typename T > Reference( const T& value ) noexcept
        : _isImmutable(true), _type( typeid( const T* ) ), _cvalue( std::addressof(value) ) {}

    // ***** added *****
    template < typename T > Reference( T&& ) = delete ; // **** do not store address of rvalue

    // implicitly declared
    // Reference( const Reference& that ) noexcept = default ;
    // Reference( Reference&& that ) noexcept = default ;


    // likewise for assignment

    // ***** added *****
    // template < typename T > operator= ( T&& ) = delete ; // **** do not store address of rvalue

    private:

        bool _isImmutable = false ;
        std::type_index _type = typeid(void*) ;

        union
        {
            void* _value = nullptr ;
            const void* _cvalue ;
        };
};

[wcassella]

Thanks! After some thought I decided to ditch having a const Reference force its referenced value to be const as well (so now there's only one copy constructor), though it still won't let you unpack an immutable Reference into a non-const reference. I also got rid of the double underscores, didn't know that was an issue.