Re: [PATCH 1/3] Introduce gdb::unique_ptr

[Pedro Alves <palves at redhat dot com>]

On 10/11/2016 06:14 PM, Luis Machado wrote:

> Some of Eli's points resonate with me. It seems we just recently got the
> C++ compatibility sorted out and we're quickly moving on to try and
> C++-ify what we can with no clear goal, priority list or high level
> picture. So, just to make it C++ now that we require a C++ compiler.

It's all described at:

 https://sourceware.org/gdb/wiki/cxx-conversion#Transition_plan

One of the goals right now is to replace cleanups with something
that the compiler does automatically.  That is, completely
eliminate the "forgot to call do_cleanups" bug by design.

That generally involves:

  - replacing cleanups with either RAII objects (Tromey's
    scoped_restore class)
  - converting structs to classes with destructors.  Make
    some classes be value objects allocated on the stack.
  - make use of std::string, which tends to simplify the code
    significantly anyway.

> 
> I thought i'd throw this question out there. Wasn't the goal of moving
> to C++ to help the implementation of features that were hard to
> implement with C like some of the Fortran bits with dynamic arrays? Also
> improve the poor experience with C++ debugging?

All the above.  Want to lend a hand?  :-)

See also:

  https://sourceware.org/ml/gdb-patches/2016-09/msg00162.html

for another example.

> Another point i worry about is that we will switch the focus, at least
> for a few months, to C++-ifying things "just because" instead of taking
> the opportunity of such a big change to review bits of GDB that need to
> be redesigned/dropped/go away. Maybe just converting existing things to
> C++ is not the right answer, even though it is fun to do and will
> provide enough hacking fun?

Well, IMO, it makes sense to first C++ify the code without changing much
the design, and then redesign on top, because it'll tend to be easier
once C++-yied.  Also in order to do "atomic" changes; one concern
per change.

Another point is that I've heard time and time again (and have
experienced it too) that redesigns don't happen exactly because
the code is hard to massage!  E.g., raise your hand if you've ever
thought that it'd be nice to just use inheritance for something
but didn't because doing it manually requires so much manual
labor in C, and so you ended with some quicker hack adding 
to the mess instead.  Ever looked at gdb/breakpoint.c, for example?
(Rhetoric, I know you have ;-) )

Back to cleanups, here's an example (from patch #3) of how the
code ends up so much more readable when using modern practices:

@@ -95,14 +95,9 @@ evaluate_subexp (struct type *expect_type, struct expression *exp,
 CORE_ADDR
 parse_and_eval_address (const char *exp)
 {
-  struct expression *expr = parse_expression (exp);
-  CORE_ADDR addr;
-  struct cleanup *old_chain =
-    make_cleanup (free_current_contents, &expr);
-
-  addr = value_as_address (evaluate_expression (expr));
-  do_cleanups (old_chain);
-  return addr;
+  expression_up expr = parse_expression (exp);
+
+  return value_as_address (evaluate_expression (expr.get ()));
 }


I've mentioned this before, but I'm a big believer in keeping
the code base inviting to work on.  If there's people willing to work
on cleaning up old ugly code, why not?  If something breaks and
we don't notice it, I'll blame it on lack of testsuite coverage.  :-)

> I have another point about whether this will stimulate more contributors
> to send patches to GDB or not, but that's enough rambling for now. :-)

Seems to be working so far, IMO.  :-)

Thanks,
Pedro Alves