Re: [boost] [integer] Promotion of endian library code from vault

From: Beman Dawes (bdawes_at_[hidden])
Date: 2008-05-29 20:49:16

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