[RFA,04/12] Change varobj to use value_ref_ptr

Message ID 20180405211507.6103-5-tom@tromey.com
State New
Headers show
Series
  • (somewhat) clean up struct value ownership
Related show

Commit Message

Tom Tromey April 5, 2018, 9:14 p.m.
This changes varobj to use value_ref_ptr, allowing the removal of some
manual reference count management.

ChangeLog
2018-04-05  Tom Tromey  <tom@tromey.com>

	* varobj.h (struct varobj) <value>: Now a value_ref_ptr.
	* varobj.c (varobj_set_display_format, varobj_set_value)
	(install_default_visualizer, construct_visualizer)
	(install_new_value, ~varobj, varobj_get_value_type)
	(my_value_of_variable, varobj_editable_p): Update.
	* c-varobj.c (c_describe_child, c_value_of_variable)
	(cplus_number_of_children, cplus_describe_child): Update.
	* ada-varobj.c (ada_number_of_children, ada_name_of_child)
	(ada_path_expr_of_child, ada_value_of_child, ada_type_of_child)
	(ada_value_of_variable, ada_value_is_changeable_p): Update.
---
 gdb/ChangeLog    | 13 +++++++++++++
 gdb/ada-varobj.c | 16 +++++++++-------
 gdb/c-varobj.c   | 15 ++++++++-------
 gdb/varobj.c     | 43 ++++++++++++++++++++++---------------------
 gdb/varobj.h     |  3 ++-
 5 files changed, 54 insertions(+), 36 deletions(-)

-- 
2.13.6

Patch

