
Neil Mayhew wrote:
On 28/05/08 05:50 PM Neil Mayhew wrote:
Perhaps the simplest and best solution is therefore:
class endian< ... > : cover_operators< ... > { public: #if defined(CXX_0X) || !defined(ENDIANS_IN_UNIONS) endian() {} endian(T val) { ... } #endif
I just discovered that an operator=(T) is needed as well:
endian& operator=(T i) { detail::store_big_endian<T, n_bits/8>(bytes, i); }
Without this, and without the endian(T val) constructor, a lot of things just don't work - for example, the binary arithmetic operators.
This makes me think that there should have been an operator= all along. For a start, this is just good practice: anywhere there's a constructor initializing from a particular type, there should usually also be an assignment operator taking the same type.
Did you get a chance to look at the new version in the vault? See the message I posted two or three days ago: http://tinyurl.com/6xejo5 It provides operator=(T), for the reasons you identified.
Second, I think the generated code for binary operators must have been suboptimal, since it seems that the computed result (a native integer) of adding two endians was being used to construct a temporary endian which was then copy-assigned into the actual result. At least, that's my interpretation of the compilation errors I was getting before I put operator= in. To test this, take out the constructors and do:
big32_t a, b; nt32_t i; i = a + b;
operator+(endian, endian) is implemented using cover_operators::operator+=, which is defined as x = +x + y, hence uses assignment of the result of adding two native integers converted from the respective endians. If you're assigning the result to a native type anyway, then returning an endian is inefficient, even with operator= defined. I'm not sure how to fix this, without abandoning the use of boost::operators. In fact, I am beginning to wonder whether the traditional approach to binary operators, which returns the same type as its two arguments, and implements by calling +=, is not actually appropriate for endian.
I've got similar concerns, plus the use of boost::operators impact on PODness under the current standard. --Beman