Re: MI/C++/references fixup

[Vladimir Prus <ghost@cs.msu.su>]

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

Daniel Jacobowitz wrote:

> On Wed, Nov 29, 2006 at 05:41:13PM +0300, Vladimir Prus wrote:
>> +# Creates varobj named NAME for EXPRESSION.
>> +# Name cannot be "-".
>> +proc mi_create_varobj { name expression } {
> 
>> +# 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
>> +# types.
>> +proc mi_varobj_update { name expected } {
> 
>> +proc mi_check_varobj_value { name value } {
> 
> I would recommend giving these an argument for the test name, as some
> of the other helper functions do.  Otherwise, if you call them twice in
> a row with the same arguments (which your test does) then you've got
> two tests with the same name.
> 
> On Thu, Nov 30, 2006 at 10:31:07AM +0300, Vladimir Prus wrote:
>> >         * gdb.mi/mi-cpp.cpp: New file.
>> >
>> > gdb.mi/mi-var-cp.exp?
>> > gdb.mi/mi-var-cp.cc?
>> >
>> > Its for variable objects, and for consistency.  The testsuite has the
>> > directory gdb.cp and it's populated with *.cc files
>> 
>> I've no problems with 'cp' and '.cc'. I don't think that '-var-' is good
>> -- now, there is just single MI test dealing with C++, so I want a
>> testcase that will accumulate all C++ specific things, not necessary
>> related to variable objects. But I don't care much.
> 
> I'd rather keep varobj tests easily identifiable; please use Nick's
> suggestions.

Here's revised patch. Ok for HEAD?

- Volodya

        * varobj.c (varobj_create): Don't call release_value.
        (varobj_set_value): Likewise.
        (install_new_value): Call coerce_ref and release_value
        on the value. Add asserts.

        testsuite/
        * lib/mi-support.exp
        (mi_runto): Accept "()" after function name.
        (mi_create_varobj): New function.
        (mi_varobj_update): New function.
        (mi_Check_varobj_value): New function.
        * gdb.mi/mi-var-cp.exp: New file.
        * gdb.mi/mi-var-cp.cc: New file.


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

Index: varobj.c
===================================================================
RCS file: /cvs/src/src/gdb/varobj.c,v
retrieving revision 1.63
diff -u -p -r1.63 varobj.c
--- varobj.c	6 Dec 2006 09:01:50 -0000	1.63
+++ varobj.c	6 Dec 2006 09:12:26 -0000
@@ -512,10 +512,7 @@ varobj_create (char *objname,
 	   right type.  */
 	value = evaluate_type (var->root->exp);
 
-      release_value (value);
-
       var->type = value_type (value);
-
       install_new_value (var, value, 1 /* Initial assignment */);
 
       /* Set language info */
@@ -847,9 +844,7 @@ varobj_set_value (struct varobj *var, ch
 	 with catch the exception.  */
       if (!gdb_value_assign (var->value, value, &val))
 	return 0;
-
-      release_value (val);
-      
+     
       /* If the value has changed, record it, so that next -var-update can
 	 report this change.  If a variable had a value of '1', we've set it
 	 to '333' and then set again to '1', when -var-update will report this
@@ -901,7 +896,10 @@ varobj_list (struct varobj ***varlist)
    and return 0.
    Otherwise, assign the value and if type_changeable returns non-zero,
    find if the new value is different from the current value.
-   Return 1 if so, and 0 if the values are equal.  */
+   Return 1 if so, and 0 if the values are equal.  
+
+   The VALUE parameter should not be released -- the function will
+   take care of releasing it when needed.  */
 static int
 install_new_value (struct varobj *var, struct value *value, int initial)
 { 
@@ -917,6 +915,15 @@ install_new_value (struct varobj *var, s
   changeable = type_changeable (var);
   need_to_fetch = changeable;
 
+  /* We are not interested in the address of references, and given
+     that in C++ a reference is not rebindable, it cannot
+     meaningfully change.  So, get hold of the real value.  */
+  if (value)
+    {
+      value = coerce_ref (value);
+      release_value (value);
+    }
+
   if (var->type && TYPE_CODE (var->type) == TYPE_CODE_UNION)
     /* For unions, we need to fetch the value implicitly because
        of implementation of union member fetch.  When gdb
@@ -985,6 +992,8 @@ install_new_value (struct varobj *var, s
     value_free (var->value);
   var->value = value;
   var->updated = 0;
+  
+  gdb_assert (!var->value || value_type (var->value));
 
   return changed;
 }
Index: testsuite/lib/mi-support.exp
===================================================================
RCS file: /cvs/src/src/gdb/testsuite/lib/mi-support.exp,v
retrieving revision 1.33
diff -u -p -r1.33 mi-support.exp
--- testsuite/lib/mi-support.exp	24 Jul 2006 20:38:08 -0000	1.33
+++ testsuite/lib/mi-support.exp	6 Dec 2006 09:12:27 -0000
@@ -869,7 +869,7 @@ proc mi_runto {func} {
 
   set test "mi runto $func"
   mi_gdb_test "200-break-insert $func" \
-    "200\\^done,bkpt=\{number=\"\[0-9\]+\",type=\"breakpoint\",disp=\"keep\",enabled=\"y\",addr=\"$hex\",func=\"$func\",file=\".*\",line=\"\[0-9\]*\",times=\"0\"\}" \
+    "200\\^done,bkpt=\{number=\"\[0-9\]+\",type=\"breakpoint\",disp=\"keep\",enabled=\"y\",addr=\"$hex\",func=\"$func\(\\\(.*\\\)\)?\",file=\".*\",line=\"\[0-9\]*\",times=\"0\"\}" \
     "breakpoint at $func"
 
   if {![regexp {number="[0-9]+"} $expect_out(buffer) str]
@@ -998,3 +998,39 @@ proc mi0_continue_to { bkptno func args 
     mi0_execute_to "exec-continue" "breakpoint-hit\",bkptno=\"$bkptno" \
 	"$func" "$args" "$file" "$line" "" "$test"
 }
+
+# Creates varobj named NAME for EXPRESSION.
+# Name cannot be "-".
+proc mi_create_varobj { name expression testname } {
+    mi_gdb_test "-var-create $name * $expression" \
+        "\\^done,name=\"$name\",numchild=\"\[0-9\]+\",type=.*" \
+        $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
+# types.
+proc mi_varobj_update { name expected testname } {
+    set er "\\^done,changelist=\\\["
+    set first 1
+    foreach item $expected {
+        set v "{name=\"$item\",in_scope=\"true\",type_changed=\"false\"}"
+        if {$first} {
+            set er "$er$v"
+        } else {
+            set er "$er,$v"
+        }
+    }
+    set er "$er\\\]"
+
+    verbose -log "Expecting: $er" 2
+    mi_gdb_test "-var-update $name" $er $testname
+}
+
+proc mi_check_varobj_value { name value testname } {
+
+    mi_gdb_test "-var-evaluate-expression $name" \
+	"\\^done,value=\"$value\"" \
+	$testname
+}

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #3: mi-var-cp.cc --]
[-- Type: text/x-objcsrc; name="mi-var-cp.cc", Size: 915 bytes --]

/* Copyright 2006 Free Software Foundation, Inc.

   This program is free software; you can redistribute it and/or modify
   it under the terms of the GNU General Public License as published by
   the Free Software Foundation; either version 2 of the License, or
   (at your option) any later version.
   
   This program is distributed in the hope that it will be useful,
   but WITHOUT ANY WARRANTY; without even the implied warranty of
   MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
   GNU General Public License for more details.
 
   You should have received a copy of the GNU General Public License
   along with this program; if not, write to the Free Software
   Foundation, Inc., 59 Temple Place - Suite 330, Boston, MA 02111-1307, USA.  
*/

void reference_update_tests ()
{
  int x = 167;
  int& rx = x;
  x = 567;
  x = 567;
}

int main ()
{
  reference_update_tests ();
  return 0;
}

[-- Attachment #4: mi-var-cp.exp --]
[-- Type: text/plain, Size: 2133 bytes --]

# Copyright 2006 Free Software Foundation, Inc.

# This program is free software; you can redistribute it and/or modify
# it under the terms of the GNU General Public License as published by
# the Free Software Foundation; either version 2 of the License, or
# (at your option) any later version.
# 
# This program is distributed in the hope that it will be useful,
# but WITHOUT ANY WARRANTY; without even the implied warranty of
# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
# GNU General Public License for more details.
# 
# You should have received a copy of the GNU General Public License
# along with this program; if not, write to the Free Software
# Foundation, Inc., 59 Temple Place - Suite 330, Boston, MA 02111-1307, USA.  

if { [skip_cplus_tests] } { continue }

load_lib mi-support.exp
set MIFLAGS "-i=mi"

gdb_exit
if [mi_gdb_start] {
    continue
}

set testfile mi-var-cp
set srcfile "$testfile.cc"
set binfile $objdir/$subdir/$testfile

if [get_compiler_info ${binfile} "c++"] {
    return -1;
}

if {[gdb_compile $srcdir/$subdir/$srcfile $binfile executable {debug c++}] != ""} {
  untested $testfile.exp
  return -1
}

mi_gdb_load ${binfile}

# Test that children of classes are properly reported

# Run to main
mi_runto reference_update_tests

mi_create_varobj "RX" "rx" "create varobj for rx"

set x_assignment [gdb_get_line_number "x = 567;"]
mi_next_to "reference_update_tests" {} ".*${srcfile}" [expr $x_assignment-1] \
    "step to x assignment"
mi_next_to "reference_update_tests" {} ".*${srcfile}" [expr $x_assignment] \
    "step to x assignment"

mi_varobj_update RX {RX} "update RX (1)"

mi_check_varobj_value RX 167 "check RX: expect 167"

# Execute the first 'x = 567' line.
mi_next_to "reference_update_tests" {} ".*${srcfile}" [expr $x_assignment+1] \
    "step to x assignment"

mi_varobj_update RX {RX} "update RX (2)"
mi_check_varobj_value RX 567 "check RX: expect 567"

# Execute the second 'x = 567' line.
mi_next_to "reference_update_tests" {} ".*${srcfile}" [expr $x_assignment+2] \
    "step to x assignment"

mi_varobj_update RX {} "update RX (3)"

mi_gdb_exit
return 0