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@boost-consulting.com> To: <boost@lists.boost.org> 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