[serialization] ADL problem
With unmodified CVS state of serialization, I get errors from compiler because archive/detail/iserializer.hpp contains unqualified call to 'load', and it find both serialization's 'load', and some other 'load' that I happen to have. I wonder if ADL is desired here? IIUC, the only customization point is the 'serialize' function -- that will call user provided 'load' and 'save' explictly. So, there's no need for ADL in iserializer.hpp. With the attached patch, things start to work for me. Is it possible to apply it? - Volodya
I made special efforts to use ADL for the following reasons a) serialization could in any of a couple of namespaces b) I believe that not using it resulted in problems with compilers that implement two-phase lookup c) a number of people asked for it Robert Ramey Vladimir Prus wrote:
With unmodified CVS state of serialization, I get errors from compiler because archive/detail/iserializer.hpp contains unqualified call to 'load', and it find both serialization's 'load', and some other 'load' that I happen to have.
I wonder if ADL is desired here? IIUC, the only customization point is the 'serialize' function -- that will call user provided 'load' and 'save' explictly. So, there's no need for ADL in iserializer.hpp.
With the attached patch, things start to work for me. Is it possible to apply it?
- Volodya
_______________________________________________ Unsubscribe & other changes: http://lists.boost.org/mailman/listinfo.cgi/boost
"Robert Ramey" <ramey@rrsd.com> writes:
I made special efforts to use ADL for the following reasons
a) serialization could in any of a couple of namespaces b) I believe that not using it resulted in problems with compilers that implement two-phase lookup c) a number of people asked for it
Use of ADL should be limited to places you intend the library's behavior to be customized ("customization points") in part to avoid the problem cited below: picking up names used in the library but never intended to interact with it. Volodya's question is whether load was really intended to be a customization point.
Vladimir Prus wrote:
With unmodified CVS state of serialization, I get errors from compiler because archive/detail/iserializer.hpp contains unqualified call to 'load', and it find both serialization's 'load', and some other 'load' that I happen to have.
I wonder if ADL is desired here? IIUC, the only customization point is the 'serialize' function -- that will call user provided 'load' and 'save' explictly. So, there's no need for ADL in iserializer.hpp.
With the attached patch, things start to work for me. Is it possible to apply it?
- Volodya
_______________________________________________ Unsubscribe & other changes: http://lists.boost.org/mailman/listinfo.cgi/boost
_______________________________________________ Unsubscribe & other changes: http://lists.boost.org/mailman/listinfo.cgi/boost
-- Dave Abrahams Boost Consulting www.boost-consulting.com
David Abrahams wrote:
Use of ADL should be limited to places you intend the library's behavior to be customized ("customization points") in part to avoid the problem cited below: picking up names used in the library but never intended to interact with it. Volodya's question is whether load was really intended to be a customization point.
The main customization point is to specialize the template template<class Archive, class T> void seriailize(Archive &ar, T &t, const unsigned int version) There is common specialization summarized by BOOST_SERIALIZATION_SPLIT_FREE(T) which implemetns void serialize(... // if its an output archive save(... // compile time else load(... So save and load can be customization points as well. I think this is pretty well explained in the manual. Robert Ramey
Robert Ramey wrote:
David Abrahams wrote:
Use of ADL should be limited to places you intend the library's behavior to be customized ("customization points") in part to avoid the problem cited below: picking up names used in the library but never intended to interact with it. Volodya's question is whether load was really intended to be a customization point.
The main customization point is to specialize the template
template<class Archive, class T> void seriailize(Archive &ar, T &t, const unsigned int version)
There is common specialization summarized by BOOST_SERIALIZATION_SPLIT_FREE(T) which implemetns void serialize(... // if its an output archive save(... // compile time else load(...
So save and load can be customization points as well. I think this is pretty well explained in the manual.
The question is different: Does serialization library (exception for *SPLIT* macros), ever calls 'load' function that is provided by the user. Your explanation above only reiterates what I've said: the serialization library only calls the "serialize" function. That function might delegate its work to the 'load' function, but it happens inside user code, not inside serialization code. That's why I think you should prevent ADL when calling 'load' in the mentioned place. - Volodya
Vladimir Prus wrote:
Does serialization library (exception for *SPLIT* macros), ever calls 'load' function that is provided by the user.
Your explanation above only reiterates what I've said: the serialization library only calls the "serialize" function. That function might delegate its work to the 'load' function, but it happens inside user code, not inside serialization code. That's why I think you should prevent ADL when calling 'load' in the mentioned place.
To be honest, I would have to go back and review a little to give you a definitive answer. But the only member function called by the library is "serialize". I think the issue comes up regarding the usage of the free function template "serialize". If one uses "split_free" then there one needs load and save functions and the question arises as to which namespaces they should be in. I prefered putting them boost::serialization:: . If I remember correctly, I then had a problem with compilers that use two-phase lookup and I hade to permit ADL so that the templates would be checked again at instantiation time. I'm speaking from memory so this explanatino might not be exactly accurate. If you look at the the included serializations in version 1.32 vs 1.33 and the old manual vs the new manual there were some baroque rules as to which namespaces spercializations could be put into depending upon whether or not the compiler being used implemented two-phase lookup. The latest version was able to do away with all that. So that's my explanation. Robert Ramey
"Robert Ramey" <ramey@rrsd.com> writes:
Vladimir Prus wrote:
Does serialization library (exception for *SPLIT* macros), ever calls 'load' function that is provided by the user.
Your explanation above only reiterates what I've said: the serialization library only calls the "serialize" function. That function might delegate its work to the 'load' function, but it happens inside user code, not inside serialization code. That's why I think you should prevent ADL when calling 'load' in the mentioned place.
To be honest, I would have to go back and review a little to give you a definitive answer. But the only member function called by the library is "serialize". I think the issue comes up regarding the usage of the free function template "serialize". If one uses "split_free" then there one needs load and save functions and the question arises as to which namespaces they should be in. I prefered putting them boost::serialization:: . If I remember correctly, I then had a problem with compilers that use two-phase lookup and I hade to permit ADL so that the templates would be checked again at instantiation time.
That's not the best way to fix the problem. Just make sure the functions are declared before use and they will be found with explicit qualification. -- Dave Abrahams Boost Consulting www.boost-consulting.com
David Abrahams wrote:
"Robert Ramey" <ramey@rrsd.com> writes:
That's not the best way to fix the problem. Just make sure the functions are declared before use and they will be found with explicit qualification.
That was my first choice - but it conflicted with export.hpp which needed to know what types and archives to instanciate serialize for. I found no other way to reconcile it. I'm sure this is not the only case where two phase lookup causes problems. Its going to cause lots of problems in working programs if it spreads to other compilers. Two phase lookup is a bad idea. Robert Ramey
"Robert Ramey" <ramey@rrsd.com> writes:
David Abrahams wrote:
"Robert Ramey" <ramey@rrsd.com> writes:
That's not the best way to fix the problem. Just make sure the functions are declared before use and they will be found with explicit qualification.
That was my first choice - but it conflicted with export.hpp which needed to know what types and archives to instanciate serialize for. I found no other way to reconcile it.
I'm sure this is not the only case where two phase lookup causes problems. Its going to cause lots of problems in working programs if it spreads to other compilers. Two phase lookup is a bad idea.
This is extremely troubling. Whether you like it or not, two phase lookup is part of the language specification, and it's going to "spread" to other compilers as they get more conformant. Boost libraries should be designed to work in a way that is consistent with the language as it is defined, rather than being patched up with the wrong band-aid when compilers fail to conform to the author's ideal of non-conformance. Before you object, I don't think my assessment is unduly harsh: the consequences of using an unqualified call as you have done have already bitten one developer and the library change is, IIUC, not even out the door to the public. Indiscriminate ADL is well-known to cause this sort of problem, and Boost developers are probably more careful than most to insulate themselves from it by isolating their types from functions. It's going to be a much bigger deal once Joe Global Namespace gets his hands on it. I don't know much about the needs of export.hpp, but there are well-known ways to navigate this terrain and I can't believe that the serialization library is unique in this area. Maybe if you ask for help and give some detail about what you're trying to do, someone can suggest a better approach. For example, did you consider using class template partial specialization? -- Dave Abrahams Boost Consulting www.boost-consulting.com
I did spend a significant amount of time trying dealing with this. The first effort resulted in the table of rules which can be found in the documentation of 1.32. I found this very unsatisfactory. If I remember correctly it was one set of requirements for compilers supporting partial template specialization and another set for compilers that didn't. Given the two alternatives, I found the current one much better. I understand that I have to live with two-phase lookup. But there's no requirement that I have to think its a good idea. I first noticed the problem when the meaning of my code silently changed depending upon header order. Of course this was compiler dependent. So it was huge hassle to find. Sorry, I just can't see how this can be a good idea. Robert Ramey
Before you object, I don't think my assessment is unduly harsh: the consequences of using an unqualified call as you have done have already bitten one developer and the library change is, IIUC, not even out the door to the public. Indiscriminate ADL is well-known to cause this sort of problem, and Boost developers are probably more careful than most to insulate themselves from it by isolating their types from functions. It's going to be a much bigger deal once Joe Global Namespace gets his hands on it.
I don't know much about the needs of export.hpp, but there are well-known ways to navigate this terrain and I can't believe that the serialization library is unique in this area. Maybe if you ask for help and give some detail about what you're trying to do, someone can suggest a better approach. For example, did you consider using class template partial specialization?
Robert Ramey wrote:
I did spend a significant amount of time trying dealing with this. The first effort resulted in the table of rules which can be found in the documentation of 1.32. I found this very unsatisfactory. If I remember correctly it was one set of requirements for compilers supporting partial template specialization and another set for compilers that didn't. Given the two alternatives, I found the current one much better.
I understand that I have to live with two-phase lookup. But there's no requirement that I have to think its a good idea. I first noticed the problem when the meaning of my code silently changed depending upon header order. Of course this was compiler dependent. So it was huge hassle to find. Sorry, I just can't see how this can be a good idea.
Did you try to apply my patch? Did it break any tests? On what compilers? You statements sound a bit vague; probably if you clearly explain what's the problem, somebody will be able to give a solution. The current library simply does not work with my code. Do you suggest that I patch Boost tree locally? Or there's some other way? - Volodya
"Robert Ramey" <ramey@rrsd.com> writes:
I did spend a significant amount of time trying dealing with this.
Did you ask for help? We have a lot of experience in this general area; I think there are better solutions.
The first effort resulted in the table of rules which can be found in the documentation of 1.32. I found this very unsatisfactory. If I remember correctly it was one set of requirements for compilers supporting partial template specialization and another set for compilers that didn't. Given the two alternatives, I found the current one much better.
I understand that I have to live with two-phase lookup. But there's no requirement that I have to think its a good idea. I first noticed the problem when the meaning of my code silently changed depending upon header order. Of course this was compiler dependent.
That's the broken compilers' fault, though, not the conformant ones'.
So it was huge hassle to find. Sorry, I just can't see how this can be a good idea.
It can be a good idea if you like to find as many errors in your code as early as possible, and it can be an especially good idea if you want to call functions in a consistent overload context, especially with exported templates. Without 2-phase lookup, templates are not usually even parsed until instantiation time. I understand why some people don't like it, but there are at least as many arguments in its favor.
Before you object, I don't think my assessment is unduly harsh: the consequences of using an unqualified call as you have done have already bitten one developer and the library change is, IIUC, not even out the door to the public. Indiscriminate ADL is well-known to cause this sort of problem, and Boost developers are probably more careful than most to insulate themselves from it by isolating their types from functions. It's going to be a much bigger deal once Joe Global Namespace gets his hands on it.
I don't know much about the needs of export.hpp, but there are well-known ways to navigate this terrain and I can't believe that the serialization library is unique in this area. Maybe if you ask for help and give some detail about what you're trying to do, someone can suggest a better approach. For example, did you consider using class template partial specialization?
So, did you? -- Dave Abrahams Boost Consulting www.boost-consulting.com
David Abrahams wrote:
So it was huge hassle to find. Sorry, I just can't see how this can be a good idea.
It can be a good idea if you like to find as many errors in your code as early as possible, and it can be an especially good idea if you want to call functions in a consistent overload context, especially with exported templates.
Can you explain why "especially with exported templates". I heard this statement several times but never understood why "export" changes anything? Isn't instantiation point and so context the same regardless of whether template is exported or not? - Volodya
Vladimir Prus <ghost@cs.msu.su> writes:
David Abrahams wrote:
So it was huge hassle to find. Sorry, I just can't see how this can be a good idea.
It can be a good idea if you like to find as many errors in your code as early as possible, and it can be an especially good idea if you want to call functions in a consistent overload context, especially with exported templates.
Can you explain why "especially with exported templates". I heard this statement several times but never understood why "export" changes anything? Isn't instantiation point and so context the same regardless of whether template is exported or not?
The idea is that the author of an exported template expects to be able to control the context of at least some of the names that are looked up. That applies to the author of a nonexported template but he doesn't have quite the same right to the expectation since he can't really control the files that are #included before the template is seen. -- Dave Abrahams Boost Consulting www.boost-consulting.com
Robert Ramey wrote:
I made special efforts to use ADL for the following reasons
a) serialization could in any of a couple of namespaces
Sorry, i can't understand that sentence, even grammatically.
b) I believe that not using it resulted in problems with compilers that implement two-phase lookup
What kind of problems? Could not those problems be solved by appropriate forward declarations?
c) a number of people asked for it
What were their reasons? - Volodya
I confess I've just looked at it. I'll apply it here and re-run all my tests with all my compilers. What version of gcc are your running. If I remember correctly - two-phase lookup was introduced at gcc 4.0 Robert Ramey Vladimir Prus wrote:
With unmodified CVS state of serialization, I get errors from compiler because archive/detail/iserializer.hpp contains unqualified call to 'load', and it find both serialization's 'load', and some other 'load' that I happen to have.
I wonder if ADL is desired here? IIUC, the only customization point is the 'serialize' function -- that will call user provided 'load' and 'save' explictly. So, there's no need for ADL in iserializer.hpp.
With the attached patch, things start to work for me. Is it possible to apply it?
- Volodya
_______________________________________________ Unsubscribe & other changes: http://lists.boost.org/mailman/listinfo.cgi/boost
participants (3)
-
David Abrahams -
Robert Ramey -
Vladimir Prus