[variant] Breaking change introduced by variadic support
The recent changes to introduce variadic support to `variant` introduce breaking changes in the documented macros. When variadic support is detected, `BOOST_VARIANT_LIMIT_TYPES` is not defined and `BOOST_VARIANT_ENUM_[SHIFTED_]PARAMS` uses variadic templates instead of enumerating parameters. This is a breaking change in the documented interface, and one that requires all but the simplest use cases to be rewritten. For instance, this breaks _Boost.Spirit_ support of `variant` (https://svn.boost.org/trac/boost/ticket/9238), requiring two different implementations to be provided. Is this the preferred path to take? I would suggest instead to keep the macros as documented at all times, and let the user handle the variadic case manually. This would allow users to support both variadic and non-variadic `variant`s up to a certain limit with a single code case (without breaking the documented interface), and result in cleaner code for those cases that only target compilers with variadic support. There's no point in using a macro if it won't work for both cases. Regards, -- Agustín K-ballo Bergé.- http://talesofcpp.fusionfenix.com
2014/1/18 Agustín K-ballo Bergé <kaballo86@hotmail.com>
The recent changes to introduce variadic support to `variant` introduce breaking changes in the documented macros. When variadic support is detected, `BOOST_VARIANT_LIMIT_TYPES` is not defined and `BOOST_VARIANT_ENUM_[SHIFTED_]PARAMS` uses variadic templates instead of enumerating parameters.
This is a breaking change in the documented interface, and one that requires all but the simplest use cases to be rewritten. For instance, this breaks _Boost.Spirit_ support of `variant` (https://svn.boost.org/trac/ boost/ticket/9238), requiring two different implementations to be provided.
Is this the preferred path to take? I would suggest instead to keep the macros as documented at all times, and let the user handle the variadic case manually. This would allow users to support both variadic and non-variadic `variant`s up to a certain limit with a single code case (without breaking the documented interface), and result in cleaner code for those cases that only target compilers with variadic support. There's no point in using a macro if it won't work for both cases.
Unfortunately this will make previous code just "look like" it works. Variadic template version of variant (`variant<T0, TN...>`) is not same as variant generated by BOOST_VARIANT_LIMIT_TYPES and preprocessor (`variant<T0, T1, T2, T3, T4, T5, T6, T7, T8>`). Leaving BOOST_VARIANT_LIMIT_TYPES defined will result in many-many hard detectable template errors. Here is an example template <class T> struct is_variant: boost::false_type {}; template </*`class TX` generated by handwritten preprocessor code and BOOST_VARIANT_LIMIT_TYPES*/> struct is_variant< variant</*`TX` generated by handwritten preprocessor code and BOOST_VARIANT_LIMIT_TYPES*/> > : boost::true_type {}; Following code will produce different results, depending on availability of variadic templates: is_variant<variant<int, char> >::value That's because *there is* a difference between variant<T0, TN...> and variant<T0, T1, T2, T3, T4, T5, T6, T7, T8> I've put a lot of effort to make transition as smooth as possible: * users that use BOOST_VARIANT_ENUM_[SHIFTED_]PARAMS shall notice no change: // BOOST_VARIANT_ENUM_PARAMS(class Something) => class Something0, class... SomethingN // BOOST_VARIANT_ENUM_PARAMS(Something) => Something0, SomethingN... * users are free to define BOOST_VARIANT_DO_NOT_USE_VARIADIC_TEMPLATES and do not use variadic templates version at all. * users that use BOOST_VARIANT_LIMIT_TYPES won't get silent templates errors and will be able to take care about the issues. Any ideas about how to make transition smoother are welcomed! -- Best regards, Antony Polukhin
On 18/01/2014 02:32 p.m., Antony Polukhin wrote:
2014/1/18 Agustín K-ballo Bergé <kaballo86@hotmail.com>
The recent changes to introduce variadic support to `variant` introduce breaking changes in the documented macros. When variadic support is detected, `BOOST_VARIANT_LIMIT_TYPES` is not defined and `BOOST_VARIANT_ENUM_[SHIFTED_]PARAMS` uses variadic templates instead of enumerating parameters.
This is a breaking change in the documented interface, and one that requires all but the simplest use cases to be rewritten. For instance, this breaks _Boost.Spirit_ support of `variant` (https://svn.boost.org/trac/ boost/ticket/9238), requiring two different implementations to be provided.
Is this the preferred path to take? I would suggest instead to keep the macros as documented at all times, and let the user handle the variadic case manually. This would allow users to support both variadic and non-variadic `variant`s up to a certain limit with a single code case (without breaking the documented interface), and result in cleaner code for those cases that only target compilers with variadic support. There's no point in using a macro if it won't work for both cases.
Unfortunately this will make previous code just "look like" it works. Variadic template version of variant (`variant<T0, TN...>`) is not same as variant generated by BOOST_VARIANT_LIMIT_TYPES and preprocessor (`variant<T0, T1, T2, T3, T4, T5, T6, T7, T8>`). Leaving BOOST_VARIANT_LIMIT_TYPES defined will result in many-many hard detectable template errors.
Nod, this is unfortunate but at least the current approach results in hard to diagnose *compilation* errors, which is better IMO.
I've put a lot of effort to make transition as smooth as possible: * users that use BOOST_VARIANT_ENUM_[SHIFTED_]PARAMS shall notice no change: // BOOST_VARIANT_ENUM_PARAMS(class Something) => class Something0, class... SomethingN // BOOST_VARIANT_ENUM_PARAMS(Something) => Something0, SomethingN...
This is not the case, see the compilation failure in Spirit. The enum macros are documented to define param##N arguments. Additionally, dealing with all template arguments at the same time gives better compile-time performance than recursive approaches. This is not prevented by variadic templates, but having them separated in head and tail do make it slightly more complicated. I would still suggest that the variadic case we handled manually by the user, while the macros would be there only to support the simplest existing use cases.
* users are free to define BOOST_VARIANT_DO_NOT_USE_VARIADIC_TEMPLATES and do not use variadic templates version at all.
The end user may be free to define it, libraries cannot.
* users that use BOOST_VARIANT_LIMIT_TYPES won't get silent templates errors and will be able to take care about the issues.
Any ideas about how to make transition smoother are welcomed!
I will continue thinking about this. I find unfortunate that users are forced to provide different codes for variadic and non-variadic cases. Worse, the compilation errors do not point to `variant` at all, and one would not usually suspect that a macro named `ENUM` does not enumerate anymore. Regards, -- Agustín K-ballo Bergé.- http://talesofcpp.fusionfenix.com
2014/1/18 Agustín K-ballo Bergé <kaballo86@hotmail.com>
On 18/01/2014 02:32 p.m., Antony Polukhin wrote:
* users that use BOOST_VARIANT_LIMIT_TYPES won't get silent templates errors and will be able to take care about the issues.
Any ideas about how to make transition smoother are welcomed!
I will continue thinking about this. I find unfortunate that users are forced to provide different codes for variadic and non-variadic cases. Worse, the compilation errors do not point to `variant` at all, and one would not usually suspect that a macro named `ENUM` does not enumerate anymore.
And how about defining BOOST_VARIANT_LIMIT_TYPES to something like "To resolve this issue see following link [link to description of the problem and its possible fixes]"? -- Best regards, Antony Polukhin
On 18/01/2014 04:10 p.m., Antony Polukhin wrote:
2014/1/18 Agustín K-ballo Bergé <kaballo86@hotmail.com>
On 18/01/2014 02:32 p.m., Antony Polukhin wrote:
* users that use BOOST_VARIANT_LIMIT_TYPES won't get silent templates errors and will be able to take care about the issues.
Any ideas about how to make transition smoother are welcomed!
I will continue thinking about this. I find unfortunate that users are forced to provide different codes for variadic and non-variadic cases. Worse, the compilation errors do not point to `variant` at all, and one would not usually suspect that a macro named `ENUM` does not enumerate anymore.
And how about defining BOOST_VARIANT_LIMIT_TYPES to something like "To resolve this issue see following link [link to description of the problem and its possible fixes]"?
That would be a good last resort solution. Regards, -- Agustín K-ballo Bergé.- http://talesofcpp.fusionfenix.com
On Sat, Jan 18, 2014 at 11:10 AM, Antony Polukhin <antoshkka@gmail.com>wrote:
And how about defining BOOST_VARIANT_LIMIT_TYPES to something like "To resolve this issue see following link [link to description of the problem and its possible fixes]"?
It might be possible to produce a nice error message via _Pragma that would be inside of the macro definition when the variadic template implementation is used, or at least a nice note during compilation. My initial experiments with that haven't led to anything worthwhile that is better than simply defining it to a string literal. Just throwing it out there as a possible direction for anyone who might have some ideas. -- -Matt Calabrese
On 18/01/2014 02:32 p.m., Antony Polukhin wrote:
2014/1/18 Agustín K-ballo Bergé <kaballo86@hotmail.com>
The recent changes to introduce variadic support to `variant` introduce breaking changes in the documented macros. When variadic support is detected, `BOOST_VARIANT_LIMIT_TYPES` is not defined and `BOOST_VARIANT_ENUM_[SHIFTED_]PARAMS` uses variadic templates instead of enumerating parameters.
This is a breaking change in the documented interface, and one that requires all but the simplest use cases to be rewritten. For instance, this breaks _Boost.Spirit_ support of `variant` (https://svn.boost.org/trac/ boost/ticket/9238), requiring two different implementations to be provided.
While trying to make _Boost.Spirit_ work after the breaking changes, I found a problem with the following snippet: #include <boost/variant.hpp> #include <boost/mpl/bool.hpp> template <typename T> struct is_container : boost::mpl::false_ {}; template <typename T> struct is_container<boost::variant<T>> : is_container<T> {}; template <BOOST_VARIANT_ENUM_PARAMS(typename T)> struct is_container<boost::variant<BOOST_VARIANT_ENUM_PARAMS(T)>> : boost::mpl::bool_<is_container<T0>::value || is_container<boost::variant<BOOST_VARIANT_ENUM_SHIFTED_PARAMS(T)>>::value> // #13 {}; int main() { is_container<boost::variant<double, int>>::value; } If I understand correctly, this should work for both the variadic and non-variadic cases. However, trying this with both MSVC2013 and NovCTP13 gives the following error:
main.cpp(13): error C2976: 'boost::variant' : too few template arguments
The code works fine for GCC4.8.2 (both for C++03 and C++11), and it works with Clang 3.3 too although it does not use the variadic version (why?). Is this a bug with MSVC, or did I do something wrong? If the former, I would suggest sticking with the non-variadic approach for MSVC, since this seems to be a pretty basic use case. Also, feel free to use the previous code as a base for a regression test. Regards, -- Agustín K-ballo Bergé.- http://talesofcpp.fusionfenix.com
2014/1/21 Agustín K-ballo Bergé <kaballo86@hotmail.com> <...>
While trying to make _Boost.Spirit_ work after the breaking changes, I found a problem with the following snippet:
#include <boost/variant.hpp> #include <boost/mpl/bool.hpp>
template <typename T> struct is_container : boost::mpl::false_ {};
template <typename T> struct is_container<boost::variant<T>> : is_container<T> {};
template <BOOST_VARIANT_ENUM_PARAMS(typename T)> struct is_container<boost::variant<BOOST_VARIANT_ENUM_PARAMS(T)>> : boost::mpl::bool_<is_container<T0>::value || is_container<boost::variant<BOOST_VARIANT_ENUM_SHIFTED_PARAMS(T)>>::value> // #13 {};
int main() { is_container<boost::variant<double, int>>::value; }
If I understand correctly, this should work for both the variadic and non-variadic cases. However, trying this with both MSVC2013 and NovCTP13 gives the following error:
main.cpp(13): error C2976: 'boost::variant' : too few template arguments
This looks like an issue in MSVC2013. MSVC2013 choose to use variadic tempalte version even if there is a specialization with one template. Following code must reproduce it (did not check it): #include <boost/mpl/bool.hpp> template <typename T0, typename TN...> struct variant{}; template <typename T> struct is_container : boost::mpl::false_ {}; template <typename T0, typename TN...> struct is_container<variant<T0, TN...>> : boost::mpl::bool_<is_container<T0>::value || is_container<variant<TN...>>::value> {};
The code works fine for GCC4.8.2 (both for C++03 and C++11), and it works with Clang 3.3 too although it does not use the variadic version (why?).
This may be an issue with Boost.Config. I've tested on clang 3.0 and it used the variadic templates.
Is this a bug with MSVC, or did I do something wrong? If the former, I would suggest sticking with the non-variadic approach for MSVC, since this seems to be a pretty basic use case.
Some GCCs prior to 4.7 have variadic templates support, but Variant does not use them because of compiler implementation issues. Looks like the same approach must be taken with MSVC2013. Also, feel free to use the previous code as a base for a regression test. Thanks! I'll commit this test case in a few days. -- Best regards, Antony Polukhin
2014/1/21 Agustín K-ballo Bergé <kaballo86@hotmail.com>
While trying to make _Boost.Spirit_ work after the breaking changes, I found a problem with the following snippet:
#include <boost/variant.hpp> #include <boost/mpl/bool.hpp>
template <typename T> struct is_container : boost::mpl::false_ {};
template <typename T> struct is_container<boost::variant<T>> : is_container<T> {};
template <BOOST_VARIANT_ENUM_PARAMS(typename T)> struct is_container<boost::variant<BOOST_VARIANT_ENUM_PARAMS(T)>> : boost::mpl::bool_<is_container<T0>::value || is_container<boost::variant<BOOST_VARIANT_ENUM_SHIFTED_PARAMS(T)>>::value> // #13 {};
int main() { is_container<boost::variant<double, int>>::value; }
If I understand correctly, this should work for both the variadic and non-variadic cases. However, trying this with both MSVC2013 and NovCTP13 gives the following error:
main.cpp(13): error C2976: 'boost::variant' : too few template arguments
Added test case and disabled usage of variadic templates in Boost.Variant for MSVC2013 ( https://github.com/boostorg/variant/commit/63fb3ff4270e3032c0a171507bb41718e... ) -- Best regards, Antony Polukhin
participants (3)
-
Agustín K-ballo Bergé -
Antony Polukhin -
Matt Calabrese