Are there any problems with this overload of operator new?

Question

I was thinking about some memory pool/allocation stuff I might write so I came up with this operator new overload that I want to use to facilitate reuse of memory. I'm wondering if there are any problems you guys can think of with my implementation (or any other possible ones).

#include <cstddef>

namespace ns {
    struct renew_t { };
    renew_t const renew;
}

template<typename T>
inline void * operator new(std::size_t size, T * p, ns::renew_t renew_constant) {
    p->~T();
    return p;
}

template<typename T>
inline void operator delete(void *, T *, ns::renew_t renew_constant) { }

It can be used like this

int main() {
    foo * p(new foo());        // allocates memory and calls foo's default constructor
    new(p, ns::renew) foo(42); // calls foo's destructor, then calls another of foo's constructors on the same memory
    delete p;                  // calls foo's destructor and deallocates the memory
}

Answer 1

The delete() operator doesn't call the destructor of the object, which is unexpected to me.

I had to do a double take on new(p, ns::renew) foo(42). To me that's simply not intuitive.

What you probably really want is to seperate the process of allocating memory and the process of constructing objects. For situations like this, you typically use "placement new" instead.

// Not exception safe!

void* p = ::operator new(sizeof(T)); // allocate raw memory

new(p) T(41); // construct a T at this memory location
p->~T();      // destruct T

new(p) T(42); // recreate a different T at the same memory location
p->~T();      // destruct T

::operator delete(p); // deallocate raw memory

In a real memory pool application, you would wrap the above lines into a MemoryPool class of some sort.

Of course, this only applies if you're actually dealing with memory directly for the purpose of implementing an actual memory pool or an allocator of a container. In other situations, you're better off overloading the = operator() as suggested by Potatoswatter.

Answer 2

SHOULD be good, as long as you don't try something crazy and try to renew a subclass. Since you said this is for a pool, it should be fine.

That said, my only question is - what is more legible? This is a question of taste, but imagine that somebody else might need to look at the code. You're basically just contracting two simple and obvious statements into one that requires deeper knowledge of what the code internally does.

In my pool functions, I typically had two separate methods, one to destroy and one to construct, both them essentially doing what you do here (p->~T and new(p) T()), but at least you know exactly what they did.

Answer 3

Please read http://www.gotw.ca/gotw/022.htm and http://www.gotw.ca/gotw/023.htm.

Really, you should define operator= rather than playing games with destructors. And adding an operator new (and operator delete, YUCK!) overload into the mix only increases the pain.

And, as C++ god Herb Sutter recommends in those links, you can simply define constructors in terms of operator=.