Mirror of the gdb-patches mailing list
 help / color / mirror / Atom feed
* MI/C++/references fixup
@ 2006-11-29  9:15 Vladimir Prus
  2006-11-29  9:25 ` Vladimir Prus
  2006-11-29 14:01 ` Daniel Jacobowitz
  0 siblings, 2 replies; 12+ messages in thread
From: Vladimir Prus @ 2006-11-29  9:15 UTC (permalink / raw)
  To: gdb-patches; +Cc: Nick Roberts

[-- 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;
}

^ permalink raw reply	[flat|nested] 12+ messages in thread
end of thread, other threads:[~2006-12-08 12:45 UTC | newest]

Thread overview: 12+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2006-11-29  9:15 MI/C++/references fixup Vladimir Prus
2006-11-29  9:25 ` Vladimir Prus
2006-11-29 14:01 ` Daniel Jacobowitz
2006-11-29 14:15   ` Vladimir Prus
2006-11-29 14:24     ` Daniel Jacobowitz
2006-11-29 14:41       ` Vladimir Prus
2006-11-29 23:59         ` Nick Roberts
2006-11-30  7:31           ` Vladimir Prus
2006-12-05 21:11       ` Daniel Jacobowitz
2006-12-06  9:17         ` Vladimir Prus
2006-12-07 14:39           ` Daniel Jacobowitz
2006-12-08 12:45             ` Vladimir Prus

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox