casting a shared_ptr<BASE> to a shared_ptr<Derived>

[sciwizeh]

Hello all,
I have not tested the code I'm posting here to see if it compiles, but I have an issue in a large codebase and I believe that this should be the simplest program to demonstrate the issue;

class base {
public:
    shared_ptr<base> clone()=0;
};
typedef shared_ptr<base> baseptr;
class derived : public base {
public:
    baseptr clone(){
        return baseptr(new derived);//the equiv of this compiles in my codebase,
        //so it should work here
    }
};
typedef shared_ptr<derived> derivedptr;
typedef vector<derivedptr> derivedList;

int main(){
    derivedptr der(new derived);//equivelent comiles
    derivedList list;//fine
    list.push_back(der);//works

    baseptr bas = der->clone();//works

    derivedptr der2 = der->clone();//compiler error 
                                    //error C2440: 'initializing' : cannot convert from 
                                    //'std::tr1::shared_ptr<_Ty>' to 
                                    //'std::tr1::shared_ptr<_Ty>'    
    derivedptr der3 = (shared_ptr<derivedptr>) der->clone();//same as last one but with 'type cast'



    list.push_back(der->clone()); //comiler error 
                                    //error C2664: 'void std::vector<_Ty>::push_back(_Ty &&)' : 
                                    //cannot convert parameter 1 from 'std::tr1::shared_ptr<_Ty>'
                                    //to 'parse::AssemblyPtr[READ DERIVEDPTR] &&' 


}

using std::shared_ptr and std::vector.

So my question is how do I convert from a shared_ptr<base> to shared_ptr<derived>?

[mike_2000_17]

There are casting operators for shared_ptr called static_pointer_cast and dynamic_pointer_cast. In other words, if you have this code for raw pointers:

base* pb;
derived* pd = static_cast< derived* >(pb);
derived* pd = dynamic_cast< derived* >(pb);

then, the equivalent with shared_ptr is the following:

shared_ptr< base > pb;
shared_ptr< derived > pd = static_pointer_cast< derived >(pb);
shared_ptr< derived > pd = dynamic_pointer_cast< derived >(pb);

The difference between static_cast and dynamic_cast is that dynamic_cast will to a run-time type check to make sure that the base-class pointer actually does indeed point to an instance of the derived class. In other words, if you can make sure, by other means, that the pointer does indeed point to an instance of the derived class, then you can use static_pointer_cast, but otherwise, it is safer to use dynamic_pointer_cast.

BTW, in your example, your base class should have a virtual destructor, otherwise the destruction is not going to be done correctly. You need to have:

class base {
public:
    virtual shared_ptr<base> clone() = 0;  // your clone() function should be virtual too.

    virtual ~base() { };  // virtual destructor.
};

[sciwizeh]

Thanks for the information, my codebase already uses the virtual destructor, I forgot it for my sample. Changing my code to use static/dynamic_pointer_cast<derived> is giving me another comiler error, this time in <memory>, I get the same C2440 but with a 'functino style cast', and a new one error C2228: left of '.swap' must have class/struct/union these occur on the same line 1565, of memory VS2010 includes, it's in the operator= or ptr_base.

    template<class _Ty2>
        _Myt& operator=(shared_ptr<_Ty2>&& _Right)
        {   // construct shared_ptr object that takes resource from _Right
        shared_ptr(_STD move(_Right)).swap(*this);//THIS LINE FAILS TO COMPILE
        return (*this);
        }

I am quite sure the two classes are sub/super class related, I tired both static and dynamic (even though for this instance static would work as I know for sure it is class derived) and they both fail to comile on the same line (possibly different function but same textual line, and same errors).

