This is the mail archive of the
gdb-patches@sourceware.org
mailing list for the GDB project.
Re: [PATCH 1/3] Introduce gdb::unique_ptr
On 10/11/2016 07:47 AM, Metzger, Markus T wrote:
>> -----Original Message-----
>> From: gdb-patches-owner@sourceware.org [mailto:gdb-patches-
>> owner@sourceware.org] On Behalf Of Pedro Alves
>> Sent: Monday, October 10, 2016 6:47 PM
>> To: gdb-patches@sourceware.org
>> Subject: [PATCH 1/3] Introduce gdb::unique_ptr
>
> Hello Pedro,
Hi Markus,
>
>> Many make_cleanup uses in the code base are best eliminated by using a
>> "owning" smart pointer to manage ownership of the resource
>> automatically.
>>
>> The question is _which_ smart pointer.
>>
>> We have std::auto_ptr in C++03, but, as is collective wisdom by now,
>> that's too easy to misuse, and has therefore been deprecated in C++11
>> and finally removed in C++17.
>
> Would it make sense to switch to C++11, instead?
I think it would make a lot of sense to switch to C++11. I'd love
that. rvalue references support, move-aware containers, "auto",
std::unique_ptr and std::shared_ptr would be all very nice to have.
The only question in my mind is -- are people OK with requiring
gcc 4.8 or later?
I think gcc 4.8 or newer were available in Fedora 20. I believe
Ubuntu 12.04 had it available as an option. On older RHEL systems,
it's available in DTS.
On older systems, you'd need to compile a newer gcc or clang first
before building gdb. Would people find that acceptable? Or put
another way, would anyone find that unacceptable?
But meanwhile, I don't want C++ conversion to be blocked
by that. Hence this new shim.
Even if we don't require C++11, I think it'd make sense to compile
in C++11 mode if we have a capable compiler. I.e., if you have
gcc 4.8 or newer, pass -std=gnu++11/-std=gnu++0x if necessary.
> Or delay this work until we can make this move?
No, I don't think so. I think we need _some_ owning smart pointer.
An alternative is some separate RAII object that destroys a pointer
when leaving a scope:
template <class T>
struct cleanup_pointer
{
explicit cleanup_pointer(T *p) : m_ptr (p) {}
cleanup_pointer { if (m_ptr != NULL) delete m_ptr; }
void discard () { m_ptr = NULL; }
T *m_ptr;
};
// Document that this returns a new object that must
// be released with "delete".
struct foo *function_returning_new_ptr ();
// Document that this uses P, ownership is still with the caller.
// May throw.
void foo (struct foo *p);
// Document that P is no longer owned by the caller after this.
// May throw.
void bar (struct foo *p);
void some_function ()
{
struct foo *ptr = function_returning_new_ptr ();
cleanup_pointer<struct foo *> cleanup (ptr);
ptr->foo = .....;
bar (ptr); // whoops, bar now owns the pointer, but
// cleanup_pointer will delete it anyway
// once we leave this scope
// would need to remember to call this.
// cleanup.discard ();
}
That looks very much like our current cleanups, except we
no longer need the "do_cleanups" call. But it's far from idiomatic
C++. And, ends up being more code to write/maintain
than using a smart pointer. We can do better. E.g.,:
// (gdb::unique_ptr is like std::unique_ptr and
// gdb::move is like std::move for gdb::unique_ptr)
// prototype clearly tells you this returns a new object.
gdb::unique_ptr <struct foo> function_returning_new_ptr ();
// Uses P, ownership is still with the caller.
void foo (struct foo *p);
// Clearly tells you this transfers ownership.
void bar (gdb::unique_ptr <struct foo> p);
void some_function ()
{
// struct foo *ptr = function_returning_new_ptr (); // would not compile.
gdb::unique_ptr <struct foo> ptr
= function_returning_new_ptr (); // OK, ownership clearly transferred to caller.
ptr->foo = .....;
foo (ptr.get ()); // clearly foo borrows the pointer.
bar (gdb::move (ptr)); // clearly transfers ownership.
}
The other use case a smart pointer clarifies is making
a structure/class be the owner of some pointer. Like:
struct foo
{
gdb::unique_ptr <struct foo> ptr;
};
When a "foo" is destroyed, then ptr is destroyed automatically
as well. Without the smart pointer, we'd have to explicitly
write delete ptr in a destructor:
struct foo
{
~foo() { delete ptr; }
struct foo *ptr;
};
Most C++03 projects past a size/history will bake their own
custom smart pointers for the use cases above. Most large projects
will have invented theirs before std::unique_ptr existed though, so
they'll differ in API details. However in our case, std::unique_ptr is
already there, so I think it makes a lot of sense to reuse the same
API, and take advantage of newer compilers as "lints", since
that's what people will be using for development anyway. Once we do
require C++11, then we won't have to relearn a new smart pointer API.
Even if we were to use std::auto_ptr only, I would still think that that
would be an improvement over raw pointers, since with raw pointers
it's even easier to confuse "borrowing pointers" from "owning pointers",
since you have 0 zero help from the type system.
The potential problems with std::auto_ptr stem from its
"move semantics", and are of the form:
std::auto_ptr<type> ptr = function_returning_type ();
std::auto_ptr<type> ptr2 = ptr; // this moves instead of
// copying. I.e., ptr is NULL after this.
and:
void function (std::auto_ptr<type> ptr);
function (ptr); // This moves instead of copying. ptr is NULL after this.
ptr->foo = 1; // crash.
In the latter case, the function prototype indicates that the caller
wants to transfer ownership of the pointer to the callee. But that's
not clear at the call site.
So in C++03 mode, gdb::unique_ptr "allows" that too. I.e., the compiler
won't complain. However, that _won't_ compile in C++11 mode, forcing you
to write instead:
gdb::unique_ptr<type> ptr2 = gdb::move (ptr); // OK, now move is clearly
// what was intended.
function (gdb::move (ptr2)); // Likewise.
ptr2->foo = 1; // crash, but, obviously incorrect.
And of course the C++03 gdb::unique_ptr version supports this as well.
There's also the advantage that gdb::unique_ptr works with malloced
objects, and arrays (for 'new some_type[];'). We'd need _some_ smart
pointer for those, so std::auto_ptr alone won't be sufficient anyway...
WDYT?
Thanks,
Pedro Alves