On Wednesday, June 29, 2011 02:31:56 PM you wrote:
Hi, Thomas and other Boosters!
I promised on #boost yesterday to try implementing make_shared<T>(...) for Boost.Phoenix. Well, I got the first version working, it was mostly a find-and-replace task to the existing phoenix::new_<T>(...) implementation. I put the modifications to the current Phoenix trunk version here:
https://github.com/pyrtsa/boost-svn/commit/b37cb155ff157199a88e59e44f1d8ede e12e0bfc
Nice start, thanks for pulling that of. I think this will be a valuable addition.
First, the questions, then some remarks and ideas for improvement:
1) If I've understood correctly, you and Hartmut used Wave to generate the preprocessed headers. What command did you use for them? (Currently, I just added a false condition to the preprocessor #if's to fallback to the Boost.Preprocessor iteration.)
That is good enough for now. For future reference, you need to change your working directory to libs/phoenix/preprocess, adapt the wave.cfg to point to your correct system headers, then run the wave tool. I usually invoke it with: $ for i in `seq 10 10 50`; do wave -o - -DBOOST_PHOENIX_LIMIT=$i preprocess_phoenix.cpp; done
2) Where would you put the new make_shared stuff? The first implementation resides below boost/phoenix/object, but I think we should use another directory for all the smart pointer stuff - especially if we're trying to support all of (boost|std::tr1|std)! How about the following pattern?
boost/phoenix/smart_ptr/... for all Boost.SmartPtr support stuff boost/phoenix/tr1/... for TR1 support (alternatively, boost/phoenix/std/tr1) boost/phoenix/std/... for C++0x (with #ifdef guards checking against std version)
I just discussed this matter with Joel this morning. We came to the conclusion we need to introduce the namespace boost::phoenix::std and maybe also phoenix::std::tr1 where this make_shared and friend functions would live. Additionnally the other stuff currently living in the boost::phoenix namespace that belong to the STL module would need to be moved there. The same will be done with the (currently undocumented) fusion namespace. With that being said, I think there would need to be three make_shared_ptr functions: - boost::phoenix::make_shared_ptr // using boost::shared_ptr - boost::phoenix::std::tr1::make_shared_ptr // using std::tr1::shared_ptr - boost::phoenix::std::make_shared_ptr // using std::shared_ptr and the same for the other flavours of smart pointers. I think it makes sense to add a smart_ptr module. Thoughts?
3) To my pleasant surprise, I didn't need any hacks to create a functor of type
function<shared_ptr<Parent>()>
using a Phoenix construct like phx::make_shared<Child>(args), where Child inherits Parent. You'll find a working example in https://gist.github.com/1051564. Nevertheless, I think there's also a need for implementing the Phoenix versions of static_pointer_cast etc.
Yup. That makes sense. I am thinking that it might be possible to add this behaviour to the already existing cast versions of phoenix.
4) Another feature worth implementing might be phoenix::make_unique<T>(...).
Which would construct a std::unique_ptr? Yes, as noted above i think it makes sense to add lazy make_* functions for all standard and boost pointers
5) We also discussed about an API that allows you to make "deep" copies of pointed objects in Phoenix. I think it might already work using something like
phoenix::make_shared<T>(ref(x))
if x is of type T, or
phoenix::make_shared<T>(*ref(p))
if p points to T.
Right this will work. What i had in mind was an API that would allow to omit T completely. But this would only make sense if new_ and friends will support it too. Anyway, let's get the basic functionality that is symmetric to construct and new_ inside trunk first and decide upon further actions later.
6) When we get this far, I'll promise help in the documentation effort too. ;-)
We definitely will, and I will get back to your offer :P Besides documentation we also need unit tests ;) Thanks for the work.