[bind] class template 'value' can corrupt compilability of sound code: Ticket #5141
Hi Peter, list, two days ago I encountered the following situation For a new project I combined some code like: #include <boost/my_lib/my_robust_portable_code.hpp> with some other code #include <boost/malicious/side_effect_code.hpp> #include <boost/my_lib/my_robust_portable_code.hpp> with the effect, that 'my_robust_portable_code.hpp' did not compile anymore. Although I found a fix, applicable to 'my_robust_portable_code.hpp', I think that such a <boost/malicious/side_effect_code.hpp> should generally not be acceptable for independent components of boost. In the concrete case <boost/tread.hpp> has this malicious potential to confuse msvc-[8..10] when it preceded some of my boost/icl code. <boost/tread.hpp> has the potential to break the code of any other boost library and any user code, if those accidentally use certain legal expressions. As the example below shows the corrupted code is not extremely nuts and might occur else were. Fortunately I was able to locate the problem: It's at boost/bind/bind.hpp(112): namespace _bi{ ... template<class T> class value { ... }; ... } //namespace _bi This is a minimal program to demonstrate how 'class value' can confuse msvc: //========================================================= #include <boost/config.hpp> //--- from boost/bind/bind.hpp(112) namespace _bi{ template<class T> class value{}; } //--- affected code --------------------------------------- template <class Type> struct some_attribute { BOOST_STATIC_CONSTANT(int, value = 0); }; template <class Type> struct some_predicate { BOOST_STATIC_CONSTANT(bool, value = ( some_attribute<Type>::value < 0) //error C2059: syntax error : ')' //IF class template value defined before ); }; //========================================================= Although I frankly don't know, how exactly msvc is derailed, it seems to have problems with 'value' in some_attribute<Type>::value < 0 a construction that is extremely frequent due to meta programming conventions introduced by Dave's and Aleksey's boost::mpl. So for a fix I tried to rename 'class value' by something else like 'class _value'. Fortunately this is possible, because the class template is a local implementation object used in boost/bind/bind.hpp only. After renaming, I successfully ran all tests from boost/bind. Find the patch file attached. I have also filed a ticket https://svn.boost.org/trac/boost/ticket/5141 Best regards, Joachim -- Interval Container Library [Boost.Icl] http://www.joachim-faulhaber.de
On 1/29/2011 3:04 PM, Joachim Faulhaber wrote:
Hi Peter, list, [...] This is a minimal program to demonstrate how 'class value' can confuse msvc: //========================================================= #include<boost/config.hpp> //--- from boost/bind/bind.hpp(112) namespace _bi{ template<class T> class value{}; }
//--- affected code --------------------------------------- template<class Type> struct some_attribute { BOOST_STATIC_CONSTANT(int, value = 0); };
template<class Type> struct some_predicate { BOOST_STATIC_CONSTANT(bool, value = ( some_attribute<Type>::value< 0) //error C2059: syntax error : ')' //IF class template value defined before
Does adding parentheses, i.e., value = ( (some_attribute<Type>::value) < 0 ) help? If so, it could be an alternative to renaming the "value" template...although it could require quite a few more changes to boost code...
); }; //=========================================================
Although I frankly don't know, how exactly msvc is derailed, it seems to have problems with 'value' in
some_attribute<Type>::value< 0
...worth filing a bug in the MS Connect database?
a construction that is extremely frequent due to meta programming conventions introduced by Dave's and Aleksey's boost::mpl. So for a fix I tried to rename 'class value' by something else like 'class _value'. Fortunately this is possible, because the class template is a local implementation object used in boost/bind/bind.hpp only.
Not sure how many value struct templates there are under boost, but I wouldn't be surprised if there were another one for which renaming were *not* an option...
After renaming, I successfully ran all tests from boost/bind. Find the patch file attached. I have also filed a ticket https://svn.boost.org/trac/boost/ticket/5141
- Jeff
2011/1/30 Jeffrey Lee Hellrung, Jr. <jhellrung@ucla.edu>:
On 1/29/2011 3:04 PM, Joachim Faulhaber wrote:
Hi Peter, list,
[...]
This is a minimal program to demonstrate how 'class value' can confuse msvc: //========================================================= #include<boost/config.hpp> //--- from boost/bind/bind.hpp(112) namespace _bi{ template<class T> class value{}; }
//--- affected code --------------------------------------- template<class Type> struct some_attribute { BOOST_STATIC_CONSTANT(int, value = 0); };
template<class Type> struct some_predicate { BOOST_STATIC_CONSTANT(bool, value = ( some_attribute<Type>::value< 0) //error C2059: syntax error : ')' //IF class template value defined before
Does adding parentheses, i.e., value = ( (some_attribute<Type>::value) < 0 ) help? If so, it could be an alternative to renaming the "value" template...although it could require quite a few more changes to boost code...
Yes, it does. That's the remedy I applied for "the affected code". But obviously it's much better to heal the "malicious code". Fixing the malicious code is a remedy for all the potential code by users that could suffer from the effect.
); }; //=========================================================
Although I frankly don't know, how exactly msvc is derailed, it seems to have problems with 'value' in
some_attribute<Type>::value< 0
...worth filing a bug in the MS Connect database?
... no experience with that here :-/
a construction that is extremely frequent due to meta programming conventions introduced by Dave's and Aleksey's boost::mpl. So for a fix I tried to rename 'class value' by something else like 'class _value'. Fortunately this is possible, because the class template is a local implementation object used in boost/bind/bind.hpp only.
Not sure how many value struct templates there are under boost, but I wouldn't be surprised if there were another one for which renaming were *not* an option...
Unfortunately you may be right. I did a grep for class value and was happy to find only the *one* in boost bind. But for struct value we have 11 occurrences in boost. Joachim -- Interval Container Library [Boost.Icl] http://www.joachim-faulhaber.de
On 1/29/2011 3:38 PM, Joachim Faulhaber wrote:
2011/1/30 Jeffrey Lee Hellrung, Jr.<jhellrung@ucla.edu>:
On 1/29/2011 3:04 PM, Joachim Faulhaber wrote: [...]
value = ( some_attribute<Type>::value< 0) //error C2059: syntax error : ')' //IF class template value defined before
Does adding parentheses, i.e., value = ( (some_attribute<Type>::value)< 0 ) help? If so, it could be an alternative to renaming the "value" template...although it could require quite a few more changes to boost code...
Yes, it does. That's the remedy I applied for "the affected code". But obviously it's much better to heal the "malicious code". Fixing the malicious code is a remedy for all the potential code by users that could suffer from the effect.
Re: "it's much better to heal the 'malicious code'", yes, usually, but maybe not in this case. This seems to fall into the same category as putting parentheses around min/max to prevent preprocessor macro substitution in the event that windows.h is included. True, we can actually change the offending code in this case unlike the min/max case, but in some instances (at least in theory) that would result in an unfortunate interface identifier change, and in the future, an interface identifier restriction. On the other hand, I agree, a policy such as "put parentheses around all nested integral static constants of a dependent type that are compared less-than some other constant" is not a pretty pill to swallow... [...]
Although I frankly don't know, how exactly msvc is derailed, it seems to have problems with 'value' in
some_attribute<Type>::value< 0
...worth filing a bug in the MS Connect database?
... no experience with that here :-/
I think you'll want http://connect.microsoft.com/VisualStudio If you give me sample cpp, I can verify that I get the same problem (I only have MSVC9 installed) and submit the bug request on your behalf. [...]
Not sure how many value struct templates there are under boost, but I wouldn't be surprised if there were another one for which renaming were *not* an option...
Unfortunately you may be right. I did a grep for
class value
and was happy to find only the *one* in boost bind. But for
struct value
we have 11 occurrences in boost.
How many of these are struct or class templates? I assume the problem only manifests itself if value is a struct or class template. I would also hope it wouldn't matter whether one used the keyword "struct" or "class" to declare the value template ;) - Jeff
2011/1/30 Jeffrey Lee Hellrung, Jr. <jhellrung@ucla.edu>:
On 1/29/2011 3:38 PM, Joachim Faulhaber wrote:
2011/1/30 Jeffrey Lee Hellrung, Jr.<jhellrung@ucla.edu>:
On 1/29/2011 3:04 PM, Joachim Faulhaber wrote:
[...]
value = ( some_attribute<Type>::value< 0) //error C2059: syntax error : ')' //IF class template value defined before
Does adding parentheses, i.e., value = ( (some_attribute<Type>::value)< 0 ) help? If so, it could be an alternative to renaming the "value" template...although it could require quite a few more changes to boost code...
Yes, it does. That's the remedy I applied for "the affected code". But obviously it's much better to heal the "malicious code". Fixing the malicious code is a remedy for all the potential code by users that could suffer from the effect.
Re: "it's much better to heal the 'malicious code'", yes, usually, but maybe not in this case. This seems to fall into the same category as putting parentheses around min/max to prevent preprocessor macro substitution in the event that windows.h is included. True, we can actually change the offending code in this case unlike the min/max case, but in some instances (at least in theory) that would result in an unfortunate interface identifier change, and in the future, an interface identifier restriction. On the other hand, I agree, a policy such as "put parentheses around all nested integral static constants of a dependent type that are compared less-than some other constant" is not a pretty pill to swallow...
not pretty but we could support this new requirement by the inspect tool. Still the problem is that user code that we can not control can be spoiled by the effect. [...]
Not sure how many value struct templates there are under boost, but I wouldn't be surprised if there were another one for which renaming were *not* an option...
Unfortunately you may be right. I did a grep for
class value
and was happy to find only the *one* in boost bind. But for
struct value
we have 11 occurrences in boost.
How many of these are struct or class templates?
All of them are templates, (this is boost code ;) It's late in Europe. More on this from my part tomorrow ... Cheers, Joachim
I was struck by the same behavior on MSVC with nt2. We had to put () or inverse the test ( seems 0 >= value is parsed correctly). I think it's a known limitation of MSVC as I enciutner it quite a lot.
2011/1/30 Joel Falcou <joel.falcou@lri.fr>:
I was struck by the same behavior on MSVC with nt2. We had to put () or inverse the test ( seems 0 >= value is parsed correctly).
At least, we should add a convention to put () around affected expressions to the list of requirements for boost libraries. It could also be checked by the inspect tool.
I think it's a known limitation of MSVC
I've sent a bug report to MS Connect as Jeff suggested ...
as I encounter it quite a lot.
I have looked in trunk/boost for all template<class T, ...> class value template<class T, ...> stuct value templates that can trigger the effect. # library location fixable 1 bind bind.hpp(112) yes 2 accumulators value_accumulator.hpp(60) no 3 flyweight value_tag.hpp(41) yes 4 proto traits.hpp(280) ? 5 phoenix primitives.hpp(114) no 6 core\value.hpp(47) no 7 xpressive regex_actions.hpp(691) no Most of these value templates are in the library's interfaces and can most likely not be renamed without breaking user code. Because these libraries are also used in other libraries, the chance for users to run into this difficulty is not so small. Joachim
[Joachim Faulhaber]
I've sent a bug report to MS Connect as Jeff suggested ...
I found this bug in our database (DevDiv#133382, http://connect.microsoft.com/VisualStudio/feedback/details/640322/class-temp... ) and added an internal note explaining that this is a widespread problem throughout Boost with a link to https://svn.boost.org/trac/boost/ticket/5141 . When filing Connect bugs, don't be shy about mentioning which Boost libraries are affected! Stephan T. Lavavej Visual C++ Libraries Developer
2011/1/31 Stephan T. Lavavej <stl@exchange.microsoft.com>:
[Joachim Faulhaber]
I've sent a bug report to MS Connect as Jeff suggested ...
I found this bug in our database (DevDiv#133382, http://connect.microsoft.com/VisualStudio/feedback/details/640322/class-temp... ) and added an internal note explaining that this is a widespread problem throughout Boost with a link to https://svn.boost.org/trac/boost/ticket/5141 .
Thank you, I hope this gives prominence to the bug-report
When filing Connect bugs, don't be shy about mentioning which Boost libraries are affected!
Du to "include proliferation" via Bind and Proto at least asio, bind, gil, graph, msm, multi_index, python, statechart, test, thread, tr1, spirit, xpressive are affected. Regards, Joachim -- Interval Container Library [Boost.Icl] http://www.joachim-faulhaber.de
participants (4)
-
Jeffrey Lee Hellrung, Jr. -
Joachim Faulhaber -
Joel Falcou -
Stephan T. Lavavej