[Fusion] begin/end ADL issues with C++0x range-based for
Hi, In some cases, Fusion Sequence cannot be used in C++0x range-based for (available on GCC 4.6 Pre-Release). For example, this code #include <vector> #include <boost/fusion/include/vector.hpp> int main(int argc, char* argv[]) { typedef boost::fusion::vector<int, int> sequence; for (auto x : std::vector<sequence>()) {} return 0; } does not compile. The C++0x range-based for for (auto x : std::vector<sequence>()) {} is equivalent to { auto&& rng = std::vector<sequence>(); for ( auto it_begin = begin(rng), it_end = end(rng); it_begin != it_end; ++it_begin ) {} } Note that rng has std and boost::fusion as its associated namespaces and the unqualified begin/end function call triggers ADL. The reason for the compiler errors is that the call to begin/end is ambiguous between std::begin/end and boost::fusion::begin/end. (Also boost::fusion::begin/end tries to instanciate result_of::begin/end::<std::vector<sequence> > which leads to a compiler error.) These compiler errors can be avoided by using SFINAE (lazy_enable_if with "is_sequence" metafunction) to boost::fusion::begin/end: // <boost/fusion/sequence/intrinsic/begin.hpp> template <typename Sequence> inline typename lazy_enable_if< traits::is_sequence<Sequence> , typename result_of::begin<Sequence> >::type const begin(Sequence& seq) { return result_of::begin<Sequence>::call(seq); } I attached a patch (against trunk) and a test case. The test case does not compile before applying the patch, but compiles fine after applying the patch. Joel and Christopher, what do you think about this change? Regards, Michel
Michel MORIN schrieb:
Hi,
In some cases, Fusion Sequence cannot be used in C++0x range-based for (available on GCC 4.6 Pre-Release). For example, this code
#include <vector> #include <boost/fusion/include/vector.hpp>
int main(int argc, char* argv[]) { typedef boost::fusion::vector<int, int> sequence;
for (auto x : std::vector<sequence>()) {}
return 0; }
does not compile.
The C++0x range-based for for (auto x : std::vector<sequence>()) {} is equivalent to { auto&& rng = std::vector<sequence>(); for ( auto it_begin = begin(rng), it_end = end(rng); it_begin != it_end; ++it_begin ) {} } Note that rng has std and boost::fusion as its associated namespaces and the unqualified begin/end function call triggers ADL.
The reason for the compiler errors is that the call to begin/end is ambiguous between std::begin/end and boost::fusion::begin/end. (Also boost::fusion::begin/end tries to instanciate result_of::begin/end::<std::vector<sequence> > which leads to a compiler error.)
These compiler errors can be avoided by using SFINAE (lazy_enable_if with "is_sequence" metafunction) to boost::fusion::begin/end:
// <boost/fusion/sequence/intrinsic/begin.hpp> template <typename Sequence> inline typename lazy_enable_if< traits::is_sequence<Sequence> , typename result_of::begin<Sequence> >::type const begin(Sequence& seq) { return result_of::begin<Sequence>::call(seq); }
I attached a patch (against trunk) and a test case. The test case does not compile before applying the patch, but compiles fine after applying the patch. Joel and Christopher, what do you think about this change?
Regards, Michel
Your patch makes perfect sense to me. BTW., Mathias Gaunard created a ticket for this issue a while ago. He also proposed disabling fusion::begin/fusion::end via SFINAE. https://svn.boost.org/trac/boost/ticket/4028 -Christopher
On 12/19/10 12:03 AM, Christopher Schmidt wrote:
Michel MORIN schrieb:
Hi,
In some cases, Fusion Sequence cannot be used in C++0x range-based for (available on GCC 4.6 Pre-Release). For example, this code
[snip]
I attached a patch (against trunk) and a test case. The test case does not compile before applying the patch, but compiles fine after applying the patch. Joel and Christopher, what do you think about this change?
Regards, Michel
Your patch makes perfect sense to me. BTW., Mathias Gaunard created a ticket for this issue a while ago. He also proposed disabling fusion::begin/fusion::end via SFINAE.
Makes perfect sense. Feel free to commit the patch. Regards, -- Joel de Guzman http://www.boostpro.com http://spirit.sf.net
Joel de Guzman schrieb:
On 12/19/10 12:03 AM, Christopher Schmidt wrote:
Michel MORIN schrieb:
Hi,
In some cases, Fusion Sequence cannot be used in C++0x range-based for (available on GCC 4.6 Pre-Release). For example, this code
[snip]
I attached a patch (against trunk) and a test case. The test case does not compile before applying the patch, but compiles fine after applying the patch. Joel and Christopher, what do you think about this change?
Regards, Michel
Your patch makes perfect sense to me. BTW., Mathias Gaunard created a ticket for this issue a while ago. He also proposed disabling fusion::begin/fusion::end via SFINAE.
Makes perfect sense. Feel free to commit the patch.
Regards,
Committed: https://svn.boost.org/trac/boost/changeset/67352 Unfortunately this still leaves us with ADL ambiguity in some cases: https://svn.boost.org/trac/boost/ticket/4028#comment:6 -Christopher
On 12/20/2010 9:21 AM, Christopher Schmidt wrote: [...]
Unfortunately this still leaves us with ADL ambiguity in some cases:
https://svn.boost.org/trac/boost/ticket/4028#comment:6
-Christopher
That boost::fusion::begin/end could be found via ADL in range-based for loops is a bit worrying, and, as mentioned in the ticket, SFINAE'ing out boost::fusion::begin/end for only Boost.Fusion sequences is only a partial solution, as it doesn't help for structures having both range and Boost.Fusion interfaces. I don't know if I fully understand the problem, and I don't know if the following is a viable solution or what all the implications are (yes, that's a lot of qualifications), but maybe we could put the Boost.Fusion data structures (and anything else that would cause boost::fusion to be included in the set of "associated namespaces" when using ADL for unqualified begin/end) into another namespace, and bring them into the boost::fusion namespace with using declarations. Then maybe boost::fusion::begin/end wouldn't be found via ADL, but one would still refer to them as boost::fusion::vector, etc. E.g., the following compiles on MSVC9: namespace fusion_data_structures { struct X { }; } // namespace fusion_data_structures namespace fusion { template< class T > void f(T) { } using fusion_data_structures::X; } // namespace fusion template< class T > void f(T) { } int main(int argc, char* argv[]) { fusion::X x; f(x); return 0; } The fusion_data_structures namespace would be an implementation-detail, never to be referred to outside of the Boost.Fusion library. Just a thought, - Jeff
Hi, Thanks Christopher for your commitment. Jeffrey Lee Hellrung, Jr. wrote:
SFINAE'ing out boost::fusion::begin/end for only Boost.Fusion sequences is only a partial solution, as it doesn't help for structures having both range and Boost.Fusion interfaces.
True. SFINAE'ing out is only a partial solution. Better solutions would be: A. (almost the same as Jeffrey's suggestion) If boost::fusion::begin/end doesn't need to be found via ADL, then use ADL barrier technique to fusion::begin/end (i.e. defining begin/end in another namespace and pull its name in namespace fusion by using directive/declaration). B. If boost::begin/end doesn't need to be found via ADL, then use ADL barrier technique to boost::begin/end. C. Define begin/end in namespace boost for all containers and ranges which live in namespace boost (boost::array, boost::iterator_range, boost::unordered_map, etc.). For example, define begin/end for boost::array in namespace boost. Since this is very tedious work, it would be better to define some BOOST_DEFINE_BEGIN_END macro for boilerplate code generation. Solutions B and C are, again, only a partial solution, since this only solves ambiguity call problems in namespace boost; the same thing can happen in another namespace. Solution A might break existing codes that relies on ADL to find boost::fusion::begin/end. Assuming any code in the Boost libraries does not rely on ADL for boost::fusion::begin/end, how about defining switch macro BOOST_FUSION_BEGIN_END_NO_ADL and making a user to choose whether A is applied or not? Regards, Michel
Michel MORIN wrote:
Better solutions would be:
A. (almost the same as Jeffrey's suggestion) If boost::fusion::begin/end doesn't need to be found via ADL, then use ADL barrier technique to fusion::begin/end (i.e. defining begin/end in another namespace and pull its name in namespace fusion by using directive/declaration).
B. If boost::begin/end doesn't need to be found via ADL, then use ADL barrier technique to boost::begin/end.
C. Define begin/end in namespace boost for all containers and ranges which live in namespace boost (boost::array, boost::iterator_range, boost::unordered_map, etc.). For example, define begin/end for boost::array in namespace boost. Since this is very tedious work, it would be better to define some BOOST_DEFINE_BEGIN_END macro for boilerplate code generation.
Oooops, I made a mistake! Definitely, solution B is not a choice. It diables Boost-interfaced range from using in range-based for. So let's forget about solution B. Regards, Michel
On 12/21/2010 12:56 PM, Michel MORIN wrote:
Michel MORIN wrote:
Better solutions would be:
A. (almost the same as Jeffrey's suggestion) If boost::fusion::begin/end doesn't need to be found via ADL, then use ADL barrier technique to fusion::begin/end (i.e. defining begin/end in another namespace and pull its name in namespace fusion by using directive/declaration).
B. If boost::begin/end doesn't need to be found via ADL, then use ADL barrier technique to boost::begin/end.
C. Define begin/end in namespace boost for all containers and ranges which live in namespace boost (boost::array, boost::iterator_range, boost::unordered_map, etc.). For example, define begin/end for boost::array in namespace boost. Since this is very tedious work, it would be better to define some BOOST_DEFINE_BEGIN_END macro for boilerplate code generation.
Oooops, I made a mistake! Definitely, solution B is not a choice. It diables Boost-interfaced range from using in range-based for.
So let's forget about solution B.
A is the only viable option, AFAIK. Regards, -- Joel de Guzman http://www.boostpro.com http://spirit.sf.net
Joel de Guzman wrote:
A. (almost the same as Jeffrey's suggestion) If boost::fusion::begin/end doesn't need to be found via ADL, then use ADL barrier technique to fusion::begin/end (i.e. defining begin/end in another namespace and pull its name in namespace fusion by using directive/declaration).
A is the only viable option, AFAIK.
+1 Regards, Michel
Joel de Guzman schrieb:
On 12/21/2010 12:56 PM, Michel MORIN wrote:
Michel MORIN wrote:
Better solutions would be:
A. (almost the same as Jeffrey's suggestion) If boost::fusion::begin/end doesn't need to be found via ADL, then use ADL barrier technique to fusion::begin/end (i.e. defining begin/end in another namespace and pull its name in namespace fusion by using directive/declaration).
B. If boost::begin/end doesn't need to be found via ADL, then use ADL barrier technique to boost::begin/end.
C. Define begin/end in namespace boost for all containers and ranges which live in namespace boost (boost::array, boost::iterator_range, boost::unordered_map, etc.). For example, define begin/end for boost::array in namespace boost. Since this is very tedious work, it would be better to define some BOOST_DEFINE_BEGIN_END macro for boilerplate code generation.
Oooops, I made a mistake! Definitely, solution B is not a choice. It diables Boost-interfaced range from using in range-based for.
So let's forget about solution B.
A is the only viable option, AFAIK.
Let me throw another possibility into the mix. This problem - a possible ambiguity between fusion::begin and std::begin/boost::begin is uncommon. In fact, only adapted boost::array's and std::array's are both fusion sequences and ranges. To get ADL kicking in we'd actually need an array of one of the inbuilt fusion container... We could disable fusion::begin/fusion::end for those few types that may collide via a new fusion extension metafunction. Say: namespace boost{namespace fusion { namespace extension {template<typename>struct disable_begin_end_impl;} namespace result_of {template<typename>struct begin;}; template <typename Seq> typename lazy_enable_if< mpl::and_< traits::is_sequence<Seq> , mpl::not_<extension::disable_begin_end_impl< typename traits::tag_of<Seq>::type >> > , result_of::begin<Seq> >::type begin(Seq&& seq); }} That way we'd just cripple the fusion interface for adapted arrays and probably a minority of user-defined fusion sequences. Ultimately, to make such types full-blown fusion sequences again, we could offer a new (meta-)function, for example fusion::pack/fusion::result_of::pack, which takes such a 'reduced' fusion sequence and returns an implementation-specific object/type that encapsulates the passed sequence and offers a full-blown fusion interface. To do so, that wrapper forwards its fusion implementation to the implementation of the underlying type - but always returns mpl::false_ for extension::disable_begin_end_impl. It's not pretty - but this approach does not break much and we'd keep ADL, even for fusion::begin/fusion::end, in most cases. Do I make sense? -Christopher
On 12/21/2010 5:33 AM, Christopher Schmidt wrote: [...]
Let me throw another possibility into the mix. This problem - a possible ambiguity between fusion::begin and std::begin/boost::begin is uncommon. In fact, only adapted boost::array's and std::array's are both fusion sequences and ranges. To get ADL kicking in we'd actually need an array of one of the inbuilt fusion container...
We could disable fusion::begin/fusion::end for those few types that may collide via a new fusion extension metafunction.
Say:
namespace boost{namespace fusion { namespace extension {template<typename>struct disable_begin_end_impl;}
namespace result_of {template<typename>struct begin;};
template<typename Seq> typename lazy_enable_if< mpl::and_< traits::is_sequence<Seq> , mpl::not_<extension::disable_begin_end_impl< typename traits::tag_of<Seq>::type >> > , result_of::begin<Seq> >::type begin(Seq&& seq); }}
That way we'd just cripple the fusion interface for adapted arrays and probably a minority of user-defined fusion sequences. Ultimately, to make such types full-blown fusion sequences again, we could offer a new (meta-)function, for example fusion::pack/fusion::result_of::pack, which takes such a 'reduced' fusion sequence and returns an implementation-specific object/type that encapsulates the passed sequence and offers a full-blown fusion interface. To do so, that wrapper forwards its fusion implementation to the implementation of the underlying type - but always returns mpl::false_ for extension::disable_begin_end_impl.
It's not pretty - but this approach does not break much and we'd keep ADL, even for fusion::begin/fusion::end, in most cases.
Do I make sense?
I would like to see a situation where finding boost::fusion::begin/end via ADL is actually useful (i.e., couldn't be replaced by explicit qualification). Until then, I believe preventing boost::fusion::begin/end (and, possibly, boost::begin/end) from being found via ADL is a better solution; further, it explicitly discourages any future code from relying on ADL (assuming that relying on ADL *should* be discouraged). - Jeff
AMDG On 12/21/2010 12:29 PM, Jeffrey Lee Hellrung, Jr. wrote:
I would like to see a situation where finding boost::fusion::begin/end via ADL is actually useful (i.e., couldn't be replaced by explicit qualification). Until then, I believe preventing boost::fusion::begin/end (and, possibly, boost::begin/end) from being found via ADL is a better solution; further, it explicitly discourages any future code from relying on ADL (assuming that relying on ADL *should* be discouraged).
ADL for Fusion may be convenient in some cases, but it should never be necessary because fusion::begin/end are not the customization points. In Christ, Steven Watanabe
ADL for Fusion may be convenient in some cases,
Such as I/O and comparisons? BTW, I'm curious to know which is better in this case: - Define Fusion sequences in an ADL-barrier namespace. For the functions that need to be found via ADL, also define them in that namespace. or - Define Fusion's begin/end (and other functions that should NOT be found via ADL) in an ADL barrier namespace. Regards, Michel
On Thu, Dec 30, 2010 at 6:54 PM, Michel MORIN <mimomorin@gmail.com> wrote:
ADL for Fusion may be convenient in some cases,
Such as I/O and comparisons?
BTW, I'm curious to know which is better in this case: - Define Fusion sequences in an ADL-barrier namespace. For the functions that need to be found via ADL, also define them in that namespace. or - Define Fusion's begin/end (and other functions that should NOT be found via ADL) in an ADL barrier namespace.
I think that putting the functions into a barrier namespace allows us to continue using ADL for other functions. For this reason I prefer protecting the functions. This is what I shall do for Boost.Range begin/end.
Regards, Michel
Regards, Neil Groves
Michel MORIN wrote:
B. If boost::begin/end doesn't need to be found via ADL, then use ADL barrier technique to boost::begin/end.
Oooops, I made a mistake! Definitely, solution B is not a choice. It diables Boost-interfaced range from using in range-based for.
Oops, again. Solution B is not a choice, but the reason is wrong. B does not solve the problem, because there are still ambiguity between std::begin/end and boost::fusion::begin/end. Sorry for the noise. Regards, Michel
participants (6)
-
Christopher Schmidt -
Jeffrey Lee Hellrung, Jr. -
Joel de Guzman -
Michel MORIN -
Neil Groves -
Steven Watanabe