MI/C++/references fixup

Vladimir Prus
2006-11-29T09:15:42Z

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

As noted by Nick, my varobj laziness patch has broken C++ references
updates -- now -var-update does not detect the situation when a reference
variable just got initialized, for example:

int x = 4;
int &rx = x;

after executing the second line, -var-update does not say that varobj
corresponding to rx has change.

References were already broken, in a different way, before my
change -- -var-update would always list reference variable as changed. That
was because gdb was comparing undereferenced old value with dereferenced new
value -- that obviously never compare equal.

Also, display of references includes the address, like this:

-var-evaluate-expression var1
^done,value="@0xbff72ebc: 4"

We discussed this at length some time ago:

http://thread.gmane.org/gmane.comp.gdb.patches/28414/

The conclusion was that this "address prefix" is not necessary. In fact, both
KDevelop and Eclipse explicitly remove it. Back then, it was decided that
change would be hard. But recent varobj refactorings make this change
straight-forward.

Attached is the patch that:

- Makes -var-update property report reference variables --
only when the referred-to value actually changed.

- Removes the "@ADDRESS" output for reference variables.

- Adds tests for references.

The patch and the new files are attached.

- 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-cpp.exp: New file.
* gdb.mi/mi-cpp.cpp: New file.

[-- Attachment #2: 00REFERENCES.diff --]
[-- Type: text/x-diff, Size: 4582 bytes --]

Index: varobj.c
===================================================================
RCS file: /cvs/src/src/gdb/varobj.c,v
retrieving revision 1.62
diff -u -p -r1.62 varobj.c
--- varobj.c 29 Nov 2006 06:41:13 -0000 1.62
+++ varobj.c 29 Nov 2006 08:52:22 -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 address of references, and given
+ that in C++ 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 29 Nov 2006 08:52:23 -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,40 @@ 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 } {
+ mi_gdb_test "-var-create $name * $expression" \
+ "\\^done,name=\"$name\",numchild=\"\[0-9\]+\",type=.*" \
+ "-var-create $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 } {
+ 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 \
+ "-var-update $name: expect $expected"
+}
+
+proc mi_check_varobj_value { name value } {
+
+ mi_gdb_test "-var-evaluate-expression $name" \
+ "\\^done,value=\"$value\"" \
+ "-var-evaluate-expression $name: expect $value"
+}
\ No newline at end of file

[-- Attachment #3: mi-cpp.exp --]
[-- Type: text/plain, Size: 2116 bytes --]

# Copyright 2002, 2003 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.

# Please email any bugs, comments, and/or additions to this file to:
# bug-gdb@prep.ai.mit.edu

if { [skip_cplus_tests] } { continue }

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

gdb_exit
if [mi_gdb_start] {
continue
}

set testfile mi-cpp
set srcfile "$testfile.cpp"
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"

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}

mi_check_varobj_value RX 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}
mi_check_varobj_value RX 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 {}

mi_gdb_exit
return 0

[-- Attachment #4: mi-cpp.cpp --]
[-- Type: text/x-c++src, Size: 146 bytes --]

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

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