Simpify varobj children handling for C++

[Vladimir Prus <ghost at cs dot msu dot su>]

This patch merges the code for getting name, value and type of
varobj children for C++ into one function, just like the previously
posted patch for C.

This was manually tested from KDevelop. We really need tests
for MI C++ functionality, and I plan to write them, but only
after my patch that makes it possible to write MI tests using
just C files is approved.

Is this patch OK provided that tests will be writted before committing?

- Volodya

	Refactor getting children name, value and type access 
	for varobjs in C++.
	* value.c (value_as_address): Use coerce_array_proper
	instead of coerce_array so that not fail for references.
	(coerce_array_proper): New function.
	(coerce_array): Use the above.
	* value.h (coerce_array_proper): Declare.
	* valops.c (value_ind): Handle references.
	* varobj.c (adjust_value_for_children_access): New.
	(c_describe_child): Use the above.
	(enum accessibility): New.
	(match_accessibility): New function.
	(cplus_describe_child): New function.
	(cplus_name_of_child, cplus_value_of_child)
	(cplus_type_of_child): Reimplement in terms
	of cplus_describe_child.

--- gdb/value.c	(/patches/gdb/path_3_unify/gdb_mainline)	(revision 2936)
+++ gdb/value.c	(/patches/gdb/path_4_unify_cpp/gdb_mainline)	(revision 2936)
@@ -1006,7 +1006,7 @@ value_as_address (struct value *val)
       || TYPE_CODE (value_type (val)) == TYPE_CODE_METHOD)
     return VALUE_ADDRESS (val);
 
-  val = coerce_array (val);
+  val = coerce_array_proper (val);
 
   /* Some architectures (e.g. Harvard), map instruction and data
      addresses onto a single large unified address space.  For
@@ -1621,9 +1621,8 @@ coerce_ref (struct value *arg)
 }
 
 struct value *
-coerce_array (struct value *arg)
+coerce_array_proper (struct value *arg)
 {
-  arg = coerce_ref (arg);
   if (current_language->c_style_arrays
       && TYPE_CODE (value_type (arg)) == TYPE_CODE_ARRAY)
     arg = value_coerce_array (arg);
@@ -1632,6 +1631,13 @@ coerce_array (struct value *arg)
   return arg;
 }
 
+
+struct value *
+coerce_array (struct value *arg)
+{
+  return coerce_array_proper (coerce_ref (arg));
+}
+
 struct value *
 coerce_number (struct value *arg)
 {
--- gdb/value.h	(/patches/gdb/path_3_unify/gdb_mainline)	(revision 2936)
+++ gdb/value.h	(/patches/gdb/path_4_unify_cpp/gdb_mainline)	(revision 2936)
@@ -224,6 +224,8 @@ extern short *deprecated_value_regnum_ha
 
 extern struct value *coerce_ref (struct value *value);
 
+extern struct value *coerce_array_proper (struct value *arg);
+
 /* If ARG is an array, convert it to a pointer.
    If ARG is an enum, convert it to an integer.
    If ARG is a function, convert it to a function pointer.
--- gdb/testsuite/gdb.mi/mi-var-cp.cc	(/patches/gdb/path_3_unify/gdb_mainline)	(revision 2936)
+++ gdb/testsuite/gdb.mi/mi-var-cp.cc	(/patches/gdb/path_4_unify_cpp/gdb_mainline)	(revision 2936)
@@ -39,7 +39,6 @@ void base_in_reference_test_main ()
   base_in_reference_test (s);
 }
 
-
 int main ()
 {
   reference_update_tests ();
--- gdb/valops.c	(/patches/gdb/path_3_unify/gdb_mainline)	(revision 2936)
+++ gdb/valops.c	(/patches/gdb/path_4_unify_cpp/gdb_mainline)	(revision 2936)
@@ -950,7 +950,8 @@ value_ind (struct value *arg1)
   if (TYPE_CODE (base_type) == TYPE_CODE_INT)
     return value_at_lazy (builtin_type_int,
 			  (CORE_ADDR) value_as_long (arg1));
-  else if (TYPE_CODE (base_type) == TYPE_CODE_PTR)
+  else if (TYPE_CODE (base_type) == TYPE_CODE_PTR
+	   || TYPE_CODE (base_type) == TYPE_CODE_PTR)
     {
       struct type *enc_type;
       /* We may be pointing to something embedded in a larger object */