OK I think I found the issue, but I either need to ask something else or start a new question.
My code looks more like this (when posting the original question I forgot derived isn't concrete either):

class base {
public:
    virtual shared_ptr<base> clone()=0;
    virtual ~base(){}
};
class derivedAbstract : public base{
    virtual void newAbstractMethod()=0;
};

//NO CONCRETE CLASSES IN REAL CODEBASE (yet)

void othermethod(vector<shared_ptr<derivedAbstract>> vec){
    vector<shared_ptr<derivedAbstract> newVec;
    newVec.push_back(vec[0]->clone());//original error cause
    newVec.push_back(dynamic_pointer_cast<derivedAbstract>(vec[0]->clone()));//new error cause

}

This last example is much more realistic to my actual code, is this not allowed? Is there any way I can do this? Does the non-concrete-ness of derived make any real difference at all?

Should I post a new question for this case because the original question was answered?

[mike_2000_17]

This last example is much more realistic to my actual code, is this not allowed?

This is certainly allowed, at least, it should be in a bug-free standard-compliant C++11-supporting compiler (VS2010 is neither of these things). The version with the dynamic_pointer_cast compiles fine on GCC 4.5.3, GCC 4.6.3 and GCC 4.7.3 (and probably everything in between and after, but I just have those three on my computer).

There is no reason for that dynamic_pointer_cast to fail to compile, or the push_back to the vector for that matter. Whatever is causing this error is not your responsibility, it is that of Microsoft developers. Either file a bug report or see if they fixed it in VS2012 (november update).

Other than that, you might have to use a workaround for the dynamic_pointer_cast function, it's not like it's very difficult to implement anyways:

template <typename T, typename U>
std::shared_ptr<T> dynamic_pointer_cast(const std::shared_ptr<U>& rhs) {
  T* tmp_ptr = dynamic_cast< T* >(rhs.get());
  if(tmp_ptr)
    return std::shared_ptr<T>(rhs, tmp_ptr);
  else
    return std::shared_ptr<T>();
};

template <typename T, typename U>
std::shared_ptr<T> static_pointer_cast(const std::shared_ptr<U>& rhs) {
  return std::shared_ptr<T>(rhs, static_cast< T* >(rhs.get()));
};

The error you pointed out is weird because it seems to come from the move-assignment operator of the VS2010 implementation of std::shared_ptr, however, that move-assignment operator is never used during the whole std::dynamic_pointer_cast implementation in VS2010's <memory> header. So, I guess it might occur within the vector push_back function, or something like that. Anyways, I have learned by experience not to waste time crying about all the annoying quirks and bugs of the Microsoft compilers and just proceed to write workarounds (as with the above replacements for the casting operators).

Is there any way I can do this?

Unless the bug is actually in the vector push_back function, the workaround of simply implementing your own casting operators should be sufficient to solve the problem.

Does the non-concrete-ness of derived make any real difference at all?

No. It definitely should not make a difference.

Should I post a new question for this case because the original question was answered?

No, I don't think so, in this forum, we like continuing discussions, and that's what this is.

[vijayan121]

There is no reason for that dynamic_pointer_cast to fail to compile, or the push_back to the vector for that matter. Whatever is causing this error is not your responsibility, it is that of Microsoft developers. Either file a bug report or see if they fixed it in VS2012 (november update).

Please don't file a bug report; std::dynamic_pointer_cast<> works as specified by the IS in VS 2010.
(IIRC correctly, it also worked correctly in VS 2008)

This last example is much more realistic to my actual code, is this not allowed? Is there any way I can do this? Does the non-concrete-ness of derived make any real difference at all?

It is allowed; there is nothing special that you have to do to make it work; and, no, non-concrete-ness of the class does not make any difference.

This code compiles (and runs) cleanly with VS 2011:

#include <memory> 
#include <iostream> 
#include <vector>

struct base { virtual ~base() {} virtual std::shared_ptr<base> clone() const = 0 ; } ;

struct derived_abstract : base {} ;

struct more_derived : derived_abstract
{ 
    virtual std::shared_ptr<base> clone() const 
    { 
        return std::make_shared<more_derived>( *this ) ; 
    }
};

int main() 
{ 
    std::shared_ptr<base> pb = std::make_shared<more_derived>() ; 
    std::shared_ptr<base> pb2 = pb->clone() ; 

    std::shared_ptr<derived_abstract> pd =
                          std::dynamic_pointer_cast<derived_abstract>(pb) ; 
    std::cout << pd.get() << '\n' ;

    std::shared_ptr<derived_abstract> pd2 = 
                          std::dynamic_pointer_cast<derived_abstract>(pb2) ; 
    std::cout << pd2.get() << '\n' ;

    std::vector< std::shared_ptr<derived_abstract> > seq ;
    seq.push_back( std::dynamic_pointer_cast<derived_abstract>( pb->clone() ) ) ; 
    seq.push_back( std::dynamic_pointer_cast<derived_abstract>( seq[0]->clone() ) ) ; 
    std::cout << seq.back().get() << '\n' ;
} 

The error seems to be in your code. You need to fix the error rather than try to sidestep it by kludging work-arounds. Strive towards having a simple, clean, transparent code base - it pays huge dividends over time.

From what has been posted so far, the error seems to arise from clone(). Could you post the full definition of derivedAbstract along with its override of clone() if it has one?

[sciwizeh]

Thanks for the responses, I find it more likely that it is my fault than the compilers.

From what has been posted so far, the error seems to arise from clone(). Could you post the full definition of derivedAbstract along with its override of clone() if it has one?

It does not have one as it can't instanciate a version of itself to return. This should be all the relevant declarations, if I missed anything let me know.

namespace parse{
    class Cloneable {
    public:
        virtual std::shared_ptr<Cloneable> clone()=0;
        virtual std::string toString() const {return "Coneable";};
        virtual ~Cloneable(){};
    };
    typedef std::shared_ptr<Cloneable> CloneablePtr;

    class Assembly : public Cloneable {
    protected:
        StackPtr stack;
        CloneablePtr target;
        int index;
    public:
        Assembly(): stack(new Stack),index(0),target( NULL){}
        Assembly(const Assembly& c) : index(c.index), target(c.target->clone()){stack=c.stack->clone();}
        virtual std::string consumed(std::string delim)=0;
        virtual std::string defaultDelimiter()=0;
        int elementsConsumed(){return index;}
        int elementsRemaining(){return length()-elementsConsumed();}
        StackPtr getStack(){return stack;}
        CloneablePtr getTarget(){return target;}
        bool hasMoreElements(){return elementsConsumed()<length();}
        virtual int length()=0;
        virtual CloneablePtr peek()=0;
        CloneablePtr pop(){return stack->pop();}
        void push(CloneablePtr c){ stack->push(c);}
        virtual std::string remainder(std::string s)=0;
        void setTarget(CloneablePtr t) {target=t;}
        bool stackIsEmpty(){return stack->empty();}
        std::string toString();
        void unget(int n);
    };
    typedef std::shared_ptr<Assembly> AssemblyPtr;
    typedef std::vector<AssemblyPtr> AssemblyList;
    class Assembler{
    public:
        virtual void workOn(std::shared_ptr<Assembly> a)=0;
        static std::vector<CloneablePtr> elementsAbove(std::shared_ptr<Assembly> a, CloneablePtr fence);
    };

    class ParseVisitor;//no definition yet, but nothing calls anything on one yet

    class Parser {
    protected:
        std::string name;
        std::shared_ptr<Assembler> assembler;
        virtual std::vector<std::string> randomExpansion(int maxDepth, int depth)=0;
        virtual std::string unvisitedString(std::vector<Parser*>& visited)=0;
        std::string toString(std::vector<Parser*> &visited);
    public:
        Parser() : name(""),assembler(NULL) {}
        Parser(std::string n) : name(n), assembler(NULL) {}
        void accept(ParseVisitor* pv);
        virtual void accept(ParseVisitor* pv, std::vector<Parser*> visited);
        template<typename T>static void add(std::vector<T> &v1, std::vector<T> &v2);
        AssemblyPtr best(AssemblyList v);
        AssemblyPtr bestMatch(AssemblyPtr a);
        AssemblyPtr completeMatch(AssemblyPtr a);
        static AssemblyList elementClone(AssemblyList v);
        std::string getName(){return name;}
        virtual AssemblyList match(AssemblyList in)=0;
        AssemblyList matchAndAssemble(AssemblyList in);
        std::string randomInput(int maxDepth, std::string seperator);
        Parser& setAssembler(std::shared_ptr<Assembler> a){assembler=a;return *this;}
        std::string toString();
    };
    typedef std::shared_ptr<Parser> ParserPtr;
    typedef std::vector<ParserPtr> ParserList;

}

the .cpp file

#include "parseparse.h"
#include <sstream>
#include <algorithm>
using namespace std;
using namespace parse;

void Assembly::unget(int n){
    index-=n;
    if(index<0) index=0;
}

string Assembly::toString(){
    string delim = defaultDelimiter();
    ostringstream oss;
    oss<< stack->toString() <<consumed(delim)<<"^"<<remainder(delim);
}

vector<shared_ptr<Cloneable>> elementsAbove(shared_ptr<Assembly> a, shared_ptr<Cloneable> fence){
    vector<shared_ptr<Cloneable>> items;
    while(!a->stackIsEmpty()){
        shared_ptr<Cloneable> top=a->pop();
        if(top==fence){
            break;
        }
        items.push_back(top);
    }
    return items;
}


string Parser::toString(std::vector<Parser*>& visited){
    if(!(name=="")) return name;
    else if(find(visited.begin(),visited.end(),this)!=visited.end())
        return "...";
    else {
        visited.push_back(this);
        return unvisitedString(visited);
    }
}

string Parser::toString(){
    vector<Parser*> vec;
    return toString(vec);
}

string Parser::randomInput(int maxDepth, string seperator){
    ostringstream oss;
    vector<string> vec = randomExpansion(maxDepth,0);
    vector<string>::iterator it = vec.begin();
    bool first=true;
    for(;it!=vec.end();it++){
        if(!first){
            oss<<seperator;
        }
        oss<<*it;
        first=false;
    }
}

AssemblyList Parser::matchAndAssemble(AssemblyList in){
    AssemblyList out = match(in);
    if(assembler!=NULL){
        AssemblyList::iterator it = out.begin();
        for(;it!=out.end();it++){
            assembler->workOn(*it);
        }
    }
    return out;
}

AssemblyList Parser::elementClone(AssemblyList v){
    AssemblyList out;
    AssemblyList::iterator it = v.begin();
    for(;it!=v.end();it++){
        out.push_back(static_pointer_cast<Assembly,Cloneable>(*it));//this is where the issue is
    }
    return out;
}

AssemblyPtr Parser::completeMatch(AssemblyPtr a){
    AssemblyPtr best = bestMatch(a);
    if(best!=NULL && !best->hasMoreElements())
        return best;
    return NULL;
}

AssemblyPtr Parser::bestMatch(AssemblyPtr a){
    AssemblyList in;
    in.push_back(a);
    AssemblyList out = matchAndAssemble(in);
    return best(out);
}

AssemblyPtr Parser::best(AssemblyList v){
    AssemblyPtr best = NULL;
    AssemblyList::iterator it = v.begin();
    for(;it!=v.end();it++){
        AssemblyPtr a = *it;
        if(!a->hasMoreElements()){
            return a;
        }
        if(best==NULL) {
            best=a;
        } else {
            if(a->elementsConsumed() > best->elementsConsumed()){
                best=a;
            }
        }
    }
}

template<typename T> void Parser::add(vector<T> & v1,vector<T> & v2){
    vector<T>::iterator it=v2.begin();
    for(;it!=v2.end();it++){
        v1.push_back(*it);
    }
}

without the static_pointer_cast the code fails to compile on line 75, with it it's in the previously mentioned <memory> file this is how my code appears in my project, if there are references to classes I haven't listed let me know and I'll post those too if you think they're relevant.

[mike_2000_17]

Shouldn't that line be the following instead:

out.push_back( std::static_pointer_cast<Assembly>((*it)->clone()) );

Otherwise, it doesn't make much sense.

Also, you could define a free function for cloning that will make that syntax easier. For example, this:

template <typename CloneableType>
std::shared_ptr< CloneableType > clone(const std::shared_ptr< CloneableType >& p) {
  return std::static_pointer_cast< CloneableType >(p->clone());
};

which will make the above line much simpler (by deduction of the template argument):

out.push_back( clone(*it) );

[sciwizeh]

yes, yes it should, I chaned it to use a temporary variable while trying to get it to compile and forgot to change it back, but even with the clone call it still will not compile.

[mike_2000_17]

Try splitting it up into multiple lines to see exactly where the error comes from. For instance, try this:

for(;it!=v.end();it++) {
    std::shared_ptr< Cloneable > pc = (*it)->clone();
    std::shared_ptr< Assembly > pa = std::static_pointer_cast< Assembly >(std::move(pc));
    out.push_back(std::move(pa));
}

This should tell you what step actually causes the error, the above code should, in essence, be equivalent to your original code. Also, since the error seems to come from some move operation (since the error in the <memory> header is within a move function), try to remove the std::move() calls from the above code and that will help you determine if the error comes from the move involved in the pointer-cast or when pushing the pointer to the vector.

So, report back when you tried the above code, and what happens if you remove one or the other std::move() calls.

[sciwizeh]

none of the lines of code in any of my files cause the error, the compiler failes in <memory>. It doesn't seem to matter what way I code the function, it's in the assignment operator of shared_ptr. And I can't tell which line of my code is causing the error because the compiler error isn't in my code.

[mike_2000_17]

The compiler error in <memory> is just where the error ends up popping up, it is not the root cause of the error. The compiler error should have a number of additional lines telling you where the error originates from. Usually, in comes in the form of one line saying "error in function operator= ..." plus a number of lines saying something like "instantiated from '...'", "instantiated from '...'", and so on. Some of these lines should point to the specific lines in your code from which the error originates.

If not, simply comment out the lines of code, re-compile, uncomment, re-compile, and so on until you pin-point what line causes the error.

[sciwizeh]

FOUND IT! turns out the error was caused by line 17 of the declarations:
Assembly(const Assembly& c) : index(c.index), target(c.target->clone()){stack=c.stack->clone();} I needed to cast the clones to the right type.

[vijayan121]

I find it more likely that it is my fault than the compilers.

Yes. Always start with the axiom 'The problem is in my code; not in the compiler/library'. And stick with it till you reach reductio ad absurdum. Get into the habit of checking the documentation; Visual Studo 2010 help or msdn would have lead you to this page: http://msdn.microsoft.com/en-us/library/bb982967(v=vs.100).aspx
Test with the example code there, and if it works, you would know for sure that the problem is not with the library.
.
.

FOUND IT! turns out the error was caused by line 17 of the declarations

Get into the habit of: write a few lines of code, just enough so that it is compilable, compile and test it, write some more code, compile, test etc. The earlier an error is discovered, the easier is it to identify the cause and rectify it. The smaller the scope in which an error is discovered, the easier is it to identify the cause and rectify it. Use explicit instantiation of templates during the development phase; this will ensure that all the template code is compilable.

.
.
Prefer std::dynamic_pointer_cast<> over std::static_pointer_cast<> for polymorphic types. And where required, check the result of the cast. If, and only if, there is a measurable performance problem with this, should we resort to std::static_pointer_cast<>, and that, after the code is tested, and debugged.

[sciwizeh]

Get into the habit of: write a few lines of code, just enough so that it is compilable, compile and test it, write some more code, compile, test etc. The earlier an error is discovered, the easier is it to identify the cause and rectify it. The smaller the scope in which an error is discovered, the easier is it to identify the cause and rectify it

The strange thing is, I did do just that, the line in question compiled fine, until I tried to copy a type that had an instance variable of one.

As for dynamic/static issue for this case a clone of a type should always return an instance of the same type, so a static cast should be safe, correct?

[vijayan121]

The strange thing is, I did do just that, the line in question compiled fine, until I tried to copy a type that had an instance variable of one.

There are two different phases of compilation when templates are involved. In somewhat simplistic terms,

The first phase is when a template definition is seen, and it is initially parsed. This takes place before the template is instantiated; the compiler only looks up "non-dependent" names during this phase. A name is a "non-dependent" name if the results of name lookup do not depend on any template parameters; the name referes to the same entity in all template instantiations.

The second phase is when when the template is instantiated; in phase two the compiler looks up "dependent" names. Obviously, the result of this lookup may vary from one template instantiation to another.

For instance:

template < typename T > struct A
{
    void bar( T& first, int second )
    {
        first[0] = 0 ; // first is a "dependent" name; looked up during second phase
        // this is not an error if the template function is not instantiated
        // it may or may not be an error for a particular template instantiation;
        // depends on what the type T is

        second = 0 ; // 'second' is a "non-dependent" name; looked up during first phase
        // second[0] = 0 ; // *** error during first phase look up
    }
};

Instantiate it, and:

template struct A<std::string> ; // fine; first is a reference to std::string

template struct A<int> ; // *** error; first is a reference to int

While testing template code, it is a good idea to explicitly instantiate templates.

.
.

As for dynamic/static issue for this case a clone of a type should always return an instance of the same type, so a static cast should be safe, correct?

Yes, and no.

No, it it is written this way:

    struct base
    {
        virtual ~base() {}
        virtual base* clone() const = 0 ;
        // ...
    };

    template < typename T > void this_code_is_broken( T* p )
    {
        base* b = static_cast< base* > ( p->clone() ) ;
        // use b
    }

    struct not_derived
    {
        void* clone() { return this ; }
    };

    void foo( not_derived* nd )
    {
       this_code_is_broken(nd) ; // leads to undefined behaviour
    }

.
Yes, if it is written this way:

#include <type_traits>

template < typename T > 
typename std::enable_if< std::is_base_of<base,T>::value, void >::type
this_code_is_not_broken( T* p )
{
        base* temp = static_cast<base*>(p) ;
        base* b = temp->clone() ;
        // use b
}

Code using a dynamic_cast is simpler and clearer; just use it as a default for all run-time polymorphic types.

[sciwizeh]

There are two different phases of compilation when templates are involved. In somewhat simplistic terms,

The first phase is when a template definition is seen, and it is initially parsed. This takes place before the template is instantiated; the compiler only looks up "non-dependent" names during this phase. A name is a "non-dependent" name if the results of name lookup do not depend on any template parameters; the name referes to the same entity in all template instantiations.

The second phase is when when the template is instantiated; in phase two the compiler looks up "dependent" names. Obviously, the result of this lookup may vary from one template instantiation to another.

There are two different phases of compilation when templates are involved. In somewhat simplistic terms,

The first phase is when a template definition is seen, and it is initially parsed. This takes place before the template is instantiated; the compiler only looks up "non-dependent" names during this phase. A name is a "non-dependent" name if the results of name lookup do not depend on any template parameters; the name referes to the same entity in all template instantiations.

The second phase is when when the template is instantiated; in phase two the compiler looks up "dependent" names. Obviously, the result of this lookup may vary from one template instantiation to another.

I did not know that, good to know.