diff --git a/gdb/ada-varobj.c b/gdb/ada-varobj.c
index 1257ee864e..6dafe472c7 100644
--- a/gdb/ada-varobj.c
+++ b/gdb/ada-varobj.c
@@ -872,7 +872,7 @@  ada_varobj_get_value_of_variable (struct value *value,
 static int
 ada_number_of_children (const struct varobj *var)
 {
-  return ada_varobj_get_number_of_children (var->value, var->type);
+  return ada_varobj_get_number_of_children (var->value.get (), var->type);
 }
 
 static std::string
@@ -884,7 +884,7 @@  ada_name_of_variable (const struct varobj *parent)
 static std::string
 ada_name_of_child (const struct varobj *parent, int index)
 {
-  return ada_varobj_get_name_of_child (parent->value, parent->type,
+  return ada_varobj_get_name_of_child (parent->value.get (), parent->type,
 				       parent->name.c_str (), index);
 }
 
@@ -894,7 +894,7 @@  ada_path_expr_of_child (const struct varobj *child)
   const struct varobj *parent = child->parent;
   const char *parent_path_expr = varobj_get_path_expr (parent);
 
-  return ada_varobj_get_path_expr_of_child (parent->value,
+  return ada_varobj_get_path_expr_of_child (parent->value.get (),
 					    parent->type,
 					    parent->name.c_str (),
 					    parent_path_expr,
@@ -904,14 +904,14 @@  ada_path_expr_of_child (const struct varobj *child)
 static struct value *
 ada_value_of_child (const struct varobj *parent, int index)
 {
-  return ada_varobj_get_value_of_child (parent->value, parent->type,
+  return ada_varobj_get_value_of_child (parent->value.get (), parent->type,
 					parent->name.c_str (), index);
 }
 
 static struct type *
 ada_type_of_child (const struct varobj *parent, int index)
 {
-  return ada_varobj_get_type_of_child (parent->value, parent->type,
+  return ada_varobj_get_type_of_child (parent->value.get (), parent->type,
 				       index);
 }
 
@@ -923,7 +923,8 @@  ada_value_of_variable (const struct varobj *var,
 
   varobj_formatted_print_options (&opts, format);
 
-  return ada_varobj_get_value_of_variable (var->value, var->type, &opts);
+  return ada_varobj_get_value_of_variable (var->value.get (), var->type,
+					   &opts);
 }
 
 /* Implement the "value_is_changeable_p" routine for Ada.  */
@@ -931,7 +932,8 @@  ada_value_of_variable (const struct varobj *var,
 static bool
 ada_value_is_changeable_p (const struct varobj *var)
 {
-  struct type *type = var->value ? value_type (var->value) : var->type;
+  struct type *type = (var->value != nullptr
+		       ? value_type (var->value.get ()) : var->type);
 
   if (ada_is_array_descriptor_type (type)
       && TYPE_CODE (type) == TYPE_CODE_TYPEDEF)
diff --git a/gdb/c-varobj.c b/gdb/c-varobj.c
index babda29278..03656c0a81 100644
--- a/gdb/c-varobj.c
+++ b/gdb/c-varobj.c
@@ -286,7 +286,7 @@  c_describe_child (const struct varobj *parent, int index,
 		  std::string *cname, struct value **cvalue,
 		  struct type **ctype, std::string *cfull_expression)
 {
-  struct value *value = parent->value;
+  struct value *value = parent->value.get ();
   struct type *type = varobj_get_value_type (parent);
   std::string parent_expression;
   int was_ptr;
@@ -513,21 +513,22 @@  c_value_of_variable (const struct varobj *var,
 	  }
 	else
 	  {
-	    if (var->not_fetched && value_lazy (var->value))
+	    if (var->not_fetched && value_lazy (var->value.get ()))
 	      /* Frozen variable and no value yet.  We don't
 		 implicitly fetch the value.  MI response will
 		 use empty string for the value, which is OK.  */
 	      return std::string ();
 
 	    gdb_assert (varobj_value_is_changeable_p (var));
-	    gdb_assert (!value_lazy (var->value));
+	    gdb_assert (!value_lazy (var->value.get ()));
 	    
 	    /* If the specified format is the current one,
 	       we can reuse print_value.  */
 	    if (format == var->format)
 	      return var->print_value;
 	    else
-	      return varobj_value_get_print_value (var->value, format, var);
+	      return varobj_value_get_print_value (var->value.get (), format,
+						   var);
 	  }
       }
     }
@@ -579,7 +580,7 @@  cplus_number_of_children (const struct varobj *var)
       /* It is necessary to access a real type (via RTTI).  */
       if (opts.objectprint)
         {
-          value = var->value;
+          value = var->value.get ();
           lookup_actual_type = (TYPE_IS_REFERENCE (var->type)
 				|| TYPE_CODE (var->type) == TYPE_CODE_PTR);
         }
@@ -616,7 +617,7 @@  cplus_number_of_children (const struct varobj *var)
         {
 	  const struct varobj *parent = var->parent;
 
-	  value = parent->value;
+	  value = parent->value.get ();
 	  lookup_actual_type = (TYPE_IS_REFERENCE (parent->type)
 				|| TYPE_CODE (parent->type) == TYPE_CODE_PTR);
         }
@@ -724,7 +725,7 @@  cplus_describe_child (const struct varobj *parent, int index,
   if (opts.objectprint)
     lookup_actual_type = (TYPE_IS_REFERENCE (var->type)
 			  || TYPE_CODE (var->type) == TYPE_CODE_PTR);
-  value = var->value;
+  value = var->value.get ();
   type = varobj_get_value_type (var);
   if (cfull_expression)
     parent_expression
diff --git a/gdb/varobj.c b/gdb/varobj.c
index 58eb531927..2d00964317 100644
--- a/gdb/varobj.c
+++ b/gdb/varobj.c
@@ -522,9 +522,9 @@  varobj_set_display_format (struct varobj *var,
     }
 
   if (varobj_value_is_changeable_p (var) 
-      && var->value && !value_lazy (var->value))
+      && var->value != nullptr && !value_lazy (var->value.get ()))
     {
-      var->print_value = varobj_value_get_print_value (var->value,
+      var->print_value = varobj_value_get_print_value (var->value.get (),
 						       var->format, var);
     }
 
@@ -1045,7 +1045,7 @@  varobj_set_value (struct varobj *var, const char *expression)
   gdb_assert (varobj_value_is_changeable_p (var));
 
   /* The value of a changeable variable object must not be lazy.  */
-  gdb_assert (!value_lazy (var->value));
+  gdb_assert (!value_lazy (var->value.get ()));
 
   /* Need to coerce the input.  We want to check if the
      value of the variable object will be different
@@ -1060,7 +1060,7 @@  varobj_set_value (struct varobj *var, const char *expression)
      rather value_contents, will take care of this.  */
   TRY
     {
-      val = value_assign (var->value, value);
+      val = value_assign (var->value.get (), value);
     }
 
   CATCH (except, RETURN_MASK_ERROR)
@@ -1112,9 +1112,9 @@  install_default_visualizer (struct varobj *var)
     {
       PyObject *pretty_printer = NULL;
 
-      if (var->value)
+      if (var->value != nullptr)
 	{
-	  pretty_printer = gdbpy_get_varobj_pretty_printer (var->value);
+	  pretty_printer = gdbpy_get_varobj_pretty_printer (var->value.get ());
 	  if (! pretty_printer)
 	    {
 	      gdbpy_print_stack ();
@@ -1149,7 +1149,8 @@  construct_visualizer (struct varobj *var, PyObject *constructor)
     pretty_printer = NULL;
   else
     {
-      pretty_printer = instantiate_pretty_printer (constructor, var->value);
+      pretty_printer = instantiate_pretty_printer (constructor,
+						   var->value.get ());
       if (! pretty_printer)
 	{
 	  gdbpy_print_stack ();
@@ -1326,8 +1327,9 @@  install_new_value (struct varobj *var, struct value *value, bool initial)
 
   /* Get a reference now, before possibly passing it to any Python
      code that might release it.  */
+  value_ref_ptr value_holder;
   if (value != NULL)
-    value_incref (value);
+    value_holder.reset (value_incref (value));
 
   /* Below, we'll be comparing string rendering of old and new
      values.  Don't get string rendering if the value is
@@ -1354,7 +1356,7 @@  install_new_value (struct varobj *var, struct value *value, bool initial)
 	{
 	  /* Try to compare the values.  That requires that both
 	     values are non-lazy.  */
-	  if (var->not_fetched && value_lazy (var->value))
+	  if (var->not_fetched && value_lazy (var->value.get ()))
 	    {
 	      /* This is a frozen varobj and the value was never read.
 		 Presumably, UI shows some "never read" indicator.
@@ -1372,7 +1374,7 @@  install_new_value (struct varobj *var, struct value *value, bool initial)
 	    }
 	  else
 	    {
-	      gdb_assert (!value_lazy (var->value));
+	      gdb_assert (!value_lazy (var->value.get ()));
 	      gdb_assert (!value_lazy (value));
 
 	      gdb_assert (!var->print_value.empty () && !print_value.empty ());
@@ -1392,9 +1394,7 @@  install_new_value (struct varobj *var, struct value *value, bool initial)
     }
 
   /* We must always keep the new value, since children depend on it.  */
-  if (var->value != NULL && var->value != value)
-    value_decref (var->value);
-  var->value = value;
+  var->value = value_holder;
   if (value && value_lazy (value) && intentionally_not_fetched)
     var->not_fetched = true;
   else
@@ -1407,8 +1407,8 @@  install_new_value (struct varobj *var, struct value *value, bool initial)
      to see if the variable changed.  */
   if (var->dynamic->pretty_printer != NULL)
     {
-      print_value = varobj_value_get_print_value (var->value, var->format,
-						  var);
+      print_value = varobj_value_get_print_value (var->value.get (),
+						  var->format, var);
       if ((var->print_value.empty () && !print_value.empty ())
 	  || (!var->print_value.empty () && print_value.empty ())
 	  || (!var->print_value.empty () && !print_value.empty ()
@@ -1417,7 +1417,7 @@  install_new_value (struct varobj *var, struct value *value, bool initial)
     }
   var->print_value = print_value;
 
-  gdb_assert (!var->value || value_type (var->value));
+  gdb_assert (var->value == nullptr || value_type (var->value.get ()));
 
   return changed;
 }
@@ -1990,7 +1990,6 @@  varobj::~varobj ()
 
   varobj_iter_delete (var->dynamic->child_iter);
   varobj_clear_saved_item (var->dynamic);
-  value_decref (var->value);
 
   if (is_root_p (var))
     delete var->root;
@@ -2014,8 +2013,8 @@  varobj_get_value_type (const struct varobj *var)
 {
   struct type *type;
 
-  if (var->value)
-    type = value_type (var->value);
+  if (var->value != nullptr)
+    type = value_type (var->value.get ());
   else
     type = var->type;
 
@@ -2261,7 +2260,8 @@  my_value_of_variable (struct varobj *var, enum varobj_display_formats format)
   if (var->root->is_valid)
     {
       if (var->dynamic->pretty_printer != NULL)
-	return varobj_value_get_print_value (var->value, var->format, var);
+	return varobj_value_get_print_value (var->value.get (), var->format,
+					     var);
       return (*var->root->lang_ops->value_of_variable) (var, format);
     }
   else
@@ -2397,7 +2397,8 @@  varobj_editable_p (const struct varobj *var)
 {
   struct type *type;
 
-  if (!(var->root->is_valid && var->value && VALUE_LVAL (var->value)))
+  if (!(var->root->is_valid && var->value != nullptr
+	&& VALUE_LVAL (var->value.get ())))
     return false;
 
   type = varobj_get_value_type (var);
diff --git a/gdb/varobj.h b/gdb/varobj.h
index 6e80d1b89e..3aba0cda67 100644
--- a/gdb/varobj.h
+++ b/gdb/varobj.h
@@ -20,6 +20,7 @@ 
 #include "symtab.h"
 #include "gdbtypes.h"
 #include "vec.h"
+#include "value.h"
 
 /* Enumeration for the format types */
 enum varobj_display_formats
@@ -122,7 +123,7 @@  struct varobj
      indicates there was an error getting this value.
      Invariant: if varobj_value_is_changeable_p (this) is non-zero, 
      the value is either NULL, or not lazy.  */
-  struct value *value = NULL;
+  value_ref_ptr value;
 
   /* The number of (immediate) children this variable has.  */
   int num_children = -1;