--- gdb/varobj.c	(/patches/gdb/path_3_unify/gdb_mainline)	(revision 2936)
+++ gdb/varobj.c	(/patches/gdb/path_4_unify_cpp/gdb_mainline)	(revision 2936)
@@ -1792,6 +1792,45 @@ value_struct_element_index (struct value
     }
 }
 
+/* Given a value and a type of a variable object,
+   adjust those value and type to those necessary
+   for getting childrens of the variable object.
+   This includes dereferencing top-level reference
+   to all types and dereferencing pointers to
+   structures.  */
+static void
+adjust_value_for_children_access (struct value **value,
+				  struct type **type)
+{
+  /* If the parent is reference, we always strip the
+     reference when getting children, since in C++,
+     reference is basically undistinguishable in
+     usage from a plain variable.  */
+
+  if (TYPE_CODE (*type) == TYPE_CODE_REF)
+    {
+      struct type *target_type = get_target_type (*type);
+      if (*value)
+	gdb_value_ind (*value, value);	  
+      *type = get_target_type (*type);
+    }
+
+  /* Pointers to structures are treated just like
+     structures when accessing children.  Don't
+     dererences pointers to other types.  */
+  if (TYPE_CODE (*type) == TYPE_CODE_PTR)
+    {
+      struct type *target_type = get_target_type (*type);
+      if (TYPE_CODE (target_type) == TYPE_CODE_STRUCT
+	  || TYPE_CODE (target_type) == TYPE_CODE_UNION)
+	{
+	  if (*value)
+	    gdb_value_ind (*value, value);	  
+	  *type = target_type;
+	}
+    }  
+}
+
 /* Obtain the information about child INDEX of the variable
    object PARENT.  
    If CNAME is not null, sets *CNAME to the name of the child relative
@@ -1816,19 +1855,7 @@ c_describe_child (struct varobj *parent,
   if (ctype)
     *ctype = NULL;
 
-  /* Pointers to structures are treated just like
-     structures when accessing children.  */
-  if (TYPE_CODE (type) == TYPE_CODE_PTR)
-    {
-      struct type *target_type = get_target_type (type);
-      if (TYPE_CODE (target_type) == TYPE_CODE_STRUCT
-	  || TYPE_CODE (target_type) == TYPE_CODE_UNION)
-	{
-	  if (value)
-	    gdb_value_ind (value, &value);	  
-	  type = target_type;
-	}
-    }
+  adjust_value_for_children_access (&value, &type);
       
   switch (TYPE_CODE (type))
     {
@@ -2128,25 +2155,56 @@ cplus_name_of_variable (struct varobj *p
   return c_name_of_variable (parent);
 }
 
-static char *
-cplus_name_of_child (struct varobj *parent, int index)
+enum accessibility { private_field, protected_field, public_field };
+
+/* Check if field INDEX of TYPE has the specified accessibility.
+   Return 0 if so and 1 otherwise.  */
+static int 
+match_accessibility (struct type *type, int index, enum accessibility acc)
+{
+  if (acc == private_field && TYPE_FIELD_PRIVATE (type, index))
+    return 1;
+  else if (acc == protected_field && TYPE_FIELD_PROTECTED (type, index))
+    return 1;
+  else if (acc == public_field && !TYPE_FIELD_PRIVATE (type, index)
+	   && !TYPE_FIELD_PROTECTED (type, index))
+    return 1;
+  else
+    return 0;
+}
+
+static void
+cplus_describe_child (struct varobj *parent, int index,
+		      char **cname, struct value **cvalue, struct type **ctype)
 {
-  char *name;
+  char *name = 0;
+  struct value *value;
   struct type *type;
 
+  if (cname)
+    *cname = NULL;
+  if (cvalue)
+    *cvalue = NULL;
+  if (ctype)
+    *ctype = NULL;
+
+
   if (CPLUS_FAKE_CHILD (parent))
     {
-      /* Looking for children of public, private, or protected. */
-      type = get_type_deref (parent->parent);
+      value = parent->parent->value;
+      type = get_type (parent->parent);
     }
   else
-    type = get_type_deref (parent);
+    {
+      value = parent->value;
+      type = get_type (parent);
+    }
 
-  name = NULL;
-  switch (TYPE_CODE (type))
+  adjust_value_for_children_access (&value, &type);
+
+  if (TYPE_CODE (type) == TYPE_CODE_STRUCT
+      || TYPE_CODE (type) == TYPE_CODE_STRUCT)
     {
-    case TYPE_CODE_STRUCT:
-    case TYPE_CODE_UNION:
       if (CPLUS_FAKE_CHILD (parent))
 	{
 	  /* The fields of the class type are ordered as they
@@ -2156,56 +2214,54 @@ cplus_name_of_child (struct varobj *pare
 	     have the access control we are looking for to properly
 	     find the indexed field. */
 	  int type_index = TYPE_N_BASECLASSES (type);
+	  enum accessibility acc = public_field;
 	  if (strcmp (parent->name, "private") == 0)
-	    {
-	      while (index >= 0)
-		{
-	  	  if (TYPE_VPTR_BASETYPE (type) == type
-	      	      && type_index == TYPE_VPTR_FIELDNO (type))
-		    ; /* ignore vptr */
-		  else if (TYPE_FIELD_PRIVATE (type, type_index))
-		    --index;
-		  ++type_index;
-		}
-	      --type_index;
-	    }
+	    acc = private_field;
 	  else if (strcmp (parent->name, "protected") == 0)
+	    acc = protected_field;
+
+	  while (index >= 0)
 	    {
-	      while (index >= 0)
-		{
-	  	  if (TYPE_VPTR_BASETYPE (type) == type
-	      	      && type_index == TYPE_VPTR_FIELDNO (type))
-		    ; /* ignore vptr */
-		  else if (TYPE_FIELD_PROTECTED (type, type_index))
+	      if (TYPE_VPTR_BASETYPE (type) == type
+		  && type_index == TYPE_VPTR_FIELDNO (type))
+		; /* ignore vptr */
+	      else if (match_accessibility (type, type_index, acc))
 		    --index;
 		  ++type_index;
-		}
-	      --type_index;
 	    }
-	  else
+	  --type_index;
+
+	  if (cname)
+	    *cname = TYPE_FIELD_NAME (type, type_index);
+
+	  if (cvalue && value)
+	    *cvalue = value_struct_element_index (value, type_index);
+
+	  if (ctype)
+	    *ctype = TYPE_FIELD_TYPE (type, type_index);
+	}
+      else if (index < TYPE_N_BASECLASSES (type))
+	{
+	  /* This is baseclass.  */
+	  if (cname)
+	    *cname = TYPE_FIELD_NAME (type, index);
+
+	  if (cvalue && value)
 	    {
-	      while (index >= 0)
-		{
-	  	  if (TYPE_VPTR_BASETYPE (type) == type
-	      	      && type_index == TYPE_VPTR_FIELDNO (type))
-		    ; /* ignore vptr */
-		  else if (!TYPE_FIELD_PRIVATE (type, type_index) &&
-		      !TYPE_FIELD_PROTECTED (type, type_index))
-		    --index;
-		  ++type_index;
-		}
-	      --type_index;
+	      *cvalue = value_cast (TYPE_FIELD_TYPE (type, index), value);
+	      release_value (*cvalue);
 	    }
 
-	  name = TYPE_FIELD_NAME (type, type_index);
+	  if (ctype)
+	    {
+	      *ctype = TYPE_FIELD_TYPE (type, index);
+	    }
 	}
-      else if (index < TYPE_N_BASECLASSES (type))
-	/* We are looking up the name of a base class */
-	name = TYPE_FIELD_NAME (type, index);
       else
 	{
+	  char *access = 0;
 	  int children[3];
-	  cplus_class_num_children(type, children);
+	  cplus_class_num_children (type, children);
 
 	  /* Everything beyond the baseclasses can
 	     only be "public", "private", or "protected"
@@ -2217,46 +2273,49 @@ cplus_name_of_child (struct varobj *pare
 	    {
 	    case 0:
 	      if (children[v_public] > 0)
-	 	name = "public";
+	 	access = "public";
 	      else if (children[v_private] > 0)
-	 	name = "private";
+	 	access = "private";
 	      else 
-	 	name = "protected";
+	 	access = "protected";
 	      break;
 	    case 1:
 	      if (children[v_public] > 0)
 		{
 		  if (children[v_private] > 0)
-		    name = "private";
+		    access = "private";
 		  else
-		    name = "protected";
+		    access = "protected";
 		}
 	      else if (children[v_private] > 0)
-	 	name = "protected";
+	 	access = "protected";
 	      break;
 	    case 2:
 	      /* Must be protected */
-	      name = "protected";
+	      access = "protected";
 	      break;
 	    default:
 	      /* error! */
 	      break;
 	    }
-	}
-      break;
+	  
+	  if (cname)
+	    *cname = access;
 
-    default:
-      break;
+	  /* Value and type are null here.  */
+	}
     }
