Putting SharedPtr<> child classes in their own header
- Mikachu
- Gnoll
- Posts: 603
- Joined: Thu Jul 28, 2005 4:11 pm
- Location: Nice, France
- x 35
Putting SharedPtr<> child classes in their own header
Hi,
In the process of pimpl'ing my library, I hit a wall with MeshPtr :
- I can't forward declare it, as it is passed by value
- If I want the complete type, I have to include "OgreMesh.h", which pulls many headers
It looks like a missed opportunity to reduce header dependencies
Would it be possible to put MeshPtr (and all other SharedPtr specialisations) in a separate header (which wouldn't depend on OgreMesh.h itself)?
In the process of pimpl'ing my library, I hit a wall with MeshPtr :
- I can't forward declare it, as it is passed by value
- If I want the complete type, I have to include "OgreMesh.h", which pulls many headers
It looks like a missed opportunity to reduce header dependencies
Would it be possible to put MeshPtr (and all other SharedPtr specialisations) in a separate header (which wouldn't depend on OgreMesh.h itself)?
OgreProcedural - Procedural Geometry for Ogre3D
- Klaim
- Old One
- Posts: 2565
- Joined: Sun Sep 11, 2005 1:04 am
- Location: Paris, France
- x 56
- Contact:
Re: Putting SharedPtr<> child classes in their own header
It should even be possible to declare all shared pointer types into one header using only forward declarations of necessary types without ever loading their headers. It's all pointers internally after all. I do this in my projects.
- Mikachu
- Gnoll
- Posts: 603
- Joined: Thu Jul 28, 2005 4:11 pm
- Location: Nice, France
- x 35
Re: Putting SharedPtr<> child classes in their own header
Ok, I tried that (only for MeshPtr atm), but compile fails on the sharedptr destructor...
It looks like Ogre's SharedPtr implementation requires knowing the complete type of the class it points to...
Does someone know if there's a way around that?
The line where compilation fails :
Code: Select all
case SPFM_DELETE_T:
OGRE_DELETE_T(pRep, T, MEMCATEGORY_GENERAL);
break;
Does someone know if there's a way around that?
OgreProcedural - Procedural Geometry for Ogre3D
- Klaim
- Old One
- Posts: 2565
- Joined: Sun Sep 11, 2005 1:04 am
- Location: Paris, France
- x 56
- Contact:
Re: Putting SharedPtr<> child classes in their own header
Wow I forgot about MeshPtr specializatoin... WOW I DIDn't KNEW THERE WAS VIRTUAL CALLS THERE !!! D:
From your description it looks like the deleter requires the type but it shouldn't.
Ah yes, maybe check if the headers types using the SharedPtr have a destructor into the header? If they do, move them in the cpp file instead. Then don't forget to include the necessary headers in the cpp file.
From your description it looks like the deleter requires the type but it shouldn't.
Ah yes, maybe check if the headers types using the SharedPtr have a destructor into the header? If they do, move them in the cpp file instead. Then don't forget to include the necessary headers in the cpp file.
- Mikachu
- Gnoll
- Posts: 603
- Joined: Thu Jul 28, 2005 4:11 pm
- Location: Nice, France
- x 35
Re: Putting SharedPtr<> child classes in their own header
The issue is that the problematic code comes from SharedPtr.h, and is inside a template, so it can't be put into a cpp...Klaim wrote:From your description it looks like the deleter requires the type but it shouldn't.
Ah yes, maybe check if the headers types using the SharedPtr have a destructor into the header? If they do, move them in the cpp file instead. Then don't forget to include the necessary headers in the cpp file.
This code is only triggered in the case where sharedPtr references a primitive type or external type, which is not the case of MeshPtr... so it would be possible, but not very elegant, to split SharedPtr in half...
I'm not sure it would be worth the cost... although I still believe it's pretty weird to have to include full type just to declare something with pointer semantics!
OgreProcedural - Procedural Geometry for Ogre3D
- Klaim
- Old One
- Posts: 2565
- Joined: Sun Sep 11, 2005 1:04 am
- Location: Paris, France
- x 56
- Contact:
-
- OGRE Expert User
- Posts: 1920
- Joined: Sun Feb 19, 2012 9:24 pm
- Location: Russia
- x 201
Re: Putting SharedPtr<> child classes in their own header
The delete operator absolutely needs to know the type as otherwise it can't call the proper destructor.
I think your best option would be taking advantage of precompiled headers. That will eliminate most overhead related to the need of including OgreMesh.h
I think your best option would be taking advantage of precompiled headers. That will eliminate most overhead related to the need of including OgreMesh.h
- Klaim
- Old One
- Posts: 2565
- Joined: Sun Sep 11, 2005 1:04 am
- Location: Paris, France
- x 56
- Contact:
Re: Putting SharedPtr<> child classes in their own header
It's correct but it don't mean that a smart pointer have to know the complete type it's pointing to in the user's header.bstone wrote:The delete operator absolutely needs to know the type as otherwise it can't call the proper destructor.
You can use a std::shared_ptr or std::unique_ptr this way perfectly legal, I use it often:
Code: Select all
//header
#include <memory>
class K; // not defined
class Foo
{
std::shared_ptr<K> m_k1;
std::unique_ptr<K> m_k2;
public:
// because these following functions are only declared,
// they will not instantiate the constructor and destructor
// of the member smart pointers if you include this header
Foo();
~Foo();
};
/////////////////////////////
// cpp file
#include "foo.hpp"
#include "k.hpp" // now we have the definition of K
Foo::Foo(){} // smart pointer construction instantiated here
Foo::~Foo(){} // smart pointer destruction instantiated here
of construction and destruction is there.
So there is no reason that Ogre::SharedPtr can't do the same.
-
- OGRE Expert User
- Posts: 1920
- Joined: Sun Feb 19, 2012 9:24 pm
- Location: Russia
- x 201
Re: Putting SharedPtr<> child classes in their own header
Yeah, and that's one of the reasons why std::unique_ptr<> is much more complicated than Ogre::SharedPtr.
- Klaim
- Old One
- Posts: 2565
- Joined: Sun Sep 11, 2005 1:04 am
- Location: Paris, France
- x 56
- Contact:
Re: Putting SharedPtr<> child classes in their own header
I don't understand your point: I use this for non-pointers types too without any problem and their code is very simple,bstone wrote:Yeah, and that's one of the reasons why std::unique_ptr<> is much more complicated than Ogre::SharedPtr.
It's only about instantiation of the destructor function of the smart pointer, so this technique should work with Ogre::SharedPtr too.
- Mikachu
- Gnoll
- Posts: 603
- Joined: Thu Jul 28, 2005 4:11 pm
- Location: Nice, France
- x 35
Re: Putting SharedPtr<> child classes in their own header
Here's a post on stackoverflow that explains how std::shared_ptr (and boost::shared_ptr) doesn't need full type : http://stackoverflow.com/questions/5606 ... t-pointers
To sum up, there's a trick that involves storing the deleter when the shared pointer is bound to the raw pointer.
At this point, since it usually occurs just after a "new", the full type is known, thus the desctructor...
Of course, there's a small overhead involved with this operation.
I can try reproducing this behaviour with Ogre's SharedPtr, but my code will probably need some review, I'm no template guru
To sum up, there's a trick that involves storing the deleter when the shared pointer is bound to the raw pointer.
At this point, since it usually occurs just after a "new", the full type is known, thus the desctructor...
Of course, there's a small overhead involved with this operation.
I can try reproducing this behaviour with Ogre's SharedPtr, but my code will probably need some review, I'm no template guru
OgreProcedural - Procedural Geometry for Ogre3D
- Mikachu
- Gnoll
- Posts: 603
- Joined: Thu Jul 28, 2005 4:11 pm
- Location: Nice, France
- x 35
Re: Putting SharedPtr<> child classes in their own header
I managed to change Ogre's SharedPtr implementation so that it doesn't need full type at declaration time...
Instead, full type is only needed at binding time, when full type is already known anyway.
I'll further test it, then will issue a pull request.
It should help reducing inter-header dependency (a static analysis tool would be helpful to tell which header inclusion became useless)
Instead, full type is only needed at binding time, when full type is already known anyway.
I'll further test it, then will issue a pull request.
It should help reducing inter-header dependency (a static analysis tool would be helpful to tell which header inclusion became useless)
OgreProcedural - Procedural Geometry for Ogre3D
- Mikachu
- Gnoll
- Posts: 603
- Joined: Thu Jul 28, 2005 4:11 pm
- Location: Nice, France
- x 35
Re: Putting SharedPtr<> child classes in their own header
Ok, my patch to SharedPtr has been merged into branch v1-9.
Now, I just need to remove un-needed header include...
Does someone have a tool to suggest?
Such a tool should be able to take into account forward definitions, and ignore PCH header...
Now, I just need to remove un-needed header include...
Does someone have a tool to suggest?
Such a tool should be able to take into account forward definitions, and ignore PCH header...
OgreProcedural - Procedural Geometry for Ogre3D
-
- OGRE Expert User
- Posts: 1920
- Joined: Sun Feb 19, 2012 9:24 pm
- Location: Russia
- x 201
Re: Putting SharedPtr<> child classes in their own header
Sorry, couldn't reply any earlier but Mikachu already explained that. And great job Mikachu, thanks for healing Ogre::SharedPtr! Can't advise on the static analysis tool though.Klaim wrote:I don't understand your point: I use this for non-pointers types too without any problem and their code is very simple,bstone wrote:Yeah, and that's one of the reasons why std::unique_ptr<> is much more complicated than Ogre::SharedPtr.
It's only about instantiation of the destructor function of the smart pointer, so this technique should work with Ogre::SharedPtr too.