This is the mail archive of the gdb-patches@sourceware.org mailing list for the GDB project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

RE: [patch] Tweak output of -var-info-path-expression


> -----Original Message-----
> From: Joel Brobecker [mailto:brobecker@adacore.com] 
> Sent: Thursday, August 25, 2011 9:48 PM
> To: Marc Khouzam
> Cc: 'Tom Tromey'; 'gdb-patches@sourceware.org'
> Subject: Re: [patch] Tweak output of -var-info-path-expression
> 
> > 2011-08-23  Marc Khouzam  <marc.khouzam@ericsson.com>
> > 
> > 	PR mi/11912
> > 	* varobj.c (cplus_describe_child): Add the keyword
> > 	'class' to the output of the method when dealing 
> > 	with a cast to a base class.
> > 
> > 2011-08-23  Marc Khouzam  <marc.khouzam@ericsson.com>
> > 
> > 	PR mi/11912
> > 	* gdb.mi/mi-inheritance-syntax-error.cc: New file.
> > 	* gdb.mi/mi-inheritance-syntax-error.exp: New file.
> > 	* gdb.mi/mi-var-cp.cc: Updated for new 'class'
> > 	keyword in output of -var-info-path-expression.
> [...]
> > Do you think it is ok for the 7.3 branch?
> 
> No objection for putting it on 7.3 since Tom thinks it's OK.

Thanks guys.  It will make using GDB better for Eclipse.

