Re: [boost] [serialization] patch for portable_binary_oarchive.hpp

From: Boris Gubenko (Boris.Gubenko_at_[hidden])
Date: 2007-11-02 23:00:25

David Abrahams wrote:
> [...]                            If you want to assert your change is
> portable, please show me where in the standard it defines the behavior
> of your specific usage.

No, it is not portable, it just "works" :-)

>> In terms of style, I think that your
>> change is better.
>
> Hah, I think it's stylistically distasteful, but at least it's
> correct.  I wish that it were properly spelled
> reinterpret_cast<char*>(&l) ;-)

The generated code is the same with both your change (a chain of
static_cast's) and my change (reinterpet_cast). This is the case with
every existing compiler. However, unlike my change, your change is
portable. This is what I meant by a better style: a portable
construct vs. non-portable.

Thanks for the explanation.

Boris

----- Original Message ----- 
From: "David Abrahams" <dave_at_[hidden]>
To: <boost_at_[hidden]>
Sent: Friday, November 02, 2007 10:28 PM
Subject: Re: [boost] [serialization] patch for portable_binary_oarchive.hpp

>
> on Fri Nov 02 2007, "Boris Gubenko" <Boris.Gubenko-AT-hp.com> wrote:
>
>> David Abrahams wrote:
>>> I can't imagine how your change would fix an endianness problem.
>>
>> It does not fix "an endianness problem". The bug is in the code
>> conditionalized with '#ifdef BOOST_BIG_ENDIAN'. This is the only
>> connection between endianness and the fix.
>>
>>> Also, your change relies on the nonportable (implementation-defined)
>>> behavior of reinterpret_cast.
>>
>> It does. As well as other Boost code using reinterpret_cast :-)
>>
>> Scanning Boost sources (trunk) for reinterpret_cast yields 700 hits,
>> including 8 instances in the Boost python library (two of them are
>> commented out):
>
> Those are most likely all wrong; they should be looked at one by one
> and fixed.
>
>> bash-2.03$ find ./boost/libs/python -type f -exec grep reinterpret_cast {} \;
>>       instance<>* self = reinterpret_cast<instance<>*>(inst);
>>         _doc += str(reinterpret_cast<const char*>(detail::py_signature_tag));
>> //            _doc += "\n"+str(reinterpret_cast<const
>> char*>(detail::cpp_signature_tag));
>>             _doc += str(reinterpret_cast<const
>> char*>(detail::cpp_signature_tag));
>>       instance<>* self = reinterpret_cast<instance<>*>(inst);
>>         _doc += str(reinterpret_cast<const char*>(detail::py_signature_tag));
>> //            _doc += "\n"+str(reinterpret_cast<const
>> char*>(detail::cpp_signature_tag));
>>             _doc += str(reinterpret_cast<const
>> char*>(detail::cpp_signature_tag));
>> bash-2.03$
>
> I am apparently responsible for one of those, though only a careful
> combing through history will tell for sure.  I need to bother other
> people about the other instances.
>
>> You are, probably, referring to 5.2.10 - Reinterpret cast
>> [expr.reinterpret.cast], para 3:
>>
>>  -3- The mapping performed by reinterpret_cast is implementation-defined.
>>      [Note: it might, or might not, produce a representation different
>>      from the original value. ]
>>
>> In my fix, reinterpret_cast performs a mapping from non-null pointer to
>> long to pointer to char. If reinterpret_cast cannot be used in this case
>> without invoking implementation-defined behavior, I'm not sure how it can
>> be used in a multiplatform code like Boost at all.
>
> Very perceptive.  There are almost no ways to use reinterpret_cast
> portably.  If you want to get a char* that traverses the bytes of a
> long, you do it as below:
>
>>> I think the correct change to silence the type conversion error is
>>>
>>>   char * first = static_cast<char *>(static_cast<void *>(const_cast<long*>(&
>>> l)));
>>
>> I'm not sure my change is incorrect.
>
> It is if you're trying to write portable code.  You've already found
> the place in the standard where it says reinterpret_cast performs an
> implementation-defined mapping.  If you want to assert your change is
> portable, please show me where in the standard it defines the behavior
> of your specific usage.
>
>> In terms of style, I think that your
>> change is better.
>
> Hah, I think it's stylistically distasteful, but at least it's
> correct.  I wish that it were properly spelled
> reinterpret_cast<char*>(&l) ;-)
>
>
> -- 
> Dave Abrahams
> Boost Consulting
> http://www.boost-consulting.com
>
> _______________________________________________
> Unsubscribe & other changes: http://lists.boost.org/mailman/listinfo.cgi/boost
>