diff mbox

[RFA] Use gdb::ref_ptr for values

Message ID 20171103173039.6397-1-tom@tromey.com
State New, archived
Headers

Commit Message

Tom Tromey Nov. 3, 2017, 5:30 p.m. UTC 
  A while back I introduced gdb_value_up, a unique_ptr specialization
for struct value.

However, I think this is incorrect -- values are reference counted
(albeit in a somewhat strange way), and so it's better to use a
gdb::ref_ptr specialization.

This patch makes this change.  There was only one use of gdb_value_up.

Regression tested on the buildbot.

gdb/ChangeLog
2017-11-03  Tom Tromey  <tom@tromey.com>

	* value.h (struct value_ref_policy): Rename from value_deleter.
	Change to be gdb::ref_ptr policy class.
	(gdb_value_ref): Rename from gdb_value_up.  Use gdb::ref_ptr., 2017
	* dwarf2loc.c (dwarf2_evaluate_loc_desc_full): Use gdb_value_ref.
---
 gdb/ChangeLog   |  7 +++++++
 gdb/dwarf2loc.c |  2 +-
 gdb/value.h     | 17 +++++++++++------
 3 files changed, 19 insertions(+), 7 deletions(-)

Comments

Pedro Alves Nov. 7, 2017, 10:25 a.m. UTC | #1 
On 11/03/2017 05:30 PM, Tom Tromey wrote:
> A while back I introduced gdb_value_up, a unique_ptr specialization
> for struct value.
> 
> However, I think this is incorrect -- values are reference counted
> (albeit in a somewhat strange way), and so it's better to use a
> gdb::ref_ptr specialization.
> 
> This patch makes this change.  There was only one use of gdb_value_up.
> 
> Regression tested on the buildbot.
> 
> gdb/ChangeLog
> 2017-11-03  Tom Tromey  <tom@tromey.com>
> 
> 	* value.h (struct value_ref_policy): Rename from value_deleter.
> 	Change to be gdb::ref_ptr policy class.
> 	(gdb_value_ref): Rename from gdb_value_up.  Use gdb::ref_ptr., 2017

Looks like a spurious ", 2017" managed to get here.

> 	* dwarf2loc.c (dwarf2_evaluate_loc_desc_full): Use gdb_value_ref.

Seems fine to me.

(IWBN to s/value_free/value_decref/g throughout, I think.)

Thanks,
Pedro Alves
diff mbox

Patch

diff --git a/gdb/dwarf2loc.c b/gdb/dwarf2loc.c
index fe2fea0d0e..0ae604e9ce 100644
--- a/gdb/dwarf2loc.c
+++ b/gdb/dwarf2loc.c
@@ -2487,7 +2487,7 @@  dwarf2_evaluate_loc_desc_full (struct type *type, struct frame_info *frame,
 	       below.  */
 	    value_incref (value);
 	    free_values.free_to_mark ();
-	    gdb_value_up value_holder (value);
+	    gdb_value_ref value_holder (value);
 
 	    retval = allocate_value (subobj_type);
 
diff --git a/gdb/value.h b/gdb/value.h
index cfc8caea05..f581071812 100644
--- a/gdb/value.h
+++ b/gdb/value.h
@@ -22,6 +22,7 @@ 
 
 #include "doublest.h"
 #include "frame.h"		/* For struct frame_id.  */
+#include "common/gdb_ref_ptr.h"
 
 struct block;
 struct expression;
@@ -1022,20 +1023,24 @@  extern void value_incref (struct value *val);
 
 extern void value_free (struct value *val);
 
-/* A free policy class to interface std::unique_ptr with
-   value_free.  */
+/* A policy class to interface gdb::ref_ptr with value.  */
 
-struct value_deleter
+struct value_ref_policy
 {
-  void operator() (struct value *value) const
+  static void incref (struct value *value)
+  {
+    value_incref (value);
+  }
+
+  static void decref (struct value *value)
   {
     value_free (value);
   }
 };
 
-/* A unique pointer to a struct value.  */
+/* A refcounted pointer to a struct value.  */
 
-typedef std::unique_ptr<struct value, value_deleter> gdb_value_up;
+typedef gdb::ref_ptr<struct value, value_ref_policy> gdb_value_ref;
 
 extern void free_all_values (void);