> One nit:
> 
> > +		 When we are in the scope of the base class or of one
> > +		 of its children, the type field name will be 
> interpreted
> > +		 as a constructor, if it exists.  Therefore, we must
> > +		 indicate that the name is a class name by using the
> > +		 'class' keyword. PR mi/11912  */
>                                ^^^^
> Missing second space after a period. I would also follow the general
> GCS guideline that everything is a setence, and thus write something
> like: `[...] keyword.  See PR mi/11912.  */'. But that's only if you
> agree, it can be seen as a matter of preference.

Sure.  I put as you suggested: 'keyword.  See PR mi/11912  */'

Also, I noticed that in the test, I forgot to update the line:
  untested gdb11912.exp
with the new test name.  I made this change in the commit:
  untested mi-inheritance-syntax-error.exp

Committed as below to HEAD and the 7.3 branch.

Thanks!

Marc

2011-08-26  Marc Khouzam  <marc.khouzam@ericsson.com>

	PR mi/11912
	* varobj.c (cplus_describe_child): Add the keyword
	'class' to the output of the method when dealing 
	with a cast to a base class.

2011-08-26  Marc Khouzam  <marc.khouzam@ericsson.com>

	PR mi/11912
	* gdb.mi/mi-inheritance-syntax-error.cc: New file.
	* gdb.mi/mi-inheritance-syntax-error.exp: New file.
	* gdb.mi/mi-var-cp.cc: Updated for new 'class'
	keyword in output of -var-info-path-expression.


### Eclipse Workspace Patch 1.0
#P src
Index: gdb/testsuite/gdb.mi/mi-inheritance-syntax-error.cc
===================================================================
RCS file: gdb/testsuite/gdb.mi/mi-inheritance-syntax-error.cc
diff -N gdb/testsuite/gdb.mi/mi-inheritance-syntax-error.cc
--- /dev/null	1 Jan 1970 00:00:00 -0000
+++ gdb/testsuite/gdb.mi/mi-inheritance-syntax-error.cc	1 Jan 1970 00:00:00 -0000
@@ -0,0 +1,29 @@
+// Test for -var-info-path-expression syntax error
+// caused by PR 11912
+#include <string.h>
+#include <stdio.h>
+
+class A
+{
+	int a;
+};
+
+class C : public A
+{
+	public:
+		C()
+		{
+		};
+		void testLocation()
+		{
+			z = 1;
+		};
+		int z;
+};
+
+int main()
+{
+	C c;
+	c.testLocation();
+	return 0;
+}
Index: gdb/testsuite/gdb.mi/mi-inheritance-syntax-error.exp
===================================================================
RCS file: gdb/testsuite/gdb.mi/mi-inheritance-syntax-error.exp
diff -N gdb/testsuite/gdb.mi/mi-inheritance-syntax-error.exp
--- /dev/null	1 Jan 1970 00:00:00 -0000
+++ gdb/testsuite/gdb.mi/mi-inheritance-syntax-error.exp	1 Jan 1970 00:00:00 -0000
@@ -0,0 +1,73 @@
+# Copyright 2011
+# 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 3 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, see <http://www.gnu.org/licenses/>.
+
+if { [skip_cplus_tests] } { continue }
+
+load_lib mi-support.exp
+set MIFLAGS "-i=mi"
+
+gdb_exit
+if [mi_gdb_start] {
+    continue
+}
+
+set testfile mi-inheritance-syntax-error
+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 mi-inheritance-syntax-error.exp
+  return -1
+}
+
+# Test that children of derived classes are given the proper
+# path by -var-info-path-expression
+
+mi_gdb_load ${binfile}
+mi_runto C::testLocation
+
+mi_create_varobj "var1" "this" "create var for THIS"
+
+mi_list_varobj_children "var1" {
+    {var1.A A 1 A}
+    {var1.public public 1}
+} "list children of THIS"
+
+# We don't care what the result of -var-info-path-expression
+# as long as it can be used in the -data-evaluate-expression
+# that follows
+mi_gdb_test "-var-info-path-expression var1.A" \
+    "\\^done,path_expr=\".*\"" \
+    "-var-info-path-expression var1.A"
+
+# Extract the result of the -var-info-path-expression command
+# to use it in -data-evaluate-expression
+set cmd_output $expect_out(2,string)
+set path [lindex [split $cmd_output "\""] 1]
+verbose "var-info-path-expression returned $cmd_output" 2
+verbose "Using $path for data-evaluate-expression" 2
+
+
+mi_gdb_test "-data-evaluate-expression \"$path\"" \
+    "\\^done,value=\"\{a = \[0-9\]*\}\"" \
+    "-data-evaluate-expression $path"
+
+mi_gdb_exit
+return 0
Index: gdb/testsuite/gdb.mi/mi-var-cp.cc
===================================================================
RCS file: /cvs/src/src/gdb/testsuite/gdb.mi/mi-var-cp.cc,v
retrieving revision 1.13
diff -u -r1.13 mi-var-cp.cc
--- gdb/testsuite/gdb.mi/mi-var-cp.cc	1 Jan 2011 15:33:47 -0000	1.13
+++ gdb/testsuite/gdb.mi/mi-var-cp.cc	26 Aug 2011 11:59:05 -0000
@@ -157,7 +157,7 @@
        {DP.Base2 Base2 1 Base2}                         \
        {DP.public public 1}} "list children of DP"
       mi_gdb_test "-var-info-path-expression DP.Base1" \
-          "\\^done,path_expr=\"\\(\\*\\(Base1\\*\\) dp\\)\"" \
+          "\\^done,path_expr=\"\\(\\*\\(class Base1\\*\\) dp\\)\"" \
 	  "-var-info-path-expression DP.Base1"       
       mi_list_varobj_children DP.public {               \
         {DP.public.i i 0 int}                           \
@@ -172,7 +172,7 @@
         {DP.Base1.public.i i 0 int}                           \
       } "list children of DP.Base1.public"
       mi_gdb_test "-var-info-path-expression DP.Base1.public.i" \
-          "\\^done,path_expr=\"\\(\\(\\(\\*\\(Base1\\*\\) dp\\)\\).i\\)\"" \
+          "\\^done,path_expr=\"\\(\\(\\(\\*\\(class Base1\\*\\) dp\\)\\).i\\)\"" \
 	  "-var-info-path-expression DP.Base1.public.i"
 
       mi_gdb_test "-var-info-path-expression DP.public" \
@@ -185,7 +185,7 @@
        {D.Base2 Base2 1 Base2}                         \
        {D.public public 1}} "list children of D"
       mi_gdb_test "-var-info-path-expression D.Base1" \
-          "\\^done,path_expr=\"\\(\\(Base1\\) d\\)\"" \
+          "\\^done,path_expr=\"\\(\\(class Base1\\) d\\)\"" \
 	  "-var-info-path-expression D.Base1"
   :*/
   int array[4] = {1,2,3};
Index: gdb/varobj.c
===================================================================
RCS file: /cvs/src/src/gdb/varobj.c,v
retrieving revision 1.181
diff -u -r1.181 varobj.c
--- gdb/varobj.c	28 Jul 2011 10:36:40 -0000	1.181
+++ gdb/varobj.c	26 Aug 2011 11:59:05 -0000
@@ -3443,8 +3443,14 @@
 		 will create an lvalue, for all appearences, so we don't
 		 need to use more fancy:
 		         *(Base1*)(&d)
-		 construct.  */
-	      *cfull_expression = xstrprintf ("(%s(%s%s) %s)", 
+		 construct.
+
+		 When we are in the scope of the base class or of one
+		 of its children, the type field name will be interpreted
+		 as a constructor, if it exists.  Therefore, we must
+		 indicate that the name is a class name by using the
+		 'class' keyword.  See PR mi/11912  */
+	      *cfull_expression = xstrprintf ("(%s(class %s%s) %s)", 
 					      ptr, 
 					      TYPE_FIELD_NAME (type, index),
 					      ptr,


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]