Re: [PATCH] agent_expr_up: gdb::unique_ptr -> std::unique_ptr

[Pedro Alves <palves at redhat dot com>]

On 11/09/2016 12:24 PM, Yao Qi wrote:
> On Wed, Nov 9, 2016 at 12:56 AM, Pedro Alves <palves@redhat.com> wrote:
>>
>> -  /* Vector owns pointers.  */
>> -  std::vector<agent_expr *> m_aexprs;
>> +  std::vector<std::unique_ptr<agent_expr>> m_aexprs;
> 
> Nit: s/std::unique_ptr<agent_expr>/agent_expr_up/ ?

Indeed.  Changed and pushed, as below.

> 
> The patch is good to me.
> 
> [Pretend having some C++ 11 knowledge after reading some pages
> in "C++ Primer, fifth edition" for 20 minutes]

Thanks, I really appreciate it.  I understand the extra learning
effort being imposed on people that may not be up to speed yet.
I'm learning as we go too!

>From 6c73cd95f96d37dbf6092a87c8ba0f35277223a5 Mon Sep 17 00:00:00 2001
From: Pedro Alves <palves@redhat.com>
Date: Wed, 9 Nov 2016 12:49:43 +0000
Subject: [PATCH] agent_expr_up: gdb::unique_ptr -> std::unique_ptr

Now that we require C++11, use std::unique_ptr directly.  This allows
simplifying collection_list a bit by placing unique pointers in the
vector directly, making the vector own its elements.

gdb/ChangeLog:
2016-11-09  Pedro Alves  <palves@redhat.com>

	* ax-gdb.c (agent_eval_command_one): Use std::move instead of
	gdb::move.
	* ax.h (agent_expr_up): Use std::unique_ptr instead of
	gdb::unique_ptr.
	* breakpoint.c (parse_cond_to_aexpr): Use std::move instead of
	gdb::move.
	* tracepoint.c (collection_list::collect_symbol): Likewise.
	(collection_list::~collection_list): Delete.
	(encode_actions_1): Use std::move instead of gdb::move.
	(collection_list::add_aexpr): Use std::move instead of
	unique_ptr::release.
	* tracepoint.h (collection_list) <~collection_list>: Delete
	declaration.
	<m_aexprs>: Now a vector of agent_ptr_up.
---
 gdb/ChangeLog    | 17 +++++++++++++++++
 gdb/ax-gdb.c     |  6 +++---
 gdb/ax.h         |  2 +-
 gdb/breakpoint.c |  4 ++--
 gdb/tracepoint.c | 16 +++++-----------
 gdb/tracepoint.h |  4 +---
 6 files changed, 29 insertions(+), 20 deletions(-)

diff --git a/gdb/ChangeLog b/gdb/ChangeLog
index 4d16c38..de49596 100644
--- a/gdb/ChangeLog
+++ b/gdb/ChangeLog
@@ -1,5 +1,22 @@
 2016-11-09  Pedro Alves  <palves@redhat.com>
 
+	* ax-gdb.c (agent_eval_command_one): Use std::move instead of
+	gdb::move.
+	* ax.h (agent_expr_up): Use std::unique_ptr instead of
+	gdb::unique_ptr.
+	* breakpoint.c (parse_cond_to_aexpr): Use std::move instead of
+	gdb::move.
+	* tracepoint.c (collection_list::collect_symbol): Likewise.
+	(collection_list::~collection_list): Delete.
+	(encode_actions_1): Use std::move instead of gdb::move.
+	(collection_list::add_aexpr): Use std::move instead of
+	unique_ptr::release.
+	* tracepoint.h (collection_list) <~collection_list>: Delete
+	declaration.
+	<m_aexprs>: Now a vector of agent_ptr_up.
+
+2016-11-09  Pedro Alves  <palves@redhat.com>
+
 	* main.c (struct cmdarg): Add constructor.
 	(captured_main_1): Use vector::emplace_back.
 	* tracepoint.c (collection_list::add_memrange): Likewise.
diff --git a/gdb/ax-gdb.c b/gdb/ax-gdb.c
index 1816ba6..cd97585 100644
--- a/gdb/ax-gdb.c
+++ b/gdb/ax-gdb.c
@@ -2555,7 +2555,7 @@ agent_eval_command_one (const char *exp, int eval, CORE_ADDR pc)
   arg = exp;
   if (!eval && strcmp (arg, "$_ret") == 0)
     {
-      agent = gdb::move (gen_trace_for_return_address (pc, get_current_arch (),
+      agent = std::move (gen_trace_for_return_address (pc, get_current_arch (),
 						       trace_string));
     }
   else
@@ -2565,10 +2565,10 @@ agent_eval_command_one (const char *exp, int eval, CORE_ADDR pc)
       if (eval)
 	{
 	  gdb_assert (trace_string == 0);
-	  agent = gdb::move (gen_eval_for_expr (pc, expr.get ()));
+	  agent = std::move (gen_eval_for_expr (pc, expr.get ()));
 	}
       else
-	agent = gdb::move (gen_trace_for_expr (pc, expr.get (), trace_string));
+	agent = std::move (gen_trace_for_expr (pc, expr.get (), trace_string));
     }
 
   ax_reqs (agent.get ());
diff --git a/gdb/ax.h b/gdb/ax.h
index 85d2943..dbbfa0b 100644
--- a/gdb/ax.h
+++ b/gdb/ax.h
@@ -168,7 +168,7 @@ struct agent_expr
   };
 
 /* An agent_expr owning pointer.  */
