CRTP copy method warns of potential memory leak

Question

I have an object hierarchy and need to be able to clone objects from the base class. I've followed the typical CRTP pattern, except that I also want to be able to return the child class if copy is called on a child directly. To do that, I've followed the suggestion here: https://stackoverflow.com/a/30252692/1180785

It seems to work fine, but Clang warns me that I have a potential memory leak. I've reduced the code down to this MCVE:

template <typename T>
class CRTP {
protected:
    virtual CRTP<T> *internal_copy(void) const {
        return new T(static_cast<const T&>(*this));
    }

public:
    T *copy(void) const {
        return static_cast<T*>(internal_copy());
    }

    virtual ~CRTP(void) = default;
};

class Impl : public CRTP<Impl> {
};

int main(void) {
    Impl a;
    Impl *b = a.copy();
    delete b;
}

As far as I can tell, there's no possible memory leak there, but running Clang through XCode shows this:

Is there a memory leak here? If not, what's causing the false positive and how can I work around it? (I'd rather not turn off static analysis)

Answer 1

I have found a workaround which makes the analyser happy while still allowing the use of this pattern. Simply reverse the link between copy and internal_copy:

template <typename T>
class CRTP : public Base {
protected:
    virtual CRTP<T> *internal_copy(void) const {
        return copy();
    }

public:
    T *copy(void) const {
        return new T(static_cast<const T&>(*this));
    }
};

This still works in the context of the original suggestion here, because when resolving copy inside CRTP, it will prefer CRTP's override (even though it isn't a virtual method), so there is no infinite loop.

As for why the analyser is happy with this order but unhappy with the original order, I have no idea.

Answer 2

I think the analyzer is getting confused by the static_cast in your copy method. If you simply change it to dynamic_cast, it ceases to flag the line as a memory leak. This leads me to believe that it thinks you are possibly casting a base type instance (CRTP<T>) to its derived type (T), which would of course be invalid when dereferenced. You are obviously not doing this, so this may be a bug in the leak detector, or it may be onto something more subtle than what I'm thinking about right now. It may not be an error now, but if Impl becomes a more complicated type (e.g., one with multiple inheritance), it may become an issue (as will your static_cast in the copy-CTOR invocation).

This implementation (with fewer casts) had zero leaks for me as well:

template <typename T>
class CRTP {
protected:
    virtual T *internal_copy() const {
        return new T(static_cast<const T&>(*this));
    }

public:
    T *copy() const {
        return internal_copy();
    }

    virtual ~CRTP() = default;
};