[Serialization] BOOST_CLASS_EXPORT regression on SunCC
From version 1.34 to 1.35, the export of classes was modified so that all the archives did not need to be included before calls to BOOST_CLASS_EXPORT. I don't know why this was a problem to begin with.
Anyway, this feature used to work on SunCC but with the new changes, there is a regression. This is caused by the dependence on ADL to instantiate the ptr_serialization_support members: SunCC just doesn't do it. So the instantiations never occur and you get an assertion failure when running the serialization tests under debug: Assertion failed: it != boost::serialization::singleton< oserializer_map<Archive> >::get_const_instance().end(), file vendor/boost_1_37_0/boost/archive/impl/archive_pointer_oseri\ alizer.ipp, line 64 I know how to fix it (instantiating manually for each Archive,Serializable pair, not a huge problem). I would suggest that Boost Serialization should have a fallback mechanism for these compilers. Perhaps along the lines of the old export functionality where the user would define the archives they are interested in. Thoughts?
Look at the code - It's not just SunCC template <class Archive, class Serializable> struct ptr_serialization_support { # if defined(BOOST_MSVC) virtual BOOST_DLLEXPORT void instantiate() BOOST_USED; # elif defined(__BORLANDC__) static BOOST_DLLEXPORT void instantiate() BOOST_USED; enum { x = sizeof(instantiate(),3) }; # else static BOOST_DLLEXPORT void instantiate() BOOST_USED; typedef instantiate_function< &ptr_serialization_support::instantiate
x; # endif };
Can we not find some bit of magic to make SunCC instatiate that whic is needed? Robert Ramey Sohail Somani wrote:
From version 1.34 to 1.35, the export of classes was modified so that all the archives did not need to be included before calls to BOOST_CLASS_EXPORT. I don't know why this was a problem to begin with.
Anyway, this feature used to work on SunCC but with the new changes, there is a regression. This is caused by the dependence on ADL to instantiate the ptr_serialization_support members: SunCC just doesn't do it. So the instantiations never occur and you get an assertion failure when running the serialization tests under debug:
Assertion failed: it != boost::serialization::singleton< oserializer_map<Archive> >::get_const_instance().end(), file vendor/boost_1_37_0/boost/archive/impl/archive_pointer_oseri\ alizer.ipp, line 64
I know how to fix it (instantiating manually for each Archive,Serializable pair, not a huge problem).
I would suggest that Boost Serialization should have a fallback mechanism for these compilers. Perhaps along the lines of the old export functionality where the user would define the archives they are interested in.
Thoughts?
_______________________________________________ Unsubscribe & other changes: http://lists.boost.org/mailman/listinfo.cgi/boost
Hey Robert, I guess that is possible. I have no idea *why* it is not getting instantiated though so I wouldn't know where to start. Any suggestions? In any case, what part of the standard says that the return type should be instantiated? I read the part referenced in register_archive.hpp (temp.dep.candidate) but I can't see how to interpret that as "must instantiate the return type." Also, it turns out that base_object also has a regression which may be related. I had to explicitly instantiate void_cast_register to get the test to pass. Robert Ramey wrote:
Look at the code - It's not just SunCC
template <class Archive, class Serializable> struct ptr_serialization_support { # if defined(BOOST_MSVC) virtual BOOST_DLLEXPORT void instantiate() BOOST_USED; # elif defined(__BORLANDC__) static BOOST_DLLEXPORT void instantiate() BOOST_USED; enum { x = sizeof(instantiate(),3) }; # else static BOOST_DLLEXPORT void instantiate() BOOST_USED; typedef instantiate_function< &ptr_serialization_support::instantiate
x; # endif };
Can we not find some bit of magic to make SunCC instatiate that whic is needed?
Robert Ramey
Sohail Somani wrote:
From version 1.34 to 1.35, the export of classes was modified so that all the archives did not need to be included before calls to BOOST_CLASS_EXPORT. I don't know why this was a problem to begin with.
Anyway, this feature used to work on SunCC but with the new changes, there is a regression. This is caused by the dependence on ADL to instantiate the ptr_serialization_support members: SunCC just doesn't do it. So the instantiations never occur and you get an assertion failure when running the serialization tests under debug:
Assertion failed: it != boost::serialization::singleton< oserializer_map<Archive> >::get_const_instance().end(), file vendor/boost_1_37_0/boost/archive/impl/archive_pointer_oseri\ alizer.ipp, line 64
I know how to fix it (instantiating manually for each Archive,Serializable pair, not a huge problem).
I would suggest that Boost Serialization should have a fallback mechanism for these compilers. Perhaps along the lines of the old export functionality where the user would define the archives they are interested in.
Thoughts?
_______________________________________________ Unsubscribe & other changes: http://lists.boost.org/mailman/listinfo.cgi/boost
_______________________________________________ Unsubscribe & other changes: http://lists.boost.org/mailman/listinfo.cgi/boost
-- Sohail Somani http://uint32t.blogspot.com
on Tue Feb 17 2009, Sohail Somani <sohail-AT-taggedtype.net> wrote:
Hey Robert,
I guess that is possible. I have no idea *why* it is not getting instantiated though so I wouldn't know where to start. Any suggestions?
In any case, what part of the standard says that the return type should be instantiated? I read the part referenced in register_archive.hpp (temp.dep.candidate) but I can't see how to interpret that as "must instantiate the return type."
My implementation of BOOST_CLASS_EXPORT used implementation-specific hacks to get instantiation to happen on different compilers. It all hinges on what the compiler considers to be "using" a name. So you may have to find the right hack for Sun. Fortunately, the code was fairly well localized into a single header IIRC. -- Dave Abrahams BoostPro Computing http://www.boostpro.com
David Abrahams wrote:
on Tue Feb 17 2009, Sohail Somani <sohail-AT-taggedtype.net> wrote:
Hey Robert,
I guess that is possible. I have no idea *why* it is not getting instantiated though so I wouldn't know where to start. Any suggestions?
In any case, what part of the standard says that the return type should be instantiated? I read the part referenced in register_archive.hpp (temp.dep.candidate) but I can't see how to interpret that as "must instantiate the return type."
My implementation of BOOST_CLASS_EXPORT used implementation-specific hacks to get instantiation to happen on different compilers. It all hinges on what the compiler considers to be "using" a name. So you may have to find the right hack for Sun. Fortunately, the code was fairly well localized into a single header IIRC.
Ok, that is a start. Thanks. Still, why intentionally restrict portability even further when the alternative was not burdensome to begin with? I would have better luck implementing a portable export (the previous version) rather than figuring out hacks for each compiler. I think the export documentation should be modified to say (in big fat bold letters) that it should not be used in portable code. Actually, I remember mentioning this at my serialization talk. I might have to follow my own advice :-) -- Sohail Somani http://uint32t.blogspot.com
On Tue, Feb 17, 2009 at 7:27 AM, Sohail Somani <sohail@taggedtype.net> wrote:
David Abrahams wrote:
on Tue Feb 17 2009, Sohail Somani <sohail-AT-taggedtype.net> wrote:
Hey Robert,
I guess that is possible. I have no idea *why* it is not getting instantiated though so I wouldn't know where to start. Any suggestions?
In any case, what part of the standard says that the return type should be instantiated? I read the part referenced in register_archive.hpp (temp.dep.candidate) but I can't see how to interpret that as "must instantiate the return type."
My implementation of BOOST_CLASS_EXPORT used implementation-specific hacks to get instantiation to happen on different compilers. It all hinges on what the compiler considers to be "using" a name. So you may have to find the right hack for Sun. Fortunately, the code was fairly well localized into a single header IIRC.
Ok, that is a start. Thanks.
Still, why intentionally restrict portability even further when the alternative was not burdensome to begin with? I would have better luck implementing a portable export (the previous version) rather than figuring out hacks for each compiler.
I think the export documentation should be modified to say (in big fat bold letters) that it should not be used in portable code.
I think it's better to deprecate BOOST_CLASS_EXPORT altogether. Emil Dotchevski Reverge Studios, Inc. http://www.revergestudios.com/reblog/index.php?n=ReCode
Emil Dotchevski wrote:
On Tue, Feb 17, 2009 at 7:27 AM, Sohail Somani <sohail@taggedtype.net> wrote:
Ok, that is a start. Thanks.
Still, why intentionally restrict portability even further when the alternative was not burdensome to begin with? I would have better luck implementing a portable export (the previous version) rather than figuring out hacks for each compiler.
I think the export documentation should be modified to say (in big fat bold letters) that it should not be used in portable code.
I think it's better to deprecate BOOST_CLASS_EXPORT altogether.
If the new policy is to only support specific compilers for this functionality, I would agree with that. Header ordering is a problem you can work around and still use the library. Compiler-specific hacks, not really. -- Sohail Somani http://uint32t.blogspot.com
On Tue, Feb 17, 2009 at 9:43 AM, Sohail Somani <sohail@taggedtype.net> wrote:
Emil Dotchevski wrote:
On Tue, Feb 17, 2009 at 7:27 AM, Sohail Somani <sohail@taggedtype.net> wrote:
Ok, that is a start. Thanks.
Still, why intentionally restrict portability even further when the alternative was not burdensome to begin with? I would have better luck implementing a portable export (the previous version) rather than figuring out hacks for each compiler.
I think the export documentation should be modified to say (in big fat bold letters) that it should not be used in portable code.
I think it's better to deprecate BOOST_CLASS_EXPORT altogether.
If the new policy is to only support specific compilers for this functionality, I would agree with that.
Header ordering is a problem you can work around and still use the library. Compiler-specific hacks, not really.
I think there is a misunderstanding here. "Header ordering" doesn't mean that BOOST_CLASS_EXPORT would be portable. The compiler-specific hacks would be necessary no matter what. The BOOST_CLASS_EXPORT problem is that the C++ standard allows the compiler to strip away all of the instantiations it triggers, and good compilers do it. The "hacks" exploit holes in that functionality. Emil Dotchevski Reverge Studios, Inc. http://www.revergestudios.com/reblog/index.php?n=ReCode
Emil Dotchevski wrote:
On Tue, Feb 17, 2009 at 9:43 AM, Sohail Somani <sohail@taggedtype.net> wrote:
Emil Dotchevski wrote:
On Tue, Feb 17, 2009 at 7:27 AM, Sohail Somani <sohail@taggedtype.net> wrote:
Ok, that is a start. Thanks.
Still, why intentionally restrict portability even further when the alternative was not burdensome to begin with? I would have better luck implementing a portable export (the previous version) rather than figuring out hacks for each compiler.
I think the export documentation should be modified to say (in big fat bold letters) that it should not be used in portable code. I think it's better to deprecate BOOST_CLASS_EXPORT altogether. If the new policy is to only support specific compilers for this functionality, I would agree with that.
Header ordering is a problem you can work around and still use the library. Compiler-specific hacks, not really.
I think there is a misunderstanding here.
"Header ordering" doesn't mean that BOOST_CLASS_EXPORT would be portable. The compiler-specific hacks would be necessary no matter what. The BOOST_CLASS_EXPORT problem is that the C++ standard allows the compiler to strip away all of the instantiations it triggers, and good compilers do it. The "hacks" exploit holes in that functionality.
Hmm, yes you are right. I guess the bottom line is that export/base_object is so fragile it is not worth using any more. void_cast_register and register_type it is! Thanks for your comments, it has helped me a lot. -- Sohail Somani http://uint32t.blogspot.com
Sohail Somani wrote:
David Abrahams wrote:
Still, why intentionally restrict portability even further when the alternative was not burdensome to begin with?
The header ordering requirement of the previous system was considered a very large burden by a number of people. My view was that it wasn't that burdensome, but that if it could be eliminated, so much the better. Of course this meant some of the indicated hacking to implement some things not guarenteed by the C++ standard. But then, the serialization library has a lot of that. On the upside, the implementation has been much improved so that these hacks are now all focused in a few places. This is much, much better. Downside is that whenever one changes anything in this area, some compiler is going to be left out in the cold.
I would have better luck implementing a portable export (the previous version) rather than figuring out hacks for each compiler.
I don't think the previous was any more portable. It just seemed that way because it had been worked over longer. Test results look to indicate to me that only a few - maybe only one - compiler is having trouble with the current system. Hopefully, some interested party will find the missing magic. The current hacks in export.hpp should provide hints of what to do.
I think the export documentation should be modified to say (in big fat bold letters) that it should not be used in portable code.
I believe that the documentation does mention this. (Though not in big bold letters). I'm always gratified to receive suggestions for documentation enhancements through TRAK items. Note there is one important scenario for which EXPORT is by far the best solution. That is for plug-ins in DLLS. In this scenario, each DLL automagically registers the types it uses when it is loaded, and unregisters those same types when it is unloaded. This means that one can make a main program which will handle all future types even though they are not currently known. As far as I know, runtime linking and its implications for code compilation are not addressed by the C++ standard and this is why we have those compiler specific hacks in there. (Maybe C++ commitee gurus want to think about this?). So a good place to start to address this for your SunCC compiler is how is such a compiler used to create a DLL? What has to be done to tell the compiler to instantiate code not explicitly called and the optimizer not to strip it as dead code? This is basically how we came up wth the hacks that are in there now. Good Luck with this. Robert Ramey
Hi Robert, Robert Ramey wrote:
Sohail Somani wrote:
David Abrahams wrote:
Still, why intentionally restrict portability even further when the alternative was not burdensome to begin with?
The header ordering requirement of the previous system was considered a very large burden by a number of people. My view was that it wasn't that burdensome, but that if it could be eliminated, so much the better.
Is that a large burden by a few, vocal people? Anyway, it has been done and from the sounds of it, does not appear to be reversible. The main issue is that it eliminated a wart for some but broke someone else's arm :-) I'll file items for these issues so that they are tracked.
Of course this meant some of the indicated hacking to implement some things not guaranteed by the C++ standard. But then, the serialization library has a lot of that. On the upside, the implementation has been much improved so that these hacks are now all focused in a few places. This is much, much better. Downside is that whenever one changes anything in this area, some compiler is going to be left out in the cold.
Factoring of compiler hacks is helpful, so I am happy you decided to do that.
I would have better luck implementing a portable export (the previous version) rather than figuring out hacks for each compiler.
I don't think the previous was any more portable. It just seemed that way because it had been worked over longer.
I'm not so sure. It wasn't thread-safe but the behaviour was definitely well-defined according to the C++ standard.
Test results look to indicate to me that only a few - maybe only one - compiler is having trouble with the current system. Hopefully, some interested party will find the missing magic. The current hacks in export.hpp should provide hints of what to do.
It seems export.hpp is the least of the problems since that is somewhat well understood. base_object seems to not work anymore either: http://www.boost.org/development/tests/trunk/developer/output/Sandia-sun-boo...
I think the export documentation should be modified to say (in big fat bold letters) that it should not be used in portable code.
I believe that the documentation does mention this. (Though not in big bold letters). I'm always gratified to receive suggestions for documentation enhancements through TRAK items.
Yep, I see it now: http://www.boost.org/doc/libs/1_35_0/libs/serialization/doc/special.html#exp... I don't recall reading such a thing in the 1.34.1 documentation.
Note there is one important scenario for which EXPORT is by far the best solution. That is for plug-ins in DLLS. In this scenario, each DLL automagically registers the types it uses when it is loaded, and unregisters those same types when it is unloaded. This means that one can make a main program which will handle all future types even though they are not currently known. As far as I know, runtime linking and its implications for code compilation are not addressed by the C++ standard and this is why we have those compiler specific hacks in there.
(Maybe C++ commitee gurus want to think about this?).
Simple, mandated [[init]] and [[fini]] attributes would be awesome.
So a good place to start to address this for your SunCC compiler is how is such a compiler used to create a DLL? What has to be done to tell the compiler to instantiate code not explicitly called and the optimizer not to strip it as dead code? This is basically how we came up wth the hacks that are in there now.
It seems that there is support for GCC's __attribute__((constructor)) in the latest version so I might give that a shot. Maybe I'll just have to bite the bullet and stop using these features.
Good Luck with this.
Thanks! I'll let you know if anything comes of it.
on Tue Feb 17 2009, Sohail Somani <sohail-AT-taggedtype.net> wrote:
Hi Robert,
Robert Ramey wrote:
Sohail Somani wrote:
David Abrahams wrote:
Still, why intentionally restrict portability even further when the alternative was not burdensome to begin with?
The header ordering requirement of the previous system was considered a very large burden by a number of people. My view was that it wasn't that burdensome, but that if it could be eliminated, so much the better.
Is that a large burden by a few, vocal people?
No, I don't think so. Header ordering is deadly-hard to control. That's why we don't use qualified calls and overloading in the library namespace for customization points ;-)
Anyway, it has been done and from the sounds of it, does not appear to be reversible. The main issue is that it eliminated a wart for some but broke someone else's arm :-)
How's that? There's no longer a way to manually instantiate what you need? When did all this stuff start breaking for you? I made my changes to BOOST_CLASS_EXPORT years ago. IIRC it was still doing nonportable things before my changes, but was worse because of the header ordering stuff.
Of course this meant some of the indicated hacking to implement some things not guaranteed by the C++ standard. But then, the serialization library has a lot of that. On the upside, the implementation has been much improved so that these hacks are now all focused in a few places. This is much, much better. Downside is that whenever one changes anything in this area, some compiler is going to be left out in the cold.
Factoring of compiler hacks is helpful, so I am happy you decided to do that.
I thought I had done that.
Test results look to indicate to me that only a few - maybe only one - compiler is having trouble with the current system. Hopefully, some interested party will find the missing magic. The current hacks in export.hpp should provide hints of what to do.
It seems export.hpp is the least of the problems since that is somewhat well understood. base_object seems to not work anymore either:
http://www.boost.org/development/tests/trunk/developer/output/Sandia-sun-boo...
I think the export documentation should be modified to say (in big fat bold letters) that it should not be used in portable code.
I believe that the documentation does mention this. (Though not in big bold letters). I'm always gratified to receive suggestions for documentation enhancements through TRAK items.
Yep, I see it now:
http://www.boost.org/doc/libs/1_35_0/libs/serialization/doc/special.html#exp...
I don't recall reading such a thing in the 1.34.1 documentation.
Nope. The way I remember it: * I assumed Robert knew about his existing non-portability with BOOST_CLASS_EXPORT, so didn't feel the need to alert him * Robert didn't know about it * Robert found out some time after 1.34.1 came out and documented it. -- Dave Abrahams BoostPro Computing http://www.boostpro.com
David Abrahams wrote:
on Tue Feb 17 2009, Sohail Somani <sohail-AT-taggedtype.net> wrote:
Hi Robert,
Robert Ramey wrote:
David Abrahams wrote: Still, why intentionally restrict portability even further when the alternative was not burdensome to begin with? The header ordering requirement of the previous system was considered a very large burden by a number of people. My view was that it wasn't
Sohail Somani wrote: that burdensome, but that if it could be eliminated, so much the better. Is that a large burden by a few, vocal people?
No, I don't think so. Header ordering is deadly-hard to control. That's why we don't use qualified calls and overloading in the library namespace for customization points ;-)
In general, yes. But I found that the way Robert had documented it was sufficient for it not to be a problem.
Anyway, it has been done and from the sounds of it, does not appear to be reversible. The main issue is that it eliminated a wart for some but broke someone else's arm :-)
How's that? There's no longer a way to manually instantiate what you need?
Manually instantiating for hundreds of classes is not exactly easy. So it will eventually be healed, but may take a while :-)
When did all this stuff start breaking for you? I made my changes to BOOST_CLASS_EXPORT years ago. IIRC it was still doing nonportable things before my changes, but was worse because of the header ordering stuff.
I am currently attempting to upgrade from 1.34.1.
Of course this meant some of the indicated hacking to implement some things not guaranteed by the C++ standard. But then, the serialization library has a lot of that. On the upside, the implementation has been much improved so that these hacks are now all focused in a few places. This is much, much better. Downside is that whenever one changes anything in this area, some compiler is going to be left out in the cold. Factoring of compiler hacks is helpful, so I am happy you decided to do that.
I thought I had done that.
You are right! [snip]
I think the export documentation should be modified to say (in big fat bold letters) that it should not be used in portable code. I believe that the documentation does mention this. (Though not in big bold letters). I'm always gratified to receive suggestions for documentation enhancements through TRAK items. Yep, I see it now:
http://www.boost.org/doc/libs/1_35_0/libs/serialization/doc/special.html#exp...
I don't recall reading such a thing in the 1.34.1 documentation.
Nope. The way I remember it:
* I assumed Robert knew about his existing non-portability with BOOST_CLASS_EXPORT, so didn't feel the need to alert him
* Robert didn't know about it
* Robert found out some time after 1.34.1 came out and documented it.
Where? http://www.boost.org/doc/libs/1_34_1/libs/serialization/doc/special.html#exp... Anyway, what is done is done. -- Sohail Somani http://uint32t.blogspot.com
On Tue, Feb 17, 2009 at 9:27 AM, Robert Ramey <ramey@rrsd.com> wrote:
Note there is one important scenario for which EXPORT is by far the best solution. That is for plug-ins in DLLS. In this scenario, each DLL automagically registers the types it uses when it is loaded, and unregisters those same types when it is unloaded. This means that one can make a main program which will handle all future types even though they are not currently known.
Dynamic libraries have initialization and termination routines, there is no problem to do the registering/unregistering there. The main program still doesn't need to know about the derived classes being registered and this does not require BOOST_CLASS_EXPORT. The benefit of BOOST_CLASS_EXPORT is that it allows the dynamic library itself not to know about the derived classes it registers.
As far as I know, runtime linking and its implications for code compilation are not addressed by the C++ standard and this is why we have those compiler specific hacks in there.
That's only half of the story. As an automagic registration of types, BOOST_CLASS_EXPORT isn't guaranteed to work even if you don't use dynamic linking. The problem is that the C++ standard allows the compiler to deadstrip the namespace-scope objects whose constructors are supposed to register the types, unless code from that module is explicitly called (which is roughly equivalent to a requirement that the program "knows about" the types being registered.) Emil Dotchevski Reverge Studios, Inc. http://www.revergestudios.com/reblog/index.php?n=ReCode
on Tue Feb 17 2009, Sohail Somani <sohail-AT-taggedtype.net> wrote:
David Abrahams wrote:
on Tue Feb 17 2009, Sohail Somani <sohail-AT-taggedtype.net> wrote:
Hey Robert,
I guess that is possible. I have no idea *why* it is not getting instantiated though so I wouldn't know where to start. Any suggestions?
In any case, what part of the standard says that the return type should be instantiated? I read the part referenced in register_archive.hpp (temp.dep.candidate) but I can't see how to interpret that as "must instantiate the return type."
My implementation of BOOST_CLASS_EXPORT used implementation-specific hacks to get instantiation to happen on different compilers. It all hinges on what the compiler considers to be "using" a name. So you may have to find the right hack for Sun. Fortunately, the code was fairly well localized into a single header IIRC.
Ok, that is a start. Thanks.
Still, why intentionally restrict portability even further when the alternative was not burdensome to begin with? I would have better luck implementing a portable export (the previous version)
Previous to what?
rather than figuring out hacks for each compiler.
Previous to my implementation, there was a nasty #include ordering dependency. That's what I fixed. -- Dave Abrahams BoostPro Computing http://www.boostpro.com
David Abrahams wrote:
on Tue Feb 17 2009, Sohail Somani <sohail-AT-taggedtype.net> wrote:
David Abrahams wrote:
on Tue Feb 17 2009, Sohail Somani <sohail-AT-taggedtype.net> wrote:
Hey Robert,
I guess that is possible. I have no idea *why* it is not getting instantiated though so I wouldn't know where to start. Any suggestions?
In any case, what part of the standard says that the return type should be instantiated? I read the part referenced in register_archive.hpp (temp.dep.candidate) but I can't see how to interpret that as "must instantiate the return type." My implementation of BOOST_CLASS_EXPORT used implementation-specific hacks to get instantiation to happen on different compilers. It all hinges on what the compiler considers to be "using" a name. So you may have to find the right hack for Sun. Fortunately, the code was fairly well localized into a single header IIRC. Ok, that is a start. Thanks.
Still, why intentionally restrict portability even further when the alternative was not burdensome to begin with? I would have better luck implementing a portable export (the previous version)
Previous to what?
Previous to the current version. 1.34.1.
rather than figuring out hacks for each compiler.
Previous to my implementation, there was a nasty #include ordering dependency. That's what I fixed.
You introduced a gigantic regression. Is that not important? -- Sohail Somani http://uint32t.blogspot.com
on Tue Feb 17 2009, Sohail Somani <sohail-AT-taggedtype.net> wrote:
rather than figuring out hacks for each compiler.
Previous to my implementation, there was a nasty #include ordering dependency. That's what I fixed.
You introduced a gigantic regression. Is that not important?
It would be if it were true. But after my changes, all the regression tests passed for compilers we were supporting. So how's that possible? -- Dave Abrahams BoostPro Computing http://www.boostpro.com
David Abrahams wrote:
on Tue Feb 17 2009, Sohail Somani <sohail-AT-taggedtype.net> wrote:
rather than figuring out hacks for each compiler. Previous to my implementation, there was a nasty #include ordering dependency. That's what I fixed. You introduced a gigantic regression. Is that not important?
It would be if it were true. But after my changes, all the regression tests passed for compilers we were supporting. So how's that possible?
That is strange. I distinctly remember it not working for g++ 4 for a while. Do you mean to distinguish between compilers Boost is supporting and compilers Boost tests on? If so I'm not sure how helpful that is. In this case, tests that passed before, no longer pass. Reading your responses to the original thread, I understand that BOOST_CLASS_EXPORT and even base_object should not be used in portable code. I am glad that Robert (or whoever) made portable syntax available. I hope that this trend continues. Anyway, if I do manage to figure it out in a way that lets the current code remain unchanged, I will definitely let you know! -- Sohail Somani http://uint32t.blogspot.com
on Tue Feb 17 2009, Sohail Somani <sohail-AT-taggedtype.net> wrote:
David Abrahams wrote:
on Tue Feb 17 2009, Sohail Somani <sohail-AT-taggedtype.net> wrote:
rather than figuring out hacks for each compiler. Previous to my implementation, there was a nasty #include ordering dependency. That's what I fixed. You introduced a gigantic regression. Is that not important?
It would be if it were true. But after my changes, all the regression tests passed for compilers we were supporting. So how's that possible?
That is strange. I distinctly remember it not working for g++ 4 for a while.
I remember something like that too. However, IIRC, that problem was a simple case of stupid incorrect code on my part, that somehow happened to work on earlier compilers. That had nothing to do with relying on compiler implementation details. Yep: https://svn.boost.org/trac/boost/ticket/1711
Do you mean to distinguish between compilers Boost is supporting and compilers Boost tests on?
No I do not. It was passing on all the compilers we tested on and/or I could get my hands on.
If so I'm not sure how helpful that is. In this case, tests that passed before, no longer pass.
Please know exactly what you're saying before you make that claim for a third time. I was *exceedingly* careful in testing these changes. Before we can know that I introduced a regression, we need to consider the original state of the code I checked in (since changed), the exact compilers that were available, and those we were testing on. So, as far as I can tell, GCC-4.1.0 came out a few months before my checkin, and I don't even know that the problem was occurring until a later release of 4.1.x, since the bug report isn't specific. https://svn.boost.org/trac/boost/changeset/34106 http://www.gnu.org/software/gcc/releases.html And again, the issue there has nothing to do with relying on implementation-specific hacks, and it wasn't reported until two years later (by you, incidentally). -- Dave Abrahams BoostPro Computing http://www.boostpro.com
David Abrahams wrote:
on Tue Feb 17 2009, Sohail Somani <sohail-AT-taggedtype.net> wrote:
That is strange. I distinctly remember it not working for g++ 4 for a while.
I remember something like that too. However, IIRC, that problem was a simple case of stupid incorrect code on my part, that somehow happened to work on earlier compilers. That had nothing to do with relying on compiler implementation details.
This comes back to a question I had earlier which was: where in the standard does it specify that looking up functions during ADL requires instantiation of return types? If I could interpret a section in that manner, then I can report a bug to Sun. I have verified that the type is never instantiated.
Do you mean to distinguish between compilers Boost is supporting and compilers Boost tests on?
No I do not. It was passing on all the compilers we tested on and/or I could get my hands on.
OK.
If so I'm not sure how helpful that is. In this case, tests that passed before, no longer pass.
Please know exactly what you're saying before you make that claim for a third time. I was *exceedingly* careful in testing these changes. Before we can know that I introduced a regression, we need to consider the original state of the code I checked in (since changed), the exact compilers that were available, and those we were testing on.
I did not mean to imply that you were not careful but wanted to be sure about why you were explicit about supported compilers. That's why I said "if so" :-) Is there an archive of test results somewhere? That would make it easier to track down when it stopped working.
And again, the issue there has nothing to do with relying on implementation-specific hacks, and it wasn't reported until two years later (by you, incidentally).
I don't think that the change was done 3 years ago means anything. Just that Boost used to take a long time to release and people are slow to upgrade. I thought it was kind of funny that I reported the same issue in two different compilers. Unfortunately, no workaround or fix yet! -- Sohail Somani http://uint32t.blogspot.com
on Tue Feb 17 2009, Sohail Somani <sohail-AT-taggedtype.net> wrote:
David Abrahams wrote:
on Tue Feb 17 2009, Sohail Somani <sohail-AT-taggedtype.net> wrote:
That is strange. I distinctly remember it not working for g++ 4 for a while.
I remember something like that too. However, IIRC, that problem was a simple case of stupid incorrect code on my part, that somehow happened to work on earlier compilers. That had nothing to do with relying on compiler implementation details.
This comes back to a question I had earlier which was: where in the standard does it specify that looking up functions during ADL requires instantiation of return types?
Well, if the type is illegal in a way that doesn't qualify for SFINAE, a diagnostic is required. That means the compiler has to figure out what the type is. However, I think the real question is whether the code is required to cause the overload resolution at all. Strictly speaking, the code can be considered to be "unused" and so the compiler is allowed to not instantiate it.
Is there an archive of test results somewhere?
Somewhere.
That would make it easier to track down when it stopped working.
And again, the issue there has nothing to do with relying on implementation-specific hacks, and it wasn't reported until two years later (by you, incidentally).
I don't think that the change was done 3 years ago means anything. Just that Boost used to take a long time to release and people are slow to upgrade.
I thought it was kind of funny that I reported the same issue in two different compilers. Unfortunately, no workaround or fix yet!
Huh? 1711 was fixed and closed. -- Dave Abrahams BoostPro Computing http://www.boostpro.com
David Abrahams wrote:
on Tue Feb 17 2009, Sohail Somani <sohail-AT-taggedtype.net> wrote:
This comes back to a question I had earlier which was: where in the standard does it specify that looking up functions during ADL requires instantiation of return types?
Well, if the type is illegal in a way that doesn't qualify for SFINAE, a diagnostic is required. That means the compiler has to figure out what the type is. However, I think the real question is whether the code is required to cause the overload resolution at all. Strictly speaking, the code can be considered to be "unused" and so the compiler is allowed to not instantiate it.
This is most likely the cause.
I thought it was kind of funny that I reported the same issue in two different compilers. Unfortunately, no workaround or fix yet!
Huh? 1711 was fixed and closed.
Sorry, I was not clear. I meant no workaround or fix for the second compiler. -- Sohail Somani http://uint32t.blogspot.com
on Tue Feb 17 2009, Sohail Somani <sohail-AT-taggedtype.net> wrote:
David Abrahams wrote:
on Tue Feb 17 2009, Sohail Somani <sohail-AT-taggedtype.net> wrote:
This comes back to a question I had earlier which was: where in the standard does it specify that looking up functions during ADL requires instantiation of return types?
Well, if the type is illegal in a way that doesn't qualify for SFINAE, a diagnostic is required. That means the compiler has to figure out what the type is. However, I think the real question is whether the code is required to cause the overload resolution at all. Strictly speaking, the code can be considered to be "unused" and so the compiler is allowed to not instantiate it.
This is most likely the cause.
I thought it was kind of funny that I reported the same issue in two different compilers. Unfortunately, no workaround or fix yet!
Huh? 1711 was fixed and closed.
Sorry, I was not clear. I meant no workaround or fix for the second compiler.
What 2nd compiler? I thought you meant two versions of GCC. If you mean Sun, then AFAIK that's a recent report, and **it's not the same issue**. -- Dave Abrahams BoostPro Computing http://www.boostpro.com
David Abrahams wrote:
on Tue Feb 17 2009, Sohail Somani <sohail-AT-taggedtype.net> wrote:
I thought it was kind of funny that I reported the same issue in two different compilers. Unfortunately, no workaround or fix yet! Huh? 1711 was fixed and closed. Sorry, I was not clear. I meant no workaround or fix for the second compiler.
What 2nd compiler? I thought you meant two versions of GCC.
If you mean Sun, then AFAIK that's a recent report, and **it's not the same issue**.
They may not have the same cause, for sure, but the issue is still that BOOST_CLASS_EXPORT doesn't work any more. -- Sohail Somani http://uint32t.blogspot.com
Just to let everyone know: Between 1.34 and the present export has undergone a lot more that one change. The one's I recall are ( more or less in order) ehancement to eliminate header order requirement enhancement to support multi-threading. This required implemenation of a suitable singleton and changing a lot of code to use it. enhancement to support serialization code dispurseed accross dynamically DLLS. enhancement to remove dependence on __LINE__ to maintain some unique names. clarification in the documentation as to the best way to use export. And after all there this there is ONLY ONE regression? (maybe two - recently detected). So I'm personally very pleased to take credit for all this. Note that its not all that easy to detect such regressions from looking at the trunk tests. The table shows as errors such things as usage with libraries which don't support wide characters or don't support spirit. Also some test platforms arn't setup correctly. And often there is a lapse of time between the change is made and when the error shows up. And also results hang around a long time so there is no easy way to see that one has actually fixed something. So as far as SunCC is concerned: I've reviewed the latest trunk tests and it looks to me that a little effort could resolve the problems with instantiation of extended_type_info for these types. In the worst case, probably a SunCC specific macro could be included. In any case, it'll have to be addressed by someone who has the compiler on hand. I'd would much like to see this looked at. The only other compiler which shows problems is the VACPP and I have it on qt that that maybe on the road to getting addressed as well. Robert Ramey
AMDG Robert Ramey wrote:
So as far as SunCC is concerned: I've reviewed the latest trunk tests and it looks to me that a little effort could resolve the problems with instantiation of extended_type_info for these types. In the worst case, probably a SunCC specific macro could be included. In any case, it'll have to be addressed by someone who has the compiler on hand. I'd would much like to see this looked at.
I'll take a look when I have time, if no one else gets to it first. In Christ, Steven Watanabe
Robert Ramey wrote:
Just to let everyone know:
Between 1.34 and the present export has undergone a lot more that one change. The one's I recall are ( more or less in order)
ehancement to eliminate header order requirement
enhancement to support multi-threading. This required implemenation of a suitable singleton and changing a lot of code to use it.
enhancement to support serialization code dispurseed accross dynamically DLLS.
enhancement to remove dependence on __LINE__ to maintain some unique names.
Can you please describe this further? I thought __LINE__ is the best way to maintain unique names.
clarification in the documentation as to the best way to use export.
And after all there this there is ONLY ONE regression? (maybe two - recently detected).
I hope that I have not come across as saying that I disagree with you there. I am also very pleased that this is the only regression!
So I'm personally very pleased to take credit for all this.
Oh I dunno, Dave had something to do with it ;-)
Note that its not all that easy to detect such regressions from looking at the trunk tests. The table shows as errors such things as usage with libraries which don't support wide characters or don't support spirit. Also some test platforms arn't setup correctly. And often there is a lapse of time between the change is made and when the error shows up. And also results hang around a long time so there is no easy way to see that one has actually fixed something.
Thanks for explaining. The Boost test harness is pretty awesome but part of that awesomeness results in information overload.
So as far as SunCC is concerned: I've reviewed the latest trunk tests and it looks to me that a little effort could resolve the problems with instantiation of extended_type_info for these types. In the worst case, probably a SunCC specific macro could be included. In any case, it'll have to be addressed by someone who has the compiler on hand. I'd would much like to see this looked at. The only other compiler which shows problems is the VACPP and I have it on qt that that maybe on the road to getting addressed as well.
I will be looking at it but if Steven is looking at it, I will lose the race. One thing I was experimenting with was a macro with the explicit purpose of calling some function before main. As you have mentioned, this is not portable but that is the point. So for SunCC it looks something like: #define BOOST_SUNCC_PRAGMA(x) _Pragma(#x) #define BOOST_INIT(function) BOOST_SUNCC_PRAGMA(init(function)) For GCC/Visual C++, the macro expands into a singleton as is currently done with a call to function() in the constructor. Then if you wanted to execute some type-specific code at initialization time, you could modify BOOST_CLASS_EXPORT to do so. So I am thinking something like the following (pretend that names are unique): #define BOOST_CLASS_EXPORT_GUID(T,K) \ namespace \ { \ void register_function() \ { \ singleton<type_info_implementation<T>::type> \ ::get_mutable_instance().key_register(K); \ } \ } \ BOOST_INIT(register_function) BOOST_INIT(x) guarantees that x() is called before main. I believe x must be a vanilla function (not function template) to work portably. The benefit is that there are equivalent attributes for UNIX platforms that *specifically* implement this init behaviour so it is more likely to be portable. I guess this doesn't really solve the problem with SunCC not instantiating ptr_serialization_support::instantiate but atleast has a specific place for porting purposes. Thanks for your time. -- Sohail Somani http://uint32t.blogspot.com
on Tue Feb 17 2009, Sohail Somani <sohail-AT-taggedtype.net> wrote:
So I'm personally very pleased to take credit for all this.
Oh I dunno, Dave had something to do with it ;-)
As long as you still maintain that my work "introduced a gigantic regression," I'm very pleased to have Robert take credit. -- Dave Abrahams BoostPro Computing http://www.boostpro.com
AMDG Robert Ramey wrote:
So as far as SunCC is concerned: I've reviewed the latest trunk tests and it looks to me that a little effort could resolve the problems with instantiation of extended_type_info for these types. In the worst case, probably a SunCC specific macro could be included. In any case, it'll have to be addressed by someone who has the compiler on hand. I'd would much like to see this looked at. The only other compiler which shows problems is the VACPP and I have it on qt that that maybe on the road to getting addressed as well.
Patch against the trunk attached. I had to use a fairly horrible hack based on the compile time counter used in the typeof library. All the tests except the shared_ptr_1_32 tests pass for me with sun 5.9. In Christ, Steven Watanabe
Steven Watanabe wrote:
AMDG
Robert Ramey wrote:
So as far as SunCC is concerned: I've reviewed the latest trunk tests and it looks to me that a little effort could resolve the problems with instantiation of extended_type_info for these types. In the worst case, probably a SunCC specific macro could be included. In any case, it'll have to be addressed by someone who has the compiler on hand. I'd would much like to see this looked at. The only other compiler which shows problems is the VACPP and I have it on qt that that maybe on the road to getting addressed as well.
Patch against the trunk attached. I had to use a fairly horrible hack based on the compile time counter used in the typeof library.
All the tests except the shared_ptr_1_32 tests pass for me with sun 5.9.
Hi Steven, Thanks for taking the time to make this patch. Against 1.37.0, it does not fix the problem. I will check out trunk now to see if they pass for me on trunk as well. Can you please tell me the output of CC -V? Mine is: CC: Sun C++ 5.9 SunOS_i386 Patch 124864-04 2008/04/16 Which is not the latest, afaik. -- Sohail Somani http://uint32t.blogspot.com
AMDG Sohail Somani wrote:
Patch against the trunk attached. I had to use a fairly horrible hack based on the compile time counter used in the typeof library.
All the tests except the shared_ptr_1_32 tests pass for me with sun 5.9.
Hi Steven,
Thanks for taking the time to make this patch. Against 1.37.0, it does not fix the problem. I will check out trunk now to see if they pass for me on trunk as well.
Whoops. I missed the other file I edited.
Can you please tell me the output of CC -V? Mine is:
CC: Sun C++ 5.9 SunOS_i386 Patch 124864-04 2008/04/16
Which is not the latest, afaik
I have an older version: CC: Sun C++ 5.9 SunOS_i386 2007/11/15 In Christ, Steven Watanabe
As I understand the standard, there is on reason that the indicated code has to be instantiated. That's why we "snooker" the compiler into instantiating the code and "re-snooker" the optimizer so that it doesn't strip the code even though it's never explicitly invoked. (for this we use the DLL macros). So its just a case of SunCC compiler being too literal minded for boost export. I would hope that there is a way to indicate to the SunCC that the indicated code should be instantiated. Robert Ramey Sohail Somani wrote:
Hey Robert,
I guess that is possible. I have no idea *why* it is not getting instantiated though so I wouldn't know where to start. Any suggestions?
In any case, what part of the standard says that the return type should be instantiated? I read the part referenced in register_archive.hpp (temp.dep.candidate) but I can't see how to interpret that as "must instantiate the return type."
Also, it turns out that base_object also has a regression which may be related. I had to explicitly instantiate void_cast_register to get the test to pass.
Robert Ramey wrote:
Look at the code - It's not just SunCC
template <class Archive, class Serializable> struct ptr_serialization_support { # if defined(BOOST_MSVC) virtual BOOST_DLLEXPORT void instantiate() BOOST_USED; # elif defined(__BORLANDC__) static BOOST_DLLEXPORT void instantiate() BOOST_USED; enum { x = sizeof(instantiate(),3) }; # else static BOOST_DLLEXPORT void instantiate() BOOST_USED; typedef instantiate_function< &ptr_serialization_support::instantiate
x; # endif };
Can we not find some bit of magic to make SunCC instatiate that whic is needed?
Robert Ramey
Sohail Somani wrote:
From version 1.34 to 1.35, the export of classes was modified so that all the archives did not need to be included before calls to BOOST_CLASS_EXPORT. I don't know why this was a problem to begin with.
Anyway, this feature used to work on SunCC but with the new changes, there is a regression. This is caused by the dependence on ADL to instantiate the ptr_serialization_support members: SunCC just doesn't do it. So the instantiations never occur and you get an assertion failure when running the serialization tests under debug:
Assertion failed: it != boost::serialization::singleton< oserializer_map<Archive> >::get_const_instance().end(), file vendor/boost_1_37_0/boost/archive/impl/archive_pointer_oseri\ alizer.ipp, line 64
I know how to fix it (instantiating manually for each Archive,Serializable pair, not a huge problem).
I would suggest that Boost Serialization should have a fallback mechanism for these compilers. Perhaps along the lines of the old export functionality where the user would define the archives they are interested in.
Thoughts?
_______________________________________________ Unsubscribe & other changes: http://lists.boost.org/mailman/listinfo.cgi/boost
_______________________________________________ Unsubscribe & other changes: http://lists.boost.org/mailman/listinfo.cgi/boost
I figured as much. I've been looking for some equivalent way of snookering but no such luck. Hopefully Dave's response will get me on the right track. If this doesn't work, would you be open to a patch which falls back to a mechanism like the old export.hpp? I'll have to figure out the void_cast_register problem first though. Robert Ramey wrote:
As I understand the standard, there is on reason that the indicated code has to be instantiated. That's why we "snooker" the compiler into instantiating the code and "re-snooker" the optimizer so that it doesn't strip the code even though it's never explicitly invoked. (for this we use the DLL macros). So its just a case of SunCC compiler being too literal minded for boost export. I would hope that there is a way to indicate to the SunCC that the indicated code should be instantiated.
Robert Ramey
Sohail Somani wrote:
Hey Robert,
I guess that is possible. I have no idea *why* it is not getting instantiated though so I wouldn't know where to start. Any suggestions?
In any case, what part of the standard says that the return type should be instantiated? I read the part referenced in register_archive.hpp (temp.dep.candidate) but I can't see how to interpret that as "must instantiate the return type."
Also, it turns out that base_object also has a regression which may be related. I had to explicitly instantiate void_cast_register to get the test to pass.
Robert Ramey wrote:
Look at the code - It's not just SunCC
template <class Archive, class Serializable> struct ptr_serialization_support { # if defined(BOOST_MSVC) virtual BOOST_DLLEXPORT void instantiate() BOOST_USED; # elif defined(__BORLANDC__) static BOOST_DLLEXPORT void instantiate() BOOST_USED; enum { x = sizeof(instantiate(),3) }; # else static BOOST_DLLEXPORT void instantiate() BOOST_USED; typedef instantiate_function< &ptr_serialization_support::instantiate
x; # endif };
Can we not find some bit of magic to make SunCC instatiate that whic is needed?
Robert Ramey
Sohail Somani wrote:
From version 1.34 to 1.35, the export of classes was modified so that all the archives did not need to be included before calls to BOOST_CLASS_EXPORT. I don't know why this was a problem to begin with.
Anyway, this feature used to work on SunCC but with the new changes, there is a regression. This is caused by the dependence on ADL to instantiate the ptr_serialization_support members: SunCC just doesn't do it. So the instantiations never occur and you get an assertion failure when running the serialization tests under debug:
Assertion failed: it != boost::serialization::singleton< oserializer_map<Archive> >::get_const_instance().end(), file vendor/boost_1_37_0/boost/archive/impl/archive_pointer_oseri\ alizer.ipp, line 64
I know how to fix it (instantiating manually for each Archive,Serializable pair, not a huge problem).
I would suggest that Boost Serialization should have a fallback mechanism for these compilers. Perhaps along the lines of the old export functionality where the user would define the archives they are interested in.
Thoughts?
_______________________________________________ Unsubscribe & other changes: http://lists.boost.org/mailman/listinfo.cgi/boost
_______________________________________________ Unsubscribe & other changes: http://lists.boost.org/mailman/listinfo.cgi/boost
_______________________________________________ Unsubscribe & other changes: http://lists.boost.org/mailman/listinfo.cgi/boost
-- Sohail Somani http://uint32t.blogspot.com
participants (5)
-
David Abrahams -
Emil Dotchevski -
Robert Ramey -
Sohail Somani -
Steven Watanabe