[mi] fix 'editable' attribute for references

[mi] fix 'editable' attribute for references

[Vladimir Prus]

[-- Attachment #1: Type: text/plain, Size: 1091 bytes --]


I've noticed that the -var-show-references returns 'attr=noneditable' for structures,
but 'attr=editable' for references to structures, which makes no sense.

This patch fixes this.

Some clarification is is order. Each varobj has three types associated with it.
One type is the declared type in the source program. Second type
is the type of the value we're actually storing. Third type is the type of value
we're operating with when getting children.

To be more concrete, first type is the type in program, no changes at all.
Second type is type in program with top-level reference stripped.
Third type also has top-level pointer dereferenced. 

This patch introduces, explicitly, a function to get the second type
and makes c_variable_ediatable use it.

OK?

- Volodya

	Fix the 'editable' attribute computation
	for references.

	testsuite/
	* gdb.mi/mi-var-cp.exp: Run the reference_to_struct
	testcase.
	* gdb.mi/mi-var-cp.cc (reference_to_struct): New function.
	(main): Call the above.

	gdb/
	* varobj.c (get_value_type): New function.
	(c_variable_editable): Use get_value_type.

[-- Attachment #2: path_3_5_references__gdb_mainline.diff --]
[-- Type: text/x-diff, Size: 3012 bytes --]

--- gdb/testsuite/gdb.mi/mi-var-cp.exp	(/mirrors/gdb_mainline)	(revision 3171)
+++ gdb/testsuite/gdb.mi/mi-var-cp.exp	(/patches/gdb/path_3_5_references/gdb_mainline)	(revision 3171)
@@ -44,6 +44,7 @@ mi_prepare_inline_tests $srcfile
 mi_run_inline_test reference_update
 mi_run_inline_test base_in_reference
 mi_run_inline_test reference_to_pointer
+mi_run_inline_test reference_to_struct
 
 mi_gdb_exit
 return 0
--- gdb/testsuite/gdb.mi/mi-var-cp.cc	(/mirrors/gdb_mainline)	(revision 3171)
+++ gdb/testsuite/gdb.mi/mi-var-cp.cc	(/patches/gdb/path_3_5_references/gdb_mainline)	(revision 3171)
@@ -96,10 +96,30 @@ int reference_to_pointer ()
   /*: END: reference_to_pointer :*/
 }
 
+int reference_to_struct ()
+{
+  /*: BEGIN: reference_to_struct :*/
+  S s;
+  S& r = s;
+  /*:
+    mi_create_varobj S s "create varobj for s"
+    mi_create_varobj R r "create varobj for s"
+    mi_gdb_test "-var-show-attributes S" \
+	"\\^done,attr=\"noneditable\"" \
+	"check attributes of S"
+    mi_gdb_test "-var-show-attributes R" \
+	"\\^done,attr=\"noneditable\"" \
+	"check attributes of R"
+    :*/
+  return 99;
+  /*: END: reference_to_struct :*/
+}
+
 int main ()
 {
   reference_update_tests ();
   base_in_reference_test_main ();
   reference_to_pointer ();
+  reference_to_struct ();
   return 0;
 }
--- gdb/varobj.c	(/mirrors/gdb_mainline)	(revision 3171)
+++ gdb/varobj.c	(/patches/gdb/path_3_5_references/gdb_mainline)	(revision 3171)
@@ -176,6 +176,8 @@ static struct cleanup *make_cleanup_free
 
 static struct type *get_type (struct varobj *var);
 
+static struct type *get_value_type (struct varobj *var);
+
 static struct type *get_type_deref (struct varobj *var);
 
 static struct type *get_target_type (struct type *);
@@ -1459,6 +1461,35 @@ get_type (struct varobj *var)
   return type;
 }
 
+/* Return the type of the value that's stored in VAR,
+   or that would have being stored there if the
+   value were accessible.  
+
+   This differs from VAR->type in that VAR->type is always
+   the true type of the expession in the source language.
+   The return value of this function is the type we're
+   actually storing in varobj, and using for displaying
+   the values and for comparing previous and new values.
+
+   For example, top-level references are always stripped.  */
+static struct type *
+get_value_type (struct varobj *var)
+{
+  struct type *type;
+
+  if (var->value)
+    type = value_type (var->value);
+  else
+    type = var->type;
+
+  if (TYPE_CODE (type) == TYPE_CODE_REF)
+    type = get_target_type (type);
+
+  type = check_typedef (type);
+
+  return type;
+}
+
 /* This returns the type of the variable, dereferencing references, pointers
    and references to pointers, too. */
 static struct type *
@@ -2020,7 +2051,7 @@ c_type_of_child (struct varobj *parent, 
 static int
 c_variable_editable (struct varobj *var)
 {
-  switch (TYPE_CODE (get_type (var)))
+  switch (TYPE_CODE (get_value_type (var)))
     {
     case TYPE_CODE_STRUCT:
     case TYPE_CODE_UNION:

Re: [mi] fix 'editable' attribute for references

[Vladimir Prus]

[-- Attachment #1: Type: text/plain, Size: 1032 bytes --]

Vladimir Prus wrote:

> 
> I've noticed that the -var-show-references returns 'attr=noneditable' for
> structures, but 'attr=editable' for references to structures, which makes
> no sense.

I've also noticed that while for structures, we don't compare old 
and new values, and never report varobj corresponding to structure as
changed, we do all that for references to structures. This patch fixes
both problems, OK?

- Volodya

        Fix the 'editable' attribute computation
        for references.

        testsuite/
        * lib/mi-support.exp (mi_delete_varobj): New.
        * gdb.mi/mi-var-cp.exp: Run the reference_to_struct
        testcase.
        * gdb.mi/mi-var-cp.cc (reference_to_struct): New function.
        (main): Call the above.
        (reference_update_test, base_in_reference_test)
        (reference_to_pointer): Delete the created varobjs.

        gdb/
        * varobj.c (get_value_type): New function.
        (c_variable_editable): Use get_value_type.
        (varobj_value_is_changeable): Likewise.

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: path_3_5_references__gdb_mainline.diff --]
[-- Type: text/x-diff; name="path_3_5_references__gdb_mainline.diff", Size: 4666 bytes --]

--- gdb/testsuite/gdb.mi/mi-var-cp.exp	(/mirrors/gdb_mainline)	(revision 3174)
+++ gdb/testsuite/gdb.mi/mi-var-cp.exp	(/patches/gdb/path_3_5_references/gdb_mainline)	(revision 3174)
@@ -44,6 +44,7 @@ mi_prepare_inline_tests $srcfile
 mi_run_inline_test reference_update
 mi_run_inline_test base_in_reference
 mi_run_inline_test reference_to_pointer
+mi_run_inline_test reference_to_struct
 
 mi_gdb_exit
 return 0
--- gdb/testsuite/gdb.mi/mi-var-cp.cc	(/mirrors/gdb_mainline)	(revision 3174)
+++ gdb/testsuite/gdb.mi/mi-var-cp.cc	(/patches/gdb/path_3_5_references/gdb_mainline)	(revision 3174)
@@ -30,6 +30,7 @@ void reference_update_tests ()
       :*/  
   x = 567;
   /*: mi_varobj_update RX {} "update RX (3)"
+      mi_delete_varobj RX "delete RX"
     :*/
   /* Dummy assignment to keep 'x' in scope.  */
   x = 444;    
@@ -59,7 +60,8 @@ int base_in_reference_test (S2& s2)
 
     mi_check_varobj_value "S2.S.public.i" "67" "check S2.S.public.i"
     mi_check_varobj_value "S2.S.public.j" "89" "check S2.S.public.j"
-
+    mi_delete_varobj S2 "delete S2"
+    
   :*/
   /*: END: base_in_reference :*/
 }
@@ -91,15 +93,39 @@ int reference_to_pointer ()
 
     mi_check_varobj_value RPTR.public.i 67 "check i member"
     mi_check_varobj_value RPTR.public.j 89 "check j member"
+    mi_delete_varobj RPTR "delete RPTR"
   :*/
   return 99;
   /*: END: reference_to_pointer :*/
 }
 
+int reference_to_struct ()
+{
+  /*: BEGIN: reference_to_struct :*/
+  S s = {7, 8};
+  S& r = s;
+  /*:
+    mi_create_varobj S s "create varobj for s"
+    mi_create_varobj R r "create varobj for s"
+    mi_gdb_test "-var-show-attributes S" \
+	"\\^done,attr=\"noneditable\"" \
+	"check attributes of S"
+    mi_gdb_test "-var-show-attributes R" \
+	"\\^done,attr=\"noneditable\"" \
+	"check attributes of R"
+    :*/
+  s.i = 56;
+  /*: mi_varobj_update * [] "-var-update should not list structure varobjs"
+    :*/
+  return 99;
+  /*: END: reference_to_struct :*/
+}
+
 int main ()
 {
   reference_update_tests ();
   base_in_reference_test_main ();
   reference_to_pointer ();
+  reference_to_struct ();
   return 0;
 }
--- gdb/testsuite/lib/mi-support.exp	(/mirrors/gdb_mainline)	(revision 3174)
+++ gdb/testsuite/lib/mi-support.exp	(/patches/gdb/path_3_5_references/gdb_mainline)	(revision 3174)
@@ -1023,6 +1023,13 @@ proc mi_create_varobj { name expression 
         $testname
 }
 
+# Deletes the specified NAME. 
+proc mi_delete_varobj { name testname } {
+    mi_gdb_test "-var-delete $name" \
+        "\\^done,ndeleted=.*" \
+        $testname
+}
+
 # Updates varobj named NAME and checks that all varobjs in EXPECTED
 # are reported as updated, and no other varobj is updated.
 # Assumes that no varobj is out of scope and that no varobj changes
--- gdb/varobj.c	(/mirrors/gdb_mainline)	(revision 3174)
+++ gdb/varobj.c	(/patches/gdb/path_3_5_references/gdb_mainline)	(revision 3174)
@@ -176,6 +176,8 @@ static struct cleanup *make_cleanup_free
 
 static struct type *get_type (struct varobj *var);
 
+static struct type *get_value_type (struct varobj *var);
+
 static struct type *get_type_deref (struct varobj *var);
 
 static struct type *get_target_type (struct type *);
@@ -1459,6 +1461,35 @@ get_type (struct varobj *var)
   return type;
 }
 