-
-  if (name == NULL)
-    return c_name_of_child (parent, index);
   else
     {
-      if (name != NULL)
-	name = savestring (name, strlen (name));
-    }
+      c_describe_child (parent, index, cname, cvalue, ctype);
+    }  
+}
 
+static char *
+cplus_name_of_child (struct varobj *parent, int index)
+{
+  char *name = NULL;
+  cplus_describe_child (parent, index, &name, NULL, NULL);
   return name;
 }
 
@@ -2269,118 +2328,16 @@ cplus_value_of_root (struct varobj **var
 static struct value *
 cplus_value_of_child (struct varobj *parent, int index)
 {
-  struct type *type;
-  struct value *value;
-
-  if (CPLUS_FAKE_CHILD (parent))
-    type = get_type_deref (parent->parent);
-  else
-    type = get_type_deref (parent);
-
-  value = NULL;
-
-  if (((TYPE_CODE (type)) == TYPE_CODE_STRUCT) ||
-      ((TYPE_CODE (type)) == TYPE_CODE_UNION))
-    {
-      if (CPLUS_FAKE_CHILD (parent))
-	{
-	  char *name;
-	  struct value *temp = parent->parent->value;
-
-	  if (temp == NULL)
-	    return NULL;
-
-	  name = name_of_child (parent, index);
-	  gdb_value_struct_elt (NULL, &value, &temp, NULL, name, NULL,
-				"cplus_structure");
-	  if (value != NULL)
-	    release_value (value);
-
-	  xfree (name);
-	}
-      else if (index >= TYPE_N_BASECLASSES (type))
-	{
-	  /* public, private, or protected */
-	  return NULL;
-	}
-      else
-	{
-	  /* Baseclass */
-	  if (parent->value != NULL)
-	    {
-	      struct value *temp = NULL;
-
-	      /* No special processing for references is needed --
-		 value_cast below handles references.  */
-	      if (TYPE_CODE (value_type (parent->value)) == TYPE_CODE_PTR)
-		{
-		  if (!gdb_value_ind (parent->value, &temp))
-		    return NULL;
-		}
-	      else
-		temp = parent->value;
-
-	      if (temp != NULL)
-		{
-		  value = value_cast (TYPE_FIELD_TYPE (type, index), temp);
-		  release_value (value);
-		}
-	      else
-		{
-		  /* We failed to evaluate the parent's value, so don't even
-		     bother trying to evaluate this child. */
-		  return NULL;
-		}
-	    }
-	}
-    }
-
-  if (value == NULL)
-    return c_value_of_child (parent, index);
-
+  struct value *value = NULL;
+  cplus_describe_child (parent, index, NULL, &value, NULL);
   return value;
 }
 
 static struct type *
 cplus_type_of_child (struct varobj *parent, int index)
 {
-  struct type *type, *t;
-
-  if (CPLUS_FAKE_CHILD (parent))
-    {
-      /* Looking for the type of a child of public, private, or protected. */
-      t = get_type_deref (parent->parent);
-    }
-  else
-    t = get_type_deref (parent);
-
-  type = NULL;
-  switch (TYPE_CODE (t))
-    {
-    case TYPE_CODE_STRUCT:
-    case TYPE_CODE_UNION:
-      if (CPLUS_FAKE_CHILD (parent))
-	{
-	  char *name = cplus_name_of_child (parent, index);
-	  type = lookup_struct_elt_type (t, name, 0);
-	  xfree (name);
-	}
-      else if (index < TYPE_N_BASECLASSES (t))
-	type = TYPE_FIELD_TYPE (t, index);
-      else
-	{
-	  /* special */
-	  return NULL;
-	}
-      break;
-
-    default:
-      break;
-    }
-
-  if (type == NULL)
-    return c_type_of_child (parent, index);
-
+  struct type *type = NULL;
+  cplus_describe_child (parent, index, NULL, NULL, &type);
   return type;
 }
 

Property changes on: 
___________________________________________________________________
Name: csl:base
 -/all/patches/gdb/path_2_children/gdb_mainline
 +/all/patches/gdb/path_3_unify/gdb_mainline
Name: svk:merge
  d48a11ec-ee1c-0410-b3f5-c20844f99675:/patches/gdb/path_1/gdb_mainline:2869
 +d48a11ec-ee1c-0410-b3f5-c20844f99675:/patches/gdb/path_3_unify/gdb_mainline:2930
  d48a11ec-ee1c-0410-b3f5-c20844f99675:/patches/gdb/path_expression/gdb_mainline:2562
  e7755896-6108-0410-9592-8049d3e74e28:/mirrors/gdb/trunk:157978