On 4/26/07, Peter Dimov <pdimov@mmltd.net> wrote:
Daniel Walker wrote:
Hello,
TR1 section 3.6.4 says that placeholders should be default and copy constructible. However, on gcc and borland boost::bind placeholders are not copy constructible. I tried the following C++0x test.
#include <boost/tr1/functional.hpp> #include <boost/concept_check.hpp>
int main() { boost::function_requires< boost::DefaultConstructible< typeof(std::tr1::placeholders::_1) >
(); boost::function_requires< boost::CopyConstructible< typeof(std::tr1::placeholders::_1) > (); }
This works on gcc 4.1 with GNU's TR1 implementation, but when using Boost.TR1 the second concept check fails to compile.
[...]
The attached patch does this by introducing a macro BOOST_BIND_ENABLE_INLINE_PLACEHOLDERS. When it is defined placeholders are not objects with internal linkage, and their types do not meet the TR1 concept requirements; i.e. they're inline functions. When the macro is not defined placeholders are objects with internal linkage, and their types are both default and copy constructible. The macro is undefined by default. If a user needs inline placeholders for precompiled headers or whatever reason, they can make the trade off and define the macro. The patch also includes user documentation.
I'm not at all sure that breaking the code of all boost::bind users who happen to use precompiled headers with Borland and g++ is acceptable.
I agree that breaking code should be avoided if at all possible. And good news! I just ran the test suite with g++ 4.1 using precompiled bind.hpp and mem_fn.hpp and everything passed with object placeholders. What version of g++ had broken PCH and needed the inline placeholders? Inline placeholders could be enabled for that version using BOOST_TESTED_AT. Perhaps, it's better not to mention PCH in the documentation and just make my change based on the compiler's version number. As for Borland, based on your comments in cvs (thanks!), I think this change is innocuous. From Borland's perspective, it's basically just reverting placeholder.hpp back to revision 1.3. I can try to test it with borland 5.5 later. Incidental, if PCH support on g++ is a big priority, some changes should probably be made to the test files in order to run the test suite with precompiled headers. Basically, #include <boost/bind.hpp> more or less needs to be the first line of each file. It may also be a good idea to add PCH set up to the Jamfile since g++ makes you jump through hoops to precompile headers with .hpp extensions.
On first reading, I was inclined to finish the sentence with "... merely in order to make an irrelevant concept check pass". On second reading, you probably have in mind the fact that Lambda takes its arguments by const reference (boost::bind takes them by value and the deduced type of _1 *is* CopyConstructible in this case.)
Actually, what led me down this road is that it's not possible to overload operators for built-in types like functions or function pointers. Lambda overloads operators for placeholders, for example _1++. So, what I'm trying to do with lambda assumes more than the concept requirements in TR1, but presumable most TR1 implementations will use classes or enums for placeholders. Implementations that don't will not be able to use TR1 placeholders in lambda expressions. They'll still have lambda placeholders until the Phoenix merger, I assume, and maybe after that. But independent of my lambda hacking, I didn't see any reason that gcc users (at least for recent versions of gcc) should be forced into using placeholder types that don't meet the TR1 concept requirements. Actually, maybe the TR1 requirements should be broadened in the final ISO draft to explicitly mandate that placeholders be of class or enum types.
I agree that this is a problem if we want placeholder interoperability under g++ and I'm not sure what is the best way to address the issue.
Well, sticking to a standard set of type requirements is a good start towards insuring interoperability, which is why concept checks are useful in testing. I think lambda can work with TR1 placeholders on g++ using GNU's TR1 implementation (and on other compilers using Boost.TR1). It would be a shame if it didn't also work on g++ (and borland) using Boost.TR1 since your original bind implementation is in many ways the mother of them all and one of the primary reasons bind is in TR1 in the first place!
I'm fairly certain that we ought to leave Borland users alone, though.
Like I said, I don't think this change will impact borland users, other than making placeholders more interoperable. I'll try to test it later though. Daniel