boost::iterator_range header surgery
Hello, I found that the input/output and comparison functions for iterator_range are inside the header boost/range/iterator_range.hpp. Actually, this causes problems, as I can't provide custom printing and custom comparison: I have my own container printing engine and it worked perfectly well with all standard containers, but failed to compile with iterator_range due to operator<< ambiguity. (Of course, I can just disable instantiation of my printing stuff for iterator_range, but this is definitely not what I want to have.) And, I have no such problems with, say, boost::tuple, as it has separate headers for printing and comparison: #include "boost/tuple/tuple_comparison.hpp" #include "boost/tuple/tuple_io.hpp" There are also other boost libraries that provide a separate "XXX_io.hpp" header. So my proposal is: 1. Move operator<< to a separate header "boost/range/iterator_range_io.hpp" 2. Move comparison operators to a separate header "boost/range/iterator_range_comparison.hpp" This will also serve for boost unification, which is the Right Thing that greatly simplifies usage of any library collection. However, two problems arise here: 1. Backward compatibility. Current programs expect that everything is automatically included. I see two solutions: - Include "boost/range/iterator_range_io.hpp" and "boost/range/iterator_range_comparison.hpp" into "boost/range/iterator_range.hpp" under a define like BOOST_RANGE_1_34_COMPATIBILITY_MODE. This will give the same layout as we have in Boost.Tuple, but I don't want to introduce extra macros if they can be easily avioded. - Make one extra header "boost/range/iterator_range_core.hpp", and have "boost/range/iterator_range.hpp" just including everything. This will the hierarchical layout similar to other libraries like Boost.Spirit. In this case, maybe it worth moving all three new headers to a subdirectory. 2. Again, backward compatibility, but from another point of view. Boost.Range has the unspecified_bool_type() conversion operator. I personally don't see a strong need in it, as it's just a negation of empty(). Also, no standard containers (which are ranges) offer this kind of conversion. So I don't expect wide use of it, and maybe it's worth removing it from the class at all and leave empty() only. Anyway, for now it's here, so if you move operator<< away, iterator_range will continue to be printed, but now, due to this conversion, it will be just "0" or "1", depending on the range emptiness. This is the problem not only for Boost.Range, but also for other types that offer this kind of conversion and separate printing, for example, consider this simple program with Boost.Optional: #include <boost/optional/optional.hpp> boost::optional<char> o('o'); #include <iostream> void without_io() { std::cout << "without_io: " << o << '\n'; } #include <boost/optional/optional_io.hpp> void with_io() { std::cout << "with_io: " << o << '\n'; } int main() { without_io(); with_io(); } So, whether we change Boost.Range header leayout or not, this unspecified_bool_type() problem should be solved anyway, for several boost classes. I discussed this with Alex Nasonov, and he proposed a patch for boost::optional: https://svn.boost.org/trac/boost/ticket/2103 But this is not the only use of unspecified_bool_type, so most probably we will need a common solution. Thanks, Maxim P.S. Please write personal e-mails to gmail.com.
on Thu Jul 10 2008, MaximYanchenko <maximyanchenko-AT-yandex.ru> wrote:
Actually, this causes problems, as I can't provide custom printing and custom comparison: I have my own container printing engine and it worked perfectly well with all standard containers, but failed to compile with iterator_range due to operator<< ambiguity. (Of course, I can just disable instantiation of my printing stuff for iterator_range, but this is definitely not what I want to have.) And, I have no such problems with, say, boost::tuple, as it has separate headers for printing and comparison: #include "boost/tuple/tuple_comparison.hpp" #include "boost/tuple/tuple_io.hpp"
There are also other boost libraries that provide a separate "XXX_io.hpp" header.
So my proposal is: 1. Move operator<< to a separate header "boost/range/iterator_range_io.hpp" 2. Move comparison operators to a separate header "boost/range/iterator_range_comparison.hpp"
This will also serve for boost unification, which is the Right Thing that greatly simplifies usage of any library collection.
Isn't this a bit of an invitation to ODR violations? // TU1 #include "MaximIO.hpp" #include "boost/range/iterator_range.hpp" #include "LibraryThatPrints.hpp" print(some_iterator_range); // TU2 #include "boost/range/iterator_range_io.hpp" #include "boost/range/iterator_range.hpp" #include "LibraryThatPrints.hpp" print(some_iterator_range); If the print template calls different IO functions in these two translation units, that's an ODR violation. Just wondering if there isn't a deeper modularity issue lurking here to be solved. -- Dave Abrahams BoostPro Computing http://www.boostpro.com
David Abrahams <dave <at> boostpro.com> writes:
Isn't this a bit of an invitation to ODR violations?
// TU1 #include "MaximIO.hpp" #include "boost/range/iterator_range.hpp" #include "LibraryThatPrints.hpp"
print(some_iterator_range);
// TU2 #include "boost/range/iterator_range_io.hpp" #include "boost/range/iterator_range.hpp" #include "LibraryThatPrints.hpp"
print(some_iterator_range);
If the print template calls different IO functions in these two translation units, that's an ODR violation.
Hi David, that's a good question, and it also addresses boost.tuple and other libraries with detachable io (and detachable anything). In my case, there is no direct ODR violation, as there is actually no print function, but an operator<< living in a separate namespace instead. So when you include "MaximIO.hpp" and use "using MaximIO::operator<<" all your calls to operator<< will be resolved to MaximIO::operator<<, and everything else will be found with help of ADL. But if you include "boost/range/iterator_range_io.hpp" at the same time, you'll get the ambiguity error (that's what I faced). But you are right, if anybody defines a function print without specifying explicitly which io engine to use inside it (pretty common for a general print function), it's easy to get ODR violation here.
Just wondering if there isn't a deeper modularity issue lurking here to be solved.
Of course, when you start using template and inline functions, there is wide area for ODR violation (due to dependency of headers inclusion), for example, ODR can be violated somewhere deep inside my operator<< and the functions it calls. I don't see any 100% safe way to ensure ODR correctness and preserve high modularity, at least now. I see more or less awkward ways like #ifdefs for inclusion control, or passing explicit functors everywhere where we need anything custom instead of defining ODR-fragile entities. Traits are also not very safe here unless they are trivial and very carefully written. Thanks, Maxim P.S. Please write private e-mails to gmail.com
on Sun Jul 13 2008, Maxim Yanchenko <maximyanchenko-AT-yandex.ru> wrote:
David Abrahams <dave <at> boostpro.com> writes:
Isn't this a bit of an invitation to ODR violations?
// TU1 #include "MaximIO.hpp" #include "boost/range/iterator_range.hpp" #include "LibraryThatPrints.hpp"
print(some_iterator_range);
// TU2 #include "boost/range/iterator_range_io.hpp" #include "boost/range/iterator_range.hpp" #include "LibraryThatPrints.hpp"
print(some_iterator_range);
If the print template calls different IO functions in these two translation units, that's an ODR violation.
Hi David, that's a good question, and it also addresses boost.tuple and other libraries with detachable io (and detachable anything).
Only when there's going to be another overload that matches. As long as there's only one definition of tuple streaming, we're OK. My point isn't that separating the IO is dangerous, although it is a little. My point is that the specific thing you are trying to do is dangerous.
In my case, there is no direct ODR violation, as there is actually no print function, but an operator<< living in a separate namespace instead. So when you include "MaximIO.hpp" and use "using MaximIO::operator<<" all your calls to operator<< will be resolved to MaximIO::operator<<,
That won't work unless the using declaration is visible from all the templates that stream these iterator ranges (e.g. std::for_each). -- Dave Abrahams BoostPro Computing http://www.boostpro.com
MaximYanchenko skrev:
Hello,
I found that the input/output and comparison functions for iterator_range are inside the header boost/range/iterator_range.hpp.
Actually, this causes problems, as I can't provide custom printing and custom comparison: I have my own container printing engine and it worked perfectly well with all standard containers, but failed to compile with iterator_range due to operator<< ambiguity. (Of course, I can just disable instantiation of my printing stuff for iterator_range, but this is definitely not what I want to have.) And, I have no such problems with, say, boost::tuple, as it has separate headers for printing and comparison: #include "boost/tuple/tuple_comparison.hpp" #include "boost/tuple/tuple_io.hpp"
There are also other boost libraries that provide a separate "XXX_io.hpp" header.
So my proposal is: 1. Move operator<< to a separate header "boost/range/iterator_range_io.hpp" 2. Move comparison operators to a separate header "boost/range/iterator_range_comparison.hpp"
This will also serve for boost unification, which is the Right Thing that greatly simplifies usage of any library collection.
However, two problems arise here: 1. Backward compatibility. Current programs expect that everything is automatically included. I see two solutions: - Include "boost/range/iterator_range_io.hpp" and "boost/range/iterator_range_comparison.hpp" into "boost/range/iterator_range.hpp" under a define like BOOST_RANGE_1_34_COMPATIBILITY_MODE. This will give the same layout as we have in Boost.Tuple, but I don't want to introduce extra macros if they can be easily avioded. - Make one extra header "boost/range/iterator_range_core.hpp", and have "boost/range/iterator_range.hpp" just including everything. This will the hierarchical layout similar to other libraries like Boost.Spirit. In this case, maybe it worth moving all three new headers to a subdirectory.
The latter approach seems better to me.
2. Again, backward compatibility, but from another point of view. Boost.Range has the unspecified_bool_type() conversion operator. I personally don't see a strong need in it, as it's just a negation of empty(). Also, no standard containers (which are ranges) offer this kind of conversion. So I don't expect wide use of it, and maybe it's worth removing it from the class at all and leave empty() only.
I'm sure it is widely used, and besides, it is there to support a special idiomatic use. So it is not likely yo go away. As for your custom printing, could you not simply avoid to use operator<< altgother (use print()/writeln() or <<=/>>=) ? -Thorsten
participants (4)
-
David Abrahams -
Maxim Yanchenko -
MaximYanchenko
-
Thorsten Ottosen