[smart_ptr] make_shared usage
Hi, I have a question on make_shared. Consider a class which intends to force all instances to be handled via a shared_ptr. I used to do this by putting the ctors into the private section and adding static create()-methods in the public section. Example: class X { private: X( int, double ); public: static shared_ptr<X> create( int i, double d ) { return shared_ptr<X>( new X( i, d ) ); } }; Replacing the returned expression with make_shared<X>( i, d ) wouldn't work, since it requires a public ctor. Is there any solution to this or do I have to make a decision between efficiency and a safe interface for X? Regards, Daniel
I think the safety goes first. So build you share_ptr<X> the way you do, IMO. On Sun, Jul 27, 2008 at 4:15 PM, Daniel Frey <d.frey@gmx.de> wrote:
Hi,
I have a question on make_shared. Consider a class which intends to force all instances to be handled via a shared_ptr. I used to do this by putting the ctors into the private section and adding static create()-methods in the public section. Example:
class X { private: X( int, double );
public: static shared_ptr<X> create( int i, double d ) { return shared_ptr<X>( new X( i, d ) ); } };
Replacing the returned expression with make_shared<X>( i, d ) wouldn't work, since it requires a public ctor.
Is there any solution to this or do I have to make a decision between efficiency and a safe interface for X?
Regards, Daniel
_______________________________________________ Unsubscribe & other changes: http://lists.boost.org/mailman/listinfo.cgi/boost
-- Alp Mestan --- http://blog.mestan.fr/ --- http://alp.developpez.com/ --- In charge of the Qt, Algorithms and Artificial Intelligence sections on Developpez
On Sun, 2008-07-27 at 16:28 +0200, Alp Mestan wrote:
I think the safety goes first. So build you share_ptr<X> the way you do, IMO.
That wasn't the question. I am capable of making the decision if I need to - and it will depend on the context whether safety or efficiency is more important. I don't think there is a general answer and that's the problem. That said, the real question is: Do I have to make that decision or is there any way to have both safety and efficiency? Regards, Daniel
Daniel Frey:
Hi,
I have a question on make_shared. Consider a class which intends to force all instances to be handled via a shared_ptr. I used to do this by putting the ctors into the private section and adding static create()-methods in the public section. Example:
class X { private: X( int, double );
public: static shared_ptr<X> create( int i, double d ) { return shared_ptr<X>( new X( i, d ) ); } };
Replacing the returned expression with make_shared<X>( i, d ) wouldn't work, since it requires a public ctor.
You should be able to make it work by making the appropriate make_shared overload a friend: template< class T, class A1, class A2 > friend boost::shared_ptr< T > boost::make_shared( A1 const & a1, A2 const & a2 );
----- Original Message ----- From: "Peter Dimov" <pdimov@pdimov.com> To: <boost@lists.boost.org> Sent: Sunday, July 27, 2008 5:25 PM Subject: Re: [boost] [smart_ptr] make_shared usage
Daniel Frey:
Hi,
I have a question on make_shared. Consider a class which intends to force all instances to be handled via a shared_ptr. I used to do this by putting the ctors into the private section and adding static create()-methods in the public section. Example:
class X { private: X( int, double );
public: static shared_ptr<X> create( int i, double d ) { return shared_ptr<X>( new X( i, d ) ); } };
Replacing the returned expression with make_shared<X>( i, d ) wouldn't work, since it requires a public ctor.
You should be able to make it work by making the appropriate make_shared overload a friend:
template< class T, class A1, class A2 > friend boost::shared_ptr< T > boost::make_shared( A1 const & a1, A2 const & a2 );
If you declare the boost::make_shared friend, the utility of the create function is questionable. shared_ptr<X> x1_ptr = X::create(0,0); shared_ptr<X> x2_ptr = boost::make_shared <X>(0,0); The make_shared<X> function states clearly that the result is a shared_ptr<X>. Vicente
On Sun, 2008-07-27 at 18:25 +0300, Peter Dimov wrote:
You should be able to make it work by making the appropriate make_shared overload a friend:
template< class T, class A1, class A2 > friend boost::shared_ptr< T > boost::make_shared( A1 const & a1, A2 const & a2 );
Good idea, thanks. It would also render create() useless, since the user could call make_shared directly. Sadly, it doesn't work for me. I've tried it with GCC 4.3: #include <memory> class A { private: A() {} public: static std::shared_ptr< A > create() { return std::make_shared< A >(); } template< typename... Args > friend std::shared_ptr< A > std::make_shared( Args... ); }; int main() { A::create(); } which fails, because the implementation forwards the ctor call into some nested functions. Here's the (terrible to read) output: frey@fiasko:~/work/test/make_shared_2$ g++ -std=c++0x -pedantic -O3 t.cc -o t && ./t t.cc: In constructor ‘std::_Sp_counted_ptr_inplace<_Tp, _Alloc, _Lp>::_Sp_counted_ptr_inplace(_Alloc) [with _Tp = A, _Alloc = std::allocator<A>, __gnu_cxx::_Lock_policy _Lp = _S_mutex]’: /home/frey/work/gcc-4.3.1-install/lib/gcc/i686-pc-linux-gnu/4.3.1/../../../../include/c++/4.3.1/bits/boost_sp_shared_count.h:293: instantiated from ‘std::__shared_count<_Lp>::__shared_count(std::_Sp_make_shared_tag, _Tp*, _Alloc, _Args&& ...) [with _Tp = A, _Alloc = std::allocator<A>, _Args = , __gnu_cxx::_Lock_policy _Lp = _S_mutex]’ /home/frey/work/gcc-4.3.1-install/lib/gcc/i686-pc-linux-gnu/4.3.1/../../../../include/c++/4.3.1/tr1_impl/boost_shared_ptr.h:509: instantiated from ‘std::__shared_ptr<_Tp, _Lp>::__shared_ptr(std::_Sp_make_shared_tag, _Alloc, _Args&& ...) [with _Alloc = std::allocator<A>, _Args = , _Tp = A, __gnu_cxx::_Lock_policy _Lp = _S_mutex]’ /home/frey/work/gcc-4.3.1-install/lib/gcc/i686-pc-linux-gnu/4.3.1/../../../../include/c++/4.3.1/tr1_impl/boost_shared_ptr.h:926: instantiated from ‘std::shared_ptr<_Tp>::shared_ptr(std::_Sp_make_shared_tag, _Alloc, _Args&& ...) [with _Alloc = std::allocator<A>, _Args = , _Tp = A]’ /home/frey/work/gcc-4.3.1-install/lib/gcc/i686-pc-linux-gnu/4.3.1/../../../../include/c++/4.3.1/tr1_impl/boost_shared_ptr.h:1104: instantiated from ‘std::shared_ptr<_Tp> std::allocate_shared(_Alloc, _Args&& ...) [with _Tp = A, _Alloc = std::allocator<A>, _Args = ]’ /home/frey/work/gcc-4.3.1-install/lib/gcc/i686-pc-linux-gnu/4.3.1/../../../../include/c++/4.3.1/tr1_impl/boost_shared_ptr.h:1119: instantiated from ‘std::shared_ptr<_Tp> std::make_shared(_Args&& ...) [with _Tp = A, _Args = ]’ t.cc:9: instantiated from here t.cc:6: error: ‘A::A()’ is private /home/frey/work/gcc-4.3.1-install/lib/gcc/i686-pc-linux-gnu/4.3.1/../../../../include/c++/4.3.1/bits/boost_sp_shared_count.h:177: error: within this context frey@fiasko:~/work/test/make_shared_2$ Is this a bug in GCC's implementation of make_shared and I should file a bug report for it? Is it intended to standardize that the above code should work? Regards, Daniel BTW: I just filed another bug for GCC's make_shared, <http://gcc.gnu.org/bugzilla/show_bug.cgi?id=36949>, but there is IMHO nothing unclear about that... :)
On Mon, 2008-07-28 at 10:12 +0200, Daniel Frey wrote:
On Sun, 2008-07-27 at 18:25 +0300, Peter Dimov wrote:
You should be able to make it work by making the appropriate make_shared overload a friend:
template< class T, class A1, class A2 > friend boost::shared_ptr< T > boost::make_shared( A1 const & a1, A2 const & a2 );
Good idea, thanks. It would also render create() useless, since the user could call make_shared directly.
Just an idea when thinking of all the pain involved in allowing shared_from_this during the construction phase with the approach we already discussed earlier: Would it be sensible to guarantee that if my class is derived from enable_shared_from_this that I could call shared_from_this during the construction phase iff it is constructed by make_shared? Would it be possible to implement that behaviour? By making the ctors private and make_shared a friend, the API would even disallow any other way to construct objects, you can not write shared_ptr<X>( new X( .. ) ) by accident, so it would IMHO be safe. Regards, Daniel
-----BEGIN PGP SIGNED MESSAGE----- Hash: SHA1 On Monday 28 July 2008 04:12 am, Daniel Frey wrote:
On Sun, 2008-07-27 at 18:25 +0300, Peter Dimov wrote:
You should be able to make it work by making the appropriate make_shared overload a friend:
template< class T, class A1, class A2 > friend boost::shared_ptr< T > boost::make_shared( A1 const & a1, A2 const & a2 );
Good idea, thanks. It would also render create() useless, since the user could call make_shared directly. Sadly, it doesn't work for me. I've tried it with GCC 4.3:
#include <memory>
class A { private: A() {}
public: static std::shared_ptr< A > create() { return std::make_shared< A >(); }
template< typename... Args > friend std::shared_ptr< A > std::make_shared( Args... );
I don't think the zero-argument call to make_shared resolves to the variadic template, see the make_shared.hpp file.
};
int main() { A::create(); }
-----BEGIN PGP SIGNATURE----- Version: GnuPG v1.4.6 (GNU/Linux) iD8DBQFIjcEj5vihyNWuA4URAmrmAJ0R7ZTjR0OmdWSJqKjhYkTrt/VylACeIYXu 4Ay4ntVFHkC0xnogG1L7Cfs= =Zr3Z -----END PGP SIGNATURE-----
On Mon, 2008-07-28 at 08:52 -0400, Frank Mori Hess wrote:
I don't think the zero-argument call to make_shared resolves to the variadic template, see the make_shared.hpp file.
I've seen that, but: a) I don't understand what the "new T()" vs. "new T" issue is - and there seems to be nothing in the code which explains that. It does use new T(), which should also work for new T( Args... ) if Args is a zero-sized parameter pack, shouldn't it? b) Looking at N2352 and N2691, there should be only one make_shared. Boost vs. GCC already shows that looking at a concrete implementation is not enough. Question is, what is/should be guaranteed by the standard. If nothing like this is guaranteed (probably because it's simply too late?), I would end up with non-portable code. Even then I would be interested in whether or not you, Peter, etc. think if something like that should be at least targeted for TR2... c) I just realized that the friend declaration is not yet correct. I think the correct (and generic) version should read: class A { // private ctors... A(); A( int, double ); // allow creation through make_shared template<typename T, typename... Args> friend std::shared_ptr<T> std::make_shared( Args&&... ); }; One might also need to add a friend declaration for allocate_shared if you intend to allow that, too. d) Besides the use case above, what do you think about using this pattern to allow shared_from_this to be called in the ctor? Is this possible? Would it be useful? Regards, Daniel
-----BEGIN PGP SIGNED MESSAGE----- Hash: SHA1 On Monday 28 July 2008 10:29 am, Daniel Frey wrote:
On Mon, 2008-07-28 at 08:52 -0400, Frank Mori Hess wrote:
I don't think the zero-argument call to make_shared resolves to the variadic template, see the make_shared.hpp file.
I've seen that, but:
a) I don't understand what the "new T()" vs. "new T" issue is - and there seems to be nothing in the code which explains that. It does use new T(), which should also work for new T( Args... ) if Args is a zero-sized parameter pack, shouldn't it?
b) Looking at N2352 and N2691, there should be only one make_shared. Boost vs. GCC already shows that looking at a concrete implementation is not enough. Question is, what is/should be guaranteed by the standard. If nothing like this is guaranteed (probably because it's simply too late?), I would end up with non-portable code. Even then I would be interested in whether or not you, Peter, etc. think if something like that should be at least targeted for TR2...
I'm just saying the friend declaration might need to be simply template< typename A > friend std::shared_ptr< A > std::make_shared(); due to the overload for zero arguments in make_shared.hpp. I should mention I didn't notice before that you were actually using std:: instead of boost:: and I'm not actually familiar with the gcc 4.3 implementations.
d) Besides the use case above, what do you think about using this pattern to allow shared_from_this to be called in the ctor? Is this possible? Would it be useful?
Do you mean something like a more generic version of deconstruct_ptr: http://www.comedi.org/projects/thread_safe_signals/libs/thread_safe_signals/... except it would have the benefits provided by make_shared, and allow a class to force its use via private constructor/friendship? It seems like it could be useful. -----BEGIN PGP SIGNATURE----- Version: GnuPG v1.4.6 (GNU/Linux) iD8DBQFIjhLN5vihyNWuA4URApSzAKDEuIXm4wgkjzN+U/gzyY+mTSOhEACgoiLw szeR3hdmp+4URvtK6v5mZ28= =j4Vs -----END PGP SIGNATURE-----
Daniel Frey:
d) Besides the use case above, what do you think about using this pattern to allow shared_from_this to be called in the ctor?
I think that once #2126 is fixed (I'm waiting for 1.36 to go out of the door), shared_from_this will start working inside of constructors invoked by make_shared, unless we do something to specifically disallow that.
On Mon, 2008-07-28 at 21:52 +0300, Peter Dimov wrote:
Daniel Frey:
d) Besides the use case above, what do you think about using this pattern to allow shared_from_this to be called in the ctor?
I think that once #2126 is fixed (I'm waiting for 1.36 to go out of the door), shared_from_this will start working inside of constructors invoked by make_shared, unless we do something to specifically disallow that.
Sounds good to me. Which leaves the question whether you intend to guarantee that or not (in Boost and/or in C++0x). But maybe I should just wait and see what happens... What about b) from my previous mail? Am I misreading the documentation (N2352/N2691)? Regards, Daniel
Daniel Frey:
What about b) from my previous mail? Am I misreading the documentation (N2352/N2691)?
N2691 is clear that there is only one std::make_shared. boost::make_shared has a zero-argument overload because g++ incorrectly translated new T(args...) to new T in the zero case. I don't know whether this has been fixed.
On Mon, 2008-07-28 at 23:01 +0300, Peter Dimov wrote:
Daniel Frey:
What about b) from my previous mail? Am I misreading the documentation (N2352/N2691)?
N2691 is clear that there is only one std::make_shared. boost::make_shared has a zero-argument overload because g++ incorrectly translated new T(args...) to new T in the zero case. I don't know whether this has been fixed.
Just to clarify: We are talking about 5.3.4/15, i.e., in case of PODs new T does leave it uninitialized, while new T() default constructs it, right? As a solution in order to fulfill N2691, wouldn't it be easier to fix it inside the function, by simply doing: if( sizeof...( Args ) == 0 ) new( pv ) T(); else new( pv ) T( detail::forward<Args>( args )... ); with an appropriate comment that it's GCC's fault? Regards, Daniel
On Mon, 2008-07-28 at 22:54 +0200, Daniel Frey wrote:
On Mon, 2008-07-28 at 23:01 +0300, Peter Dimov wrote:
Daniel Frey:
What about b) from my previous mail? Am I misreading the documentation (N2352/N2691)?
N2691 is clear that there is only one std::make_shared. boost::make_shared has a zero-argument overload because g++ incorrectly translated new T(args...) to new T in the zero case. I don't know whether this has been fixed.
As a solution in order to fulfill N2691, wouldn't it be easier to fix it inside the function, by simply doing:
if( sizeof...( Args ) == 0 ) new( pv ) T(); else new( pv ) T( detail::forward<Args>( args )... );
Ping?
On Jul 28, 2008, at 2:52 PM, Peter Dimov wrote:
Daniel Frey:
d) Besides the use case above, what do you think about using this pattern to allow shared_from_this to be called in the ctor?
I think that once #2126 is fixed (I'm waiting for 1.36 to go out of the door), shared_from_this will start working inside of constructors invoked by make_shared, unless we do something to specifically disallow that.
Are you either on the 1.36 release team and/or waiting for a 1.36 version of a dependency library? If neither, then what are you waiting for? If the trunk had to be frozen for a release run-up, then there would be no reason to have a release branch in the first place. Also, there are already 1.37 changes already in the trunk, so it can't be arbitrarily raided for 1.36 code. [Hmm, looking at <http://svn.boost.org/trac/boost/ticket/2126>, it seems that the reporter wants a fix to be part of the 1.36 release, not after. If that's not going to be done, then you should adjust the Milestone and Version fields.] -- Daryle Walker Mac, Internet, and Video Game Junkie darylew AT hotmail DOT com
Daniel Frey wrote:
On Sun, 2008-07-27 at 18:25 +0300, Peter Dimov wrote:
You should be able to make it work by making the appropriate make_shared overload a friend:
template< class T, class A1, class A2 > friend boost::shared_ptr< T > boost::make_shared( A1 const & a1, A2 const & a2 );
Good idea, thanks. It would also render create() useless, since the user could call make_shared directly. Sadly, it doesn't work for me. I've tried it with GCC 4.3: [...] Is this a bug in GCC's implementation of make_shared and I should file a bug report for it? Is it intended to standardize that the above code should work?
It's not a bug. The draft standard says nothing about where the instance will be created. I would expect most implementations to do it in some private auxiliary function. -- Daniel Wallin BoostPro Computing http://www.boostpro.com
participants (7)
-
Alp Mestan -
Daniel Frey -
Daniel Wallin -
Daryle Walker -
Frank Mori Hess -
Peter Dimov -
vicente.botet