0
\$\begingroup\$

I need to make a vector of different types and be able to delete the values (please ignore anything else but deleting the values for now). Is the next code safe to delete the values?

#include <memory>
#include <vector>
#include <string>

using MyPtr = std::unique_ptr<void, void(*)(void*)>;

template<typename T>
MyPtr MakePtr(const T& val)
{
    return MyPtr(new T(val), [](void* ptr){delete static_cast<T*>(ptr);});
}

int main()
{
    std::vector<MyPtr> ptrs;

    ptrs.push_back(MakePtr(1));
    ptrs.push_back(MakePtr(1.1));
    ptrs.push_back(MakePtr(1.f));
    ptrs.push_back(MakePtr(std::string("1")));

    return 0;
}
\$\endgroup\$
  • \$\begingroup\$ Might be more appropriate at stackoverflow.com? \$\endgroup\$ – CashCow Oct 20 '14 at 10:06
  • \$\begingroup\$ There's not enough context to review this. Whatever you are trying to do, this is probably not the way to do it. \$\endgroup\$ – Yuushi Oct 20 '14 at 10:14
1
\$\begingroup\$

You have asked for a "review" but really asked whether your deleter is safe or not.

Whilst the deleter looks to be technically not an issue as you can go from your pointer type to void* and back and should get to where you started, it doesn't look right to create your variant through unique_ptr<void> in which you are not storing any type-info so how exactly do you get the values out as the values?

Variants in C++: Yes I have seen loads of implementations. I have made my own too. Even boost contains two. We would need to know really more context of what types you might store, and how you need to extract them.

The reason there are so many is that often you are going to have lots of them, carrying lots of data about, and they need to work efficiently for your own situation.

\$\endgroup\$
3
\$\begingroup\$

Is the next code safe to delete the values?

No. At the very least, you should probably use reinterpret_cast instead of static_cast.Yes.

That said, it seems you are having the x-y problem. Storing data in C++, when you have heterogenous types in a sequence, should be done using a variant type, not void*; in other words, you should create a system that stores your type information per type/variable instance, instead of discarding all type information - like void* does). You are better off maintaining sepatate element lists (sparse arrays or some kind) by type, than you are with a vector<void*> (or vector<unique_ptr<void>> for that matter).

Have a look at boost::any and boost::variant before implementing your solution. You could use one of those, or a separate (similar) implementation.

\$\endgroup\$
  • \$\begingroup\$ Why do you say I should use reinterpret_cast instead of static_cast? \$\endgroup\$ – Mircea Ispas Oct 20 '14 at 11:32
  • \$\begingroup\$ I don't see that reinterpret_cast will be any different / better here although I am not sure if the user is asking if his deleter is safe or whether he should make his own variant from unique_ptr<void> \$\endgroup\$ – CashCow Oct 20 '14 at 11:37
  • \$\begingroup\$ @CashCow I'm just asking if my deleter is safe. \$\endgroup\$ – Mircea Ispas Oct 20 '14 at 11:37
  • \$\begingroup\$ @Felics, you shouldn't - my bad (I have edited my response). \$\endgroup\$ – utnapistim Oct 20 '14 at 11:51
  • 1
    \$\begingroup\$ @CashCow: I prefer reinterpret_cast<> in this situation. It should work the same as static_cast<> but it sticks out more. So you have a harder look a reinterpret_cast<> to make sure they are correct. People have a tendency (incorrectly) to think that static_cast<> is safer than reinterpret_cast<> I want them to see this situation as the most dangerous way they can think of it so that they put real eyeballs on the code and scrutinize it. \$\endgroup\$ – Martin York Oct 20 '14 at 19:21

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.