Let’s just ignore the associatedComponent lifetime for the moment, and concentrate on the callback stuff.
It looks like the method:
void AlertWindow::showMessageBoxAsync (MessageBoxIconType iconType,
const String& title,
const String& message,
const String& buttonText,
Component* associatedComponent,
ModalComponentManager::Callback* callback)
first takes a raw pointer to a “callback”, but then turns this into a std::unique_pointer later on inside:
static std::unique_ptr<ModalComponentManager::Callback>
getWrappedCallback (ModalComponentManager::Callback* callbackIn, ...)
{
...
innerCallback = rawToUniquePtr (callbackIn)
...
}
where:
template <typename T>
std::unique_ptr<T> rawToUniquePtr (T* ptr)
{
return std::unique_ptr<T> (ptr);
}
I’m not an expert on this stuff, but shouldn’t that initial call use some of the std::unique_ptr stuff to begin with?
Hey,
If i understand correctly your question, you’re worried about passing a raw pointer which is then wrapped into a unique pointer so it’s automatically deleted when destroyed. I think it’s fine as long as you ensure you don’t keep a ref or anything to the pointer you pass since its lifetime will now be managed by AlertWindow or whatever. I think it’s mentioned relatively clearly in the doc (“The callback object will be owned and deleted by the system”): JUCE: AlertWindow Class Reference
@param callback if this is non-null, the callback will receive a call to its
modalStateFinished() when the box is dismissed. The callback object
will be owned and deleted by the system, so make sure that it works
safely and doesn’t keep any references to objects that might be deleted
before it gets called.
The point of std::smart_ptr is to make it clear the object lifetime and who owns what. Calling new to create a raw pointer defeats this purpose. For example from here:
“The rule would be this - if you know that an entity must take a certain kind of ownership of the object, always use smart pointers - the one that gives you the kind of ownership you need. If there is no notion of ownership, never use smart pointers.”
Here there is ownership but a raw pointer is used, which is in the never category.
i think the idea of this is that it’s nice that you can just call new into the function without having to deal with the lifetime, because “new” is shorter than “std::make_unique<>”
This is just the nature of long-lived APIs. This stuff was written before unique_ptr was a thing so the interface uses raw pointers. Changing to unique_ptr would break peoples older code. You’ll find this all over the JUCE codebase, but with newer APIs you’ll typically see smart pointers used.
1 Like
This is simply an old function. A major part of JUCE has been written before std::unique_ptr even existed or was considered general best practice. In the old days you just passed pointers to functions and had to read the function documentation in order to figure out if the receiver takes over ownership of the pointed to argument or not. In this case, the documentation is clear on that:
I’m pretty sure that if this function would have been added to the library nowadays, it would expect a std::unique_ptr right away, because that’s the modern way of expressing an ownership transfer in C++. But that change would be breaking for existing code and adding a second overload would probably make a confusing API, so this is why we still see a few occurrences like this in the library.
2 Likes
If you want to make it clearer that ownership is being moved you could do something like:
void someOldAPI (Object* owningPtr);
auto object = std::make_unique<Object> (...);
// do stuff with object...
someOldAPI (object.release());
Which should keep tools like clang-tidy happy too as they might throw warnings when they see new.
2 Likes