-typedef gdb::unique_ptr<agent_expr> agent_expr_up;
+typedef std::unique_ptr<agent_expr> agent_expr_up;
 
 /* The actual values of the various bytecode operations.  */
 
diff --git a/gdb/breakpoint.c b/gdb/breakpoint.c
index a3a81f7..245e078 100644
--- a/gdb/breakpoint.c
+++ b/gdb/breakpoint.c
@@ -2268,7 +2268,7 @@ parse_cond_to_aexpr (CORE_ADDR scope, struct expression *cond)
      that may show up.  */
   TRY
     {
-      aexpr = gdb::move (gen_eval_for_expr (scope, cond));
+      aexpr = std::move (gen_eval_for_expr (scope, cond));
     }
 
   CATCH (ex, RETURN_MASK_ERROR)
@@ -2452,7 +2452,7 @@ parse_cmd_to_aexpr (CORE_ADDR scope, char *cmd)
      that may show up.  */
   TRY
     {
-      aexpr = gdb::move (gen_printf (scope, gdbarch, 0, 0,
+      aexpr = std::move (gen_printf (scope, gdbarch, 0, 0,
 				     format_start, format_end - format_start,
 				     fpieces, nargs, argvec));
     }
diff --git a/gdb/tracepoint.c b/gdb/tracepoint.c
index 2a40b16..0cb12c7 100644
--- a/gdb/tracepoint.c
+++ b/gdb/tracepoint.c
@@ -1076,7 +1076,7 @@ collection_list::collect_symbol (struct symbol *sym,
 	    }
 	}
 
-      add_aexpr (gdb::move (aexpr));
+      add_aexpr (std::move (aexpr));
     }
 }
 
@@ -1180,12 +1180,6 @@ collection_list::collection_list ()
   m_aexprs.reserve (128);
 }
 
-collection_list::~collection_list ()
-{
-  for (int ndx = 0; ndx < m_aexprs.size (); ndx++)
-    delete m_aexprs[ndx];
-}
-
 /* Reduce a collection list to string form (for gdb protocol).  */
 
 char **
@@ -1419,7 +1413,7 @@ encode_actions_1 (struct command_line *action,
 			}
 		    }
 
-		  collect->add_aexpr (gdb::move (aexpr));
+		  collect->add_aexpr (std::move (aexpr));
 		  action_exp = strchr (action_exp, ',');	/* more? */
 		}
 	      else if (0 == strncasecmp ("$_sdata", action_exp, 7))
@@ -1508,7 +1502,7 @@ encode_actions_1 (struct command_line *action,
 			    }
 			}
 
-		      collect->add_aexpr (gdb::move (aexpr));
+		      collect->add_aexpr (std::move (aexpr));
 		      collect->append_exp (exp.get ());
 		      break;
 		    }		/* switch */
@@ -1538,7 +1532,7 @@ encode_actions_1 (struct command_line *action,
 
 		  /* Even though we're not officially collecting, add
 		     to the collect list anyway.  */
-		  collect->add_aexpr (gdb::move (aexpr));
+		  collect->add_aexpr (std::move (aexpr));
 		}		/* do */
 	    }
 	  while (action_exp && *action_exp++ == ',');
@@ -1602,7 +1596,7 @@ encode_actions_rsp (struct bp_location *tloc, char ***tdp_actions,
 void
 collection_list::add_aexpr (agent_expr_up aexpr)
 {
-  m_aexprs.push_back (aexpr.release ());
+  m_aexprs.push_back (std::move (aexpr));
 }
 
 static void
diff --git a/gdb/tracepoint.h b/gdb/tracepoint.h
index 7dfd8cc..855bb1d 100644
--- a/gdb/tracepoint.h
+++ b/gdb/tracepoint.h
@@ -243,7 +243,6 @@ class collection_list
 {
 public:
   collection_list ();
-  ~collection_list ();
 
   void add_wholly_collected (const char *print_name);
 
@@ -282,8 +281,7 @@ private:
 
   std::vector<memrange> m_memranges;
 
-  /* Vector owns pointers.  */
-  std::vector<agent_expr *> m_aexprs;
+  std::vector<agent_expr_up> m_aexprs;
 
   /* True is the user requested a collection of "$_sdata", "static
      tracepoint data".  */
-- 
2.5.5