Strange error trying to do a shared_ptr swap()
Asked Answered
M

2

5

I'm a relative C++ newbie trying to convert an existing project from raw pointers, with a convoluted memory-management protocol, over to using C++11's shared_ptr. Overall it is going very smoothly, and I think I understand how shared_ptr works in terms of move semantics, rvalue references, etc. Good stuff.

However, I have hit a weird error that I don't understand and don't know how to fix. A little background first. I have a class hierarchy rooted in an abstract base class named EidosValue, and a class named EidosValue_Int_vector is (indirectly) a concrete subclass of that:

class EidosValue
class EidosValue_Int : public EidosValue
class EidosValue_Int_vector : public EidosValue_Int

My code generally traffics in EidosValue, but sometimes, especially when creating a new value, I need to handle a specific subclass.

I have done typedefs for the shared_ptrs for these classes, so I have:

typedef std::shared_ptr<EidosValue> EidosValue_SP;
typedef std::shared_ptr<EidosValue_Int_vector> EidosValue_Int_vector_SP;

among others. OK, so now to the core of the problem. I have a function that returns an EidosValue_SP that it creates. Depending on the logic within the function, it might create one of several different concrete subclasses of EidosValue. So I do something like this:

EidosValue_SP MyClass::MyMethod(...)
{
    EidosValue_SP result;

    if (...)
    {
        EidosValue_Int_vector_SP int_result_SP = make_shared<EidosValue_Int_vector>();
        ... do subclass-specific stuff with int_result_SP...
        result.swap(int_result_SP);
    }
    else (...)
    {
        ...similar logic for other subclasses...
    }

    ...other shared logic...
    return result;
}

The problem is with the swap() call. I get an error: "Non-const lvalue reference to type 'shared_ptr<EidosValue>' cannot bind to a value of unrelated type 'shared_ptr<EidosValue_Int_vector>'". This is puzzling since EidosValue_Int_vector is not an "unrelated type", it's a public subclass of EidosValue, and the code here knows it. If I type result = make_shared<EidosValue_Int_vector>(); the compiler has no problem whatsoever with that, so it clearly knows that the types are related and compatible. It just doesn't like it in the context of swap() for some reason. In other places in my project I have been able to simply do a return int_result_SP;, with a declared return type of EidosValue_SP, and that has worked fine – the compiler is happy to consider an EidosValue_Int_vector_SP to be an EidosValue_SP in that context – but I can't do that here because of the shared logic at the bottom of the function.

I am somewhat constrained in my implementation here because this code is a bottleneck and needs to run fast (and yes, I know that from actually instrumenting the code, and yes, it really does matter). So it is essential to use make_shared to avoid a double allocation, and I would also strongly prefer to avoid a refcount increment/decrement when I hand the pointer from int_result_SP to result; I don't want there to be a moment in time when there are two shared_ptrs pointing to the new instance. So swap() seems like the obvious way to go; but I'm blocked by this compiler error. Why is it happening and how can I fix it? Thanks!

ADDENDUM:

Oh, pondering this further I bet I know why the error is happening. swap() has no objection to putting the EidosValue_Int_vector into the EidosValue_SP, but it does have a problem with putting the EidosValue into the EidosValue_Int_vector_SP; in that direction the types are not compatible. I hadn't thought about it that way since result has no value (i.e. has nullptr, I guess) in it; but of course swap() doesn't know that. OK, so if that is the problem, the question remains: how can I effect the transfer while keeping the code fast – not doing a refcount inc/dec and not moving away from using make_shared? Now that I understand the problem (I think) it seems likely that there is just some API or trick that I have overlooked...

Markland answered 23/9, 2015 at 16:42 Comment(0)
D
4

This is exactly what std::move is for. Use:

    result = std::move (int_result_SP);
Drysalt answered 23/9, 2015 at 17:23 Comment(0)
K
5

You can't swap because, although EidosValue_Int_vector is a EidosValue the converse isn't true.

Klink answered 23/9, 2015 at 16:48 Comment(8)
Yep, I figured that out just after I clicked post. See the addendum I just put up. The question remains how to make my code work.Markland
just result = int_result_SP;. No swap is neededTrinitrotoluene
"while keeping the code fast" - what makes you think the assignment is making your code not fast?Stamford
@qehgt, result = int_result_SP would cause a refcount increment/decrement, would it not? After the assignment, both result and int_result_SP would point to the object, and that would remain true until int_result_SP went out of scope and the refcount decremented again. Am I mistaken in how that works?Markland
@bhaller, you can assume that this increments/decrements are very cheap operations vs dynamic allocation of your object in heap.Trinitrotoluene
@qehgt, sure, but that could be like saying that elephants are very small compared to blue whales. :-> David Schwartz seems to hit the nail on the head with his answer, which avoids the overhead of the increment/decrement entirely. (Although it would be interesting to profile both and compare; perhaps once I've got my code running I will do that.)Markland
@bhaller, it's literally just a few CPU instructions if optimizations are enabled.Trinitrotoluene
Unless the compiler decides not to inline the operator= function call, in which case it's a function call. In any case, this code is hot enough that a few CPU instructions matter. Why would you use a solution that causes a refcount inc/dec if there is an equally good solution that avoids that?Markland
D
4

This is exactly what std::move is for. Use:

    result = std::move (int_result_SP);
Drysalt answered 23/9, 2015 at 17:23 Comment(0)

© 2022 - 2024 — McMap. All rights reserved.