+/* Return the type of the value that's stored in VAR,
+   or that would have being stored there if the
+   value were accessible.  
+
+   This differs from VAR->type in that VAR->type is always
+   the true type of the expession in the source language.
+   The return value of this function is the type we're
+   actually storing in varobj, and using for displaying
+   the values and for comparing previous and new values.
+
+   For example, top-level references are always stripped.  */
+static struct type *
+get_value_type (struct varobj *var)
+{
+  struct type *type;
+
+  if (var->value)
+    type = value_type (var->value);
+  else
+    type = var->type;
+
+  if (TYPE_CODE (type) == TYPE_CODE_REF)
+    type = get_target_type (type);
+
+  type = check_typedef (type);
+
+  return type;
+}
+
 /* This returns the type of the variable, dereferencing references, pointers
    and references to pointers, too. */
 static struct type *
@@ -1723,7 +1754,7 @@ varobj_value_is_changeable_p (struct var
   if (CPLUS_FAKE_CHILD (var))
     return 0;
 
-  type = get_type (var);
+  type = get_value_type (var);
 
   switch (TYPE_CODE (type))
     {
@@ -2020,7 +2051,7 @@ c_type_of_child (struct varobj *parent, 
 static int
 c_variable_editable (struct varobj *var)
 {
-  switch (TYPE_CODE (get_type (var)))
+  switch (TYPE_CODE (get_value_type (var)))
     {
     case TYPE_CODE_STRUCT:
     case TYPE_CODE_UNION:

Re: [mi] fix 'editable' attribute for references

[Daniel Jacobowitz]

On Tue, Jan 16, 2007 at 10:06:30PM +0300, Vladimir Prus wrote:
> Vladimir Prus wrote:
> 
> > 
> > I've noticed that the -var-show-references returns 'attr=noneditable' for
> > structures, but 'attr=editable' for references to structures, which makes
> > no sense.
> 
> I've also noticed that while for structures, we don't compare old 
> and new values, and never report varobj corresponding to structure as
> changed, we do all that for references to structures. This patch fixes
> both problems, OK?

This is included in the C++ references patch I just reviewed, right?


-- 
Daniel Jacobowitz
CodeSourcery

Re: [mi] fix 'editable' attribute for references

[Vladimir Prus]

Daniel Jacobowitz wrote:

> On Tue, Jan 16, 2007 at 10:06:30PM +0300, Vladimir Prus wrote:
>> Vladimir Prus wrote:
>> 
>> > 
>> > I've noticed that the -var-show-references returns 'attr=noneditable'
>> > for structures, but 'attr=editable' for references to structures, which
>> > makes no sense.
>> 
>> I've also noticed that while for structures, we don't compare old
>> and new values, and never report varobj corresponding to structure as
>> changed, we do all that for references to structures. This patch fixes
>> both problems, OK?
> 
> This is included in the C++ references patch I just reviewed, right?

Grr, right. I meant to post them as completely separate patches to ease review,
but messed that up and you've reviewed combined patch.

My only excuse is that I'll still commit those as separate patches, so that
at least the history is right.

- Volodya

Re: [mi] fix 'editable' attribute for references

[Vladimir Prus]

[-- Attachment #1: Type: text/plain, Size: 726 bytes --]

Vladimir Prus wrote:

>>> I've also noticed that while for structures, we don't compare old
>>> and new values, and never report varobj corresponding to structure as
>>> changed, we do all that for references to structures. This patch fixes
>>> both problems, OK?
>> 
>> This is included in the C++ references patch I just reviewed, right?
> 
> Grr, right. I meant to post them as completely separate patches to ease
> review, but messed that up and you've reviewed combined patch.
> 
> My only excuse is that I'll still commit those as separate patches, so
> that at least the history is right.

Here's the version of the patch that was just checked in. Differs only
by extra check_typedef, per your other email.

- Volodya


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: path_3_5_as_checked_in.diff --]
[-- Type: text/x-diff; name="path_3_5_as_checked_in.diff", Size: 6801 bytes --]

Index: ChangeLog
===================================================================
RCS file: /cvs/src/src/gdb/ChangeLog,v
retrieving revision 1.8134
diff -u -p -r1.8134 ChangeLog
--- ChangeLog	24 Jan 2007 00:03:15 -0000	1.8134
+++ ChangeLog	24 Jan 2007 10:48:19 -0000
@@ -1,3 +1,11 @@
+2007-01-24  Vladimir Prus  <vladimir@codesourcery.com>
+
+	Fix computation of the 'editable' attribute and
+	value changeability for for references.
+	* varobj.c (get_value_type): New function.
+	(c_variable_editable): Use get_value_type.
+	(varobj_value_is_changeable): Likewise.
+		
 2007-01-24  Joel Brobecker  <brobecker@adacore.com>
 
 	* source.c (find_and_open_source): Try rewriting the source
Index: varobj.c
===================================================================
RCS file: /cvs/src/src/gdb/varobj.c,v
retrieving revision 1.79
diff -u -p -r1.79 varobj.c
--- varobj.c	16 Jan 2007 02:12:49 -0000	1.79
+++ varobj.c	24 Jan 2007 10:48:20 -0000
@@ -176,6 +176,8 @@ static struct cleanup *make_cleanup_free
 
 static struct type *get_type (struct varobj *var);
 
+static struct type *get_value_type (struct varobj *var);
+
 static struct type *get_type_deref (struct varobj *var);
 
 static struct type *get_target_type (struct type *);
@@ -1459,6 +1461,37 @@ get_type (struct varobj *var)
   return type;
 }
 
+/* Return the type of the value that's stored in VAR,
+   or that would have being stored there if the
+   value were accessible.  
+
+   This differs from VAR->type in that VAR->type is always
+   the true type of the expession in the source language.
+   The return value of this function is the type we're
+   actually storing in varobj, and using for displaying
+   the values and for comparing previous and new values.
+
+   For example, top-level references are always stripped.  */
+static struct type *
+get_value_type (struct varobj *var)
+{
+  struct type *type;
+
+  if (var->value)
+    type = value_type (var->value);
+  else
+    type = var->type;
+
+  type = check_typedef (type);
+
+  if (TYPE_CODE (type) == TYPE_CODE_REF)
+    type = get_target_type (type);
+
+  type = check_typedef (type);
+
+  return type;
+}
+
 /* This returns the type of the variable, dereferencing references, pointers
    and references to pointers, too. */
 static struct type *
@@ -1723,7 +1756,7 @@ varobj_value_is_changeable_p (struct var
   if (CPLUS_FAKE_CHILD (var))
     return 0;
 
-  type = get_type (var);
+  type = get_value_type (var);
 
   switch (TYPE_CODE (type))
     {
@@ -2020,7 +2053,7 @@ c_type_of_child (struct varobj *parent, 
 static int
 c_variable_editable (struct varobj *var)
 {
-  switch (TYPE_CODE (get_type (var)))
+  switch (TYPE_CODE (get_value_type (var)))
     {
     case TYPE_CODE_STRUCT:
     case TYPE_CODE_UNION:
Index: testsuite/ChangeLog
===================================================================
RCS file: /cvs/src/src/gdb/testsuite/ChangeLog,v
retrieving revision 1.1329
diff -u -p -r1.1329 ChangeLog
--- testsuite/ChangeLog	23 Jan 2007 17:11:54 -0000	1.1329
+++ testsuite/ChangeLog	24 Jan 2007 10:48:26 -0000
@@ -1,3 +1,13 @@
+2007-01-24  Vladimir Prus  <vladimir@codesourcery.com>
+
+	* lib/mi-support.exp (mi_delete_varobj): New.
+	* gdb.mi/mi-var-cp.exp: Run the reference_to_struct
+	testcase.
+	* gdb.mi/mi-var-cp.cc (reference_to_struct): New function.
+	(main): Call the above.
+	(reference_update_test, base_in_reference_test)
+	(reference_to_pointer): Delete the created varobjs.
+	
 2007-01-23  Daniel Jacobowitz  <dan@codesourcery.com>
 
 	* Makefile.in (ALL_SUBDIRS): Add gdb.xml.
Index: testsuite/gdb.mi/mi-var-cp.cc
===================================================================
RCS file: /cvs/src/src/gdb/testsuite/gdb.mi/mi-var-cp.cc,v
retrieving revision 1.6
diff -u -p -r1.6 mi-var-cp.cc
--- testsuite/gdb.mi/mi-var-cp.cc	11 Jan 2007 20:48:49 -0000	1.6
+++ testsuite/gdb.mi/mi-var-cp.cc	24 Jan 2007 10:48:26 -0000
@@ -30,6 +30,7 @@ void reference_update_tests ()
       :*/  
   x = 567;
   /*: mi_varobj_update RX {} "update RX (3)"
+      mi_delete_varobj RX "delete RX"
     :*/
   /* Dummy assignment to keep 'x' in scope.  */
   x = 444;    
@@ -59,7 +60,8 @@ int base_in_reference_test (S2& s2)
 
     mi_check_varobj_value "S2.S.public.i" "67" "check S2.S.public.i"
     mi_check_varobj_value "S2.S.public.j" "89" "check S2.S.public.j"
-
+    mi_delete_varobj S2 "delete S2"
+    
   :*/
   /*: END: base_in_reference :*/
 }
@@ -91,15 +93,39 @@ int reference_to_pointer ()
 
     mi_check_varobj_value RPTR.public.i 67 "check i member"
     mi_check_varobj_value RPTR.public.j 89 "check j member"
+    mi_delete_varobj RPTR "delete RPTR"
   :*/
   return 99;
   /*: END: reference_to_pointer :*/
 }
 
+int reference_to_struct ()
+{
+  /*: BEGIN: reference_to_struct :*/
+  S s = {7, 8};
+  S& r = s;
+  /*:
+    mi_create_varobj S s "create varobj for s"
+    mi_create_varobj R r "create varobj for s"
+    mi_gdb_test "-var-show-attributes S" \
+	"\\^done,attr=\"noneditable\"" \
+	"check attributes of S"
+    mi_gdb_test "-var-show-attributes R" \
+	"\\^done,attr=\"noneditable\"" \
+	"check attributes of R"
+    :*/
+  s.i = 56;
+  /*: mi_varobj_update * [] "-var-update should not list structure varobjs"
+    :*/
+  return 99;
+  /*: END: reference_to_struct :*/
+}
+
 int main ()
 {
   reference_update_tests ();
   base_in_reference_test_main ();
   reference_to_pointer ();
+  reference_to_struct ();
   return 0;
 }
Index: testsuite/gdb.mi/mi-var-cp.exp
===================================================================
RCS file: /cvs/src/src/gdb/testsuite/gdb.mi/mi-var-cp.exp,v
retrieving revision 1.5
diff -u -p -r1.5 mi-var-cp.exp
--- testsuite/gdb.mi/mi-var-cp.exp	9 Jan 2007 17:59:13 -0000	1.5
+++ testsuite/gdb.mi/mi-var-cp.exp	24 Jan 2007 10:48:26 -0000
@@ -44,6 +44,7 @@ mi_prepare_inline_tests $srcfile
 mi_run_inline_test reference_update
 mi_run_inline_test base_in_reference
 mi_run_inline_test reference_to_pointer
+mi_run_inline_test reference_to_struct
 
 mi_gdb_exit
 return 0
Index: testsuite/lib/mi-support.exp
===================================================================
RCS file: /cvs/src/src/gdb/testsuite/lib/mi-support.exp,v
retrieving revision 1.39
diff -u -p -r1.39 mi-support.exp
--- testsuite/lib/mi-support.exp	9 Jan 2007 17:59:14 -0000	1.39
+++ testsuite/lib/mi-support.exp	24 Jan 2007 10:48:27 -0000
@@ -1023,6 +1023,13 @@ proc mi_create_varobj { name expression 
         $testname
 }
 
+# Deletes the specified NAME. 
+proc mi_delete_varobj { name testname } {
+    mi_gdb_test "-var-delete $name" \
+        "\\^done,ndeleted=.*" \
+        $testname
+}
+
 # Updates varobj named NAME and checks that all varobjs in EXPECTED
 # are reported as updated, and no other varobj is updated.
 # Assumes that no varobj is out of scope and that no varobj changes