Introduce a gdb_ref_ptr specialization for struct value

struct value is internally reference counted and so, while it also has
some ownership rules unique to it, it makes sense to use a gdb_ref_ptr
when managing it automatically.

This patch removes the existing unique_ptr specialization in favor of
a reference-counted pointer.  It also introduces two other
clarifications:

1. Rename value_free to value_decref, which I think is more in line
   with what the function actually does; and

2. Change release_value to return a gdb_ref_ptr.  This change allows
   us to remove the confusing release_value_or_incref function,
   primarily by making it much simpler to reason about the result of
   release_value.

gdb/ChangeLog
2018-04-06  Tom Tromey  <tom@tromey.com>

	* varobj.c (varobj_clear_saved_item)
	(update_dynamic_varobj_children, install_new_value, ~varobj):
	Update.
	* value.h (value_incref): Move declaration earlier.
	(value_decref): Rename from value_free.
	(struct value_ref_policy): New.
	(value_ref_ptr): New typedef.
	(struct value_deleter): Remove.
	(gdb_value_up): Remove typedef.
	(release_value): Change return type.
	(release_value_or_incref): Remove.
	* value.c (set_value_parent): Update.
	(value_incref): Change return type.
	(value_decref): Rename from value_free.
	(value_free_to_mark, free_all_values, free_value_chain): Update.
	(release_value): Return value_ref_ptr.
	(release_value_or_incref): Remove.
	(record_latest_value, set_internalvar, clear_internalvar):
	Update.
	* stack.c (info_frame_command): Don't call value_free.
	* python/py-value.c (valpy_dealloc, valpy_new)
	(value_to_value_object): Update.
	* printcmd.c (do_examine): Update.
	* opencl-lang.c (lval_func_free_closure): Update.
	* mi/mi-main.c (register_changed_p): Don't call value_free.
	* mep-tdep.c (mep_frame_prev_register): Don't call value_free.
	* m88k-tdep.c (m88k_frame_prev_register): Don't call value_free.
	* m68hc11-tdep.c (m68hc11_frame_prev_register): Don't call
	value_free.
	* guile/scm-value.c (vlscm_free_value_smob)
	(vlscm_scm_from_value): Update.
	* frame.c (frame_register_unwind, frame_unwind_register_signed)
	(frame_unwind_register_unsigned, get_frame_register_bytes)
	(put_frame_register_bytes): Don't call value_free.
	* findvar.c (address_from_register): Don't call value_free.
	* dwarf2read.c (dwarf2_compute_name): Don't call value_free.
	* dwarf2loc.c (entry_data_value_free_closure)
	(value_of_dwarf_reg_entry, free_pieced_value_closure)
	(dwarf2_evaluate_loc_desc_full): Update.
	* breakpoint.c (update_watchpoint, breakpoint_init_inferior)
	(~bpstats, bpstats, bpstat_clear_actions, watchpoint_check)
	(~watchpoint, watch_command_1)
	(invalidate_bp_value_on_memory_change): Update.
	* alpha-tdep.c (alpha_register_to_value): Don't call value_free.

1 files changed, 31 insertions, 37 deletions

diff --git a/gdb/value.c b/gdb/value.c
index 110d16d..002270f 100644
--- a/gdb/value.c
+++ b/gdb/value.c
@@ -1151,7 +1151,7 @@ set_value_parent (struct value *value, struct value *parent)
   value->parent = parent;
   if (parent != NULL)
     value_incref (parent);
-  value_free (old);
+  value_decref (old);
 }
 
 gdb_byte *
@@ -1594,10 +1594,11 @@ value_mark (void)
 /* Take a reference to VAL.  VAL will not be deallocated until all
    references are released.  */
 
-void
+struct value *
 value_incref (struct value *val)
 {
   val->reference_count++;
+  return val;
 }
 
 /* Release a reference to VAL, which was acquired with value_incref.
@@ -1605,7 +1606,7 @@ value_incref (struct value *val)
    chain.  */
 
 void
-value_free (struct value *val)
+value_decref (struct value *val)
 {
   if (val)
     {
@@ -1617,7 +1618,7 @@ value_free (struct value *val)
       /* If there's an associated parent value, drop our reference to
 	 it.  */
       if (val->parent != NULL)
-	value_free (val->parent);
+	value_decref (val->parent);
 
       if (VALUE_LVAL (val) == lval_computed)
 	{
@@ -1647,7 +1648,7 @@ value_free_to_mark (const struct value *mark)
     {
       next = val->next;
       val->released = 1;
-      value_free (val);
+      value_decref (val);
     }
   all_values = val;
 }
@@ -1666,7 +1667,7 @@ free_all_values (void)
     {
       next = val->next;
       val->released = 1;
-      value_free (val);
+      value_decref (val);
     }
 
   all_values = 0;
@@ -1682,50 +1683,48 @@ free_value_chain (struct value *v)
   for (; v; v = next)
     {
       next = value_next (v);
-      value_free (v);
+      value_decref (v);
     }
 }
 
 /* Remove VAL from the chain all_values
    so it will not be freed automatically.  */
 
-void
+value_ref_ptr
 release_value (struct value *val)
 {
   struct value *v;
+  bool released = false;
 
   if (all_values == val)
     {
       all_values = val->next;
       val->next = NULL;
-      val->released = 1;
-      return;
+      released = true;
     }
-
-  for (v = all_values; v; v = v->next)
+  else
     {
-      if (v->next == val)
+      for (v = all_values; v; v = v->next)
 	{
-	  v->next = val->next;
-	  val->next = NULL;
-	  val->released = 1;
-	  break;
+	  if (v->next == val)
+	    {
+	      v->next = val->next;
+	      val->next = NULL;
+	      released = true;
+	      break;
+	    }
 	}
     }
-}
 
-/* If the value is not already released, release it.
-   If the value is already released, increment its reference count.
-   That is, this function ensures that the value is released from the
-   value chain and that the caller owns a reference to it.  */
+  if (!released)
+    {
+      /* We must always return an owned reference.  Normally this
+	 happens because we transfer the reference from the value
+	 chain, but in this case the value was not on the chain.  */
+      value_incref (val);
+    }
 
-void
-release_value_or_incref (struct value *val)
-{
-  if (val->released)
-    value_incref (val);
-  else
-    release_value (val);
+  return value_ref_ptr (val);
 }
 
 /* Release all values up to mark  */
@@ -1896,11 +1895,6 @@ record_latest_value (struct value *val)
      but the current contents of that location.  c'est la vie...  */
   val->modifiable = 0;
 
-  /* The value may have already been released, in which case we're adding a
-     new reference for its entry in the history.  That is why we call
-     release_value_or_incref here instead of release_value.  */
-  release_value_or_incref (val);
-
   /* Here we treat value_history_count as origin-zero
      and applying to the value being stored now.  */
 
@@ -1913,7 +1907,7 @@ record_latest_value (struct value *val)
       value_history_chain = newobj;
     }
 
-  value_history_chain->values[i] = val;
+  value_history_chain->values[i] = release_value (val).release ();
 
   /* Now we regard value_history_count as origin-one
      and applying to the value just stored.  */
@@ -2416,7 +2410,7 @@ set_internalvar (struct internalvar *var, struct value *val)
 	 deleted by free_all_values.  From here on this function should not
 	 call error () until new_data is installed into the var->u to avoid
 	 leaking memory.  */
-      release_value (new_data.value);
+      release_value (new_data.value).release ();
 
       /* Internal variables which are created from values with a dynamic
          location don't need the location property of the origin anymore.
@@ -2478,7 +2472,7 @@ clear_internalvar (struct internalvar *var)
   switch (var->kind)
     {
     case INTERNALVAR_VALUE:
-      value_free (var->u.value);
+      value_decref (var->u.value);
       break;
 
     case INTERNALVAR_STRING: