Re:[boost] [serialization] CW problems...
Rene Rivera wrote:
Robert Ramey wrote:
While your looking at CW, might you find the time to look a little bit into the serialization library build problems. The error message is pretty bizarre so I don't think it can be addressed without having CW installed on ones machine.
OK, I'll try... Should I look for anything in specific? Or is it a general health issue?
I'm looking at this and one of the basic problems is that you seem to assume a variety of things are available in the global namespace, specifically size_t and other std types. Those are easy to fix by specifying std::size_t. But I do have some questions...
1. Is using an unqualified size_t your intention? And therefore something else is broken?
Wow - its amazing to me I could have come so far and not know that size_t wasn't a built in type or macro - I think it used to be a macro definition. Anyway, I'll qualify this with std::size_t and check that no other compilers complain. If fact, now I can't understand why no other compiler complained since I never included <cstddef>
2. There is a similar problem with mbstate_t, but in this case you attempt to do something special to handle it (from boost/archive/codecvt_null.hpp):
#if BOOST_WORKAROUND(__BORLANDC__, <= 0x564) #ifndef _RWSTD_NO_NAMESPACE using std::codecvt; using std::min; #ifdef _RWSTD_NO_MBSTATE_T using std::mbstate_t; #endif #endif #elif defined(__COMO__) || defined(_MSC_VER) && _MSC_VER <= 1300 || defined(__BORLANDC__) typedef ::mbstate_t mbstate_t; #elif defined(BOOST_NO_STDC_NAMESPACE) typedef std::mbstate_t mbstate_t; namespace std{ using ::codecvt; } // namespace std #endif
And then you go ahead to use an unqualified mbstate_t. This just doesn't work for CW. And I don't see how this would work for any library runtime that puts mbstate_t only in the std namespace. Could you explain what your intent is for this?
The situation with mbstate_t is problematic. Some compilers define it in the global namespace while others define it in std::mbstate_t. The intent is to permit the code to use mbstate_t without qualification for those compilers that don't define it as std::mbstate_t. This sort of grew one compiler at time without too much thought. The typedef ::mbstate_t mbstate_t makes me wonder what I was thinking. Following the logic here for CW this would add #elif defined(CW) using std::mbstate_t; so that the rest of the code could use unqualified mbstate_t . I'll fix this (and a bunch other stuff) and we'll try again later. Thanks a lot. Robert Ramey
"Robert Ramey" <ramey@rrsd.com> wrote in message news:20040718020109.784743197E@acme.west.net...
Rene Rivera wrote:
I'm looking at this and one of the basic problems is that you seem to assume a variety of things are available in the global namespace, specifically size_t and other std types. Those are easy to fix by specifying std::size_t. But I do have some questions...
1. Is using an unqualified size_t your intention? And therefore something else is broken?
Wow - its amazing to me I could have come so far and not know that size_t wasn't a built in type or macro - I think it used to be a macro definition. Anyway, I'll qualify this with std::size_t and check that no other compilers complain. If fact, now I can't understand why no other compiler complained since I never included <cstddef>
<Butting in...> If you include <boost/config.hpp> before <cstddef> then size_t and ptrdiff_t get put in namespace std for standard libraries which only define them in the global namespace. Jonathan
"Robert Ramey" <ramey@rrsd.com> writes:
And then you go ahead to use an unqualified mbstate_t. This just doesn't work for CW. And I don't see how this would work for any library runtime that puts mbstate_t only in the std namespace. Could you explain what your intent is for this?
The situation with mbstate_t is problematic. Some compilers define it in the global namespace while others define it in std::mbstate_t. The intent is to permit the code to use mbstate_t without qualification for those compilers that don't define it as std::mbstate_t. This sort of grew one compiler at time without too much thought. The typedef ::mbstate_t mbstate_t makes me wonder what I was thinking. Following the logic here for CW this would add
#elif defined(CW) using std::mbstate_t;
so that the rest of the code could use unqualified mbstate_t .
Seems wrong; standard is that it's only in std:: if you're not including any of the <xxx.h> headers. You ought to look for it there by default and only pluck it from the global namespace on implementations that you know to be nonconforming in this regard. Same with size_t. -- Dave Abrahams Boost Consulting http://www.boost-consulting.com
David Abrahams wrote:
"Robert Ramey" <ramey@rrsd.com> writes:
And then you go ahead to use an unqualified mbstate_t. This just doesn't work for CW. And I don't see how this would work for any library runtime that puts mbstate_t only in the std namespace. Could you explain what your intent is for this?
The situation with mbstate_t is problematic. Some compilers define it in the global namespace while others define it in std::mbstate_t. The intent is to permit the code to use mbstate_t without qualification for those compilers that don't define it as std::mbstate_t. This sort of grew one compiler at time without too much thought. The typedef ::mbstate_t mbstate_t makes me wonder what I was thinking. Following the logic here for CW this would add
#elif defined(CW) using std::mbstate_t;
so that the rest of the code could use unqualified mbstate_t .
Seems wrong; standard is that it's only in std:: if you're not including any of the <xxx.h> headers. You ought to look for it there by default and only pluck it from the global namespace on implementations that you know to be nonconforming in this regard. Same with size_t.
Not only that... The code that's doing this is boost-root/boost/utf8_codecvt_facet.hpp which happens to be somewhat of a duplicate for the file boost-root/boost/program_options/utf8_codecvt_facet.hpp. But with the added disturbance of defining the class in the global namespace instead of a namespace within the boost group. Perhaps the intent was to place the entire thing inside some namespace and hence the mbstate_t would not collide? -- -- Grafik - Don't Assume Anything -- Redshift Software, Inc. - http://redshift-software.com -- rrivera/acm.org - grafik/redshift-software.com - 102708583/icq
"Rene Rivera" <grafik.list@redshift-software.com> wrote:
Not only that... The code that's doing this is boost-root/boost/utf8_codecvt_facet.hpp which happens to be somewhat of a duplicate for the file boost-root/boost/program_options/utf8_codecvt_facet.hpp.
boost-root/boost/utf8_codecvt_facet.hpp is original. boost-root/boost/program_options/utf8_codecvt_facet.hpp + *.cpp is slightly modified copy. Intention is that utf8_codecvt_facet will become standalone library at some time and these files will be replaced. /Pavel
Pavel Vozenilek wrote:
"Rene Rivera" <grafik.list@redshift-software.com> wrote:
Not only that... The code that's doing this is boost-root/boost/utf8_codecvt_facet.hpp which happens to be somewhat of a duplicate for the file boost-root/boost/program_options/utf8_codecvt_facet.hpp.
boost-root/boost/utf8_codecvt_facet.hpp is original.
boost-root/boost/program_options/utf8_codecvt_facet.hpp + *.cpp is slightly modified copy.
Intention is that utf8_codecvt_facet will become standalone library at some time and these files will be replaced.
Then it should be moved to boost.detail, directory and namespace. -- -- Grafik - Don't Assume Anything -- Redshift Software, Inc. - http://redshift-software.com -- rrivera/acm.org - grafik/redshift-software.com - 102708583/icq
Rene Rivera <grafik.list@redshift-software.com> writes:
boost-root/boost/utf8_codecvt_facet.hpp is original. boost-root/boost/program_options/utf8_codecvt_facet.hpp + *.cpp is slightly modified copy. Intention is that utf8_codecvt_facet will become standalone library at some time and these files will be replaced.
Then it should be moved to boost.detail, directory and namespace.
Right. We should never knowingly allow (near) duplicate code in the repository. -- Dave Abrahams Boost Consulting http://www.boost-consulting.com
David Abrahams wrote:
Rene Rivera <grafik.list@redshift-software.com> writes:
boost-root/boost/utf8_codecvt_facet.hpp is original. boost-root/boost/program_options/utf8_codecvt_facet.hpp + *.cpp is slightly modified copy. Intention is that utf8_codecvt_facet will become standalone library at some time and these files will be replaced.
Then it should be moved to boost.detail, directory and namespace.
Right. We should never knowingly allow (near) duplicate code in the repository.
Then I can only repeat the question I've asked in another email -- do you think it's OK to add a buildable library (which probably should be in regression tests as well), without review. If you tell how it can be done, I can create such a library. For now, I think I'd rather take my modified version, on the grounds that program_options was comitted earlier, and I've made utf8 work everywhere it was possible. Serialization's version currently surely fails on CW, and results for several other toolsets are not available since Spirit #errors saying they are not supported. - Volodya
Vladimir Prus <ghost@cs.msu.su> writes:
David Abrahams wrote:
Rene Rivera <grafik.list@redshift-software.com> writes:
boost-root/boost/utf8_codecvt_facet.hpp is original. boost-root/boost/program_options/utf8_codecvt_facet.hpp + *.cpp is slightly modified copy. Intention is that utf8_codecvt_facet will become standalone library at some time and these files will be replaced.
Then it should be moved to boost.detail, directory and namespace.
Right. We should never knowingly allow (near) duplicate code in the repository.
Then I can only repeat the question I've asked in another email -- do you think it's OK to add a buildable library (which probably should be in regression tests as well), without review.
I never saw the question. But yes, just put it in detail. Though it's not clear to me why a buildable library is neccessary.
If you tell how it can be done
The same way as with all the other libraries (?) -- Dave Abrahams Boost Consulting http://www.boost-consulting.com
Rene Rivera wrote:
Seems wrong; standard is that it's only in std:: if you're not including any of the <xxx.h> headers. You ought to look for it there by default and only pluck it from the global namespace on implementations that you know to be nonconforming in this regard. Same with size_t.
Perhaps the intent was to place the entire thing inside some namespace and hence the mbstate_t would not collide?
The story with program_options is simple. Initially it used std::mbstate_t and had namespace std { using ::mbstate_t; } for broken compilers. Later, Pavel reported that this does not work with his version of borland. For unknown reason, changing std::mbstate_t to just mbstate_t fixes those failures. That's why program_options now has mbstate_t in global scope. I know this is not the best thing to do, but my version of borland does not show that error, so I can't experiment with different workarounds. - Volodya
Vladimir Prus wrote:
Rene Rivera wrote:
Perhaps the intent was to place the entire thing inside some namespace and hence the mbstate_t would not collide?
The story with program_options is simple. Initially it used std::mbstate_t and had
namespace std { using ::mbstate_t; }
for broken compilers. Later, Pavel reported that this does not work with his version of borland. For unknown reason, changing std::mbstate_t to just mbstate_t fixes those failures. That's why program_options now has mbstate_t in global scope.
Yes. But did anyone try other non intrusive approaches? Here are some possibilities: namespace std { typedef ::mbstate_t mbstate_t; } namespace boost { namespace detail { using ::mbstate_t; } } namespace boost { namespace detail { typedef ::mbstate_t mbstate_t; } } namespace boost { namespace detail { struct mbstate_t_workaround { typedef ::mbstate_t mbstate_t; }; } } etc... -- -- Grafik - Don't Assume Anything -- Redshift Software, Inc. - http://redshift-software.com -- rrivera/acm.org - grafik/redshift-software.com - 102708583/icq
Rene Rivera wrote:
for broken compilers. Later, Pavel reported that this does not work with his version of borland. For unknown reason, changing std::mbstate_t to just mbstate_t fixes those failures. That's why program_options now has mbstate_t in global scope.
Yes. But did anyone try other non intrusive approaches?
Nope. The only thing I know for sure is that with "std::mbstate_t" borland generates vague errors and with "mbstate_t" it does not. Since the error does not happen with my version, I really can't try all the possibilities. - Volodya
participants (6)
-
David Abrahams -
Jonathan Turkanis -
Pavel Vozenilek -
Rene Rivera -
Robert Ramey -
Vladimir Prus