[review] Style field names in "print"

Message ID gerrit.1574617029000.I070e1293c6cc830c9ea916af8243410aa384e944@gnutoolchain-gerrit.osci.io
State New, archived
Headers

Commit Message

Simon Marchi (Code Review) Nov. 24, 2019, 5:37 p.m. UTC
  Change URL: https://gnutoolchain-gerrit.osci.io/r/c/binutils-gdb/+/702
......................................................................

Style field names in "print"

This changes gdb to use the "variable" style when printing field
names.  I've added new tests for C and Rust, but not other languages.

I chose "variable" because that seemed most straightforward.  However,
another option would be to introduce a new "field" style.  Similarly,
this patch uses the variable style for enumerator constants -- but
again, a new style could be used if that's preferred.

gdb/ChangeLog
2019-11-24  Tom Tromey  <tom@tromey.com>

	* valprint.c (generic_val_print_enum_1)
	(val_print_type_code_flags): Style member names.
	* rust-lang.c (val_print_struct, rust_print_enum)
	(rust_print_struct_def, rust_internal_print_type): Style member
	names.
	* p-valprint.c (pascal_object_print_value_fields): Style member
	names.  Only call fprintf_symbol_filtered for static members.
	* m2-typeprint.c (m2_record_fields, m2_enum): Style member names.
	* f-valprint.c (f_val_print): Style member names.
	* f-typeprint.c (f_type_print_base): Style member names.
	* cp-valprint.c (cp_print_value_fields): Style member names.  Only
	call fprintf_symbol_filtered for static members.
	(cp_print_class_member): Style member names.
	* c-typeprint.c (c_print_type_1, c_type_print_base_1): Style
	member names.
	* ada-valprint.c (ada_print_scalar): Style enum names.
	(ada_val_print_enum): Likewise.
	* ada-typeprint.c (print_enum_type): Style enum names.

gdb/testsuite/ChangeLog
2019-11-24  Tom Tromey  <tom@tromey.com>

	* gdb.rust/rust-style.rs: New file.
	* gdb.rust/rust-style.exp: New file.
	* gdb.base/style.exp: Test structure printing.
	* gdb.base/style.c (struct some_struct): New type.
	(enum etype): New type.
	(struct_value): New global.

Change-Id: I070e1293c6cc830c9ea916af8243410aa384e944
---
M gdb/ChangeLog
M gdb/ada-typeprint.c
M gdb/ada-valprint.c
M gdb/c-typeprint.c
M gdb/cp-valprint.c
M gdb/f-typeprint.c
M gdb/f-valprint.c
M gdb/m2-typeprint.c
M gdb/p-valprint.c
M gdb/rust-lang.c
M gdb/testsuite/ChangeLog
M gdb/testsuite/gdb.base/style.c
M gdb/testsuite/gdb.base/style.exp
A gdb/testsuite/gdb.rust/rust-style.exp
A gdb/testsuite/gdb.rust/rust-style.rs
M gdb/valprint.c
16 files changed, 190 insertions(+), 32 deletions(-)
  

Comments

Tom Tromey Feb. 22, 2020, 5:11 p.m. UTC | #1
>>>>> "Tom" == Tom Tromey (Code Review) <gerrit@gnutoolchain-gerrit.osci.io> writes:

Tom> Change URL: https://gnutoolchain-gerrit.osci.io/r/c/binutils-gdb/+/702
Tom> ......................................................................

Tom> Style field names in "print"

Tom> This changes gdb to use the "variable" style when printing field
Tom> names.  I've added new tests for C and Rust, but not other languages.

Tom> I chose "variable" because that seemed most straightforward.  However,
Tom> another option would be to introduce a new "field" style.  Similarly,
Tom> this patch uses the variable style for enumerator constants -- but
Tom> again, a new style could be used if that's preferred.

I am checking this in now.

Tom
  

Patch

diff --git a/gdb/ChangeLog b/gdb/ChangeLog
index 65da2ff..cb001b2 100644
--- a/gdb/ChangeLog
+++ b/gdb/ChangeLog
@@ -1,3 +1,24 @@ 
+2019-11-24  Tom Tromey  <tom@tromey.com>
+
+	* valprint.c (generic_val_print_enum_1)
+	(val_print_type_code_flags): Style member names.
+	* rust-lang.c (val_print_struct, rust_print_enum)
+	(rust_print_struct_def, rust_internal_print_type): Style member
+	names.
+	* p-valprint.c (pascal_object_print_value_fields): Style member
+	names.  Only call fprintf_symbol_filtered for static members.
+	* m2-typeprint.c (m2_record_fields, m2_enum): Style member names.
+	* f-valprint.c (f_val_print): Style member names.
+	* f-typeprint.c (f_type_print_base): Style member names.
+	* cp-valprint.c (cp_print_value_fields): Style member names.  Only
+	call fprintf_symbol_filtered for static members.
+	(cp_print_class_member): Style member names.
+	* c-typeprint.c (c_print_type_1, c_type_print_base_1): Style
+	member names.
+	* ada-valprint.c (ada_print_scalar): Style enum names.
+	(ada_val_print_enum): Likewise.
+	* ada-typeprint.c (print_enum_type): Style enum names.
+
 2019-11-22  Tom Tromey  <tom@tromey.com>
 
 	* observable.h: Update comments.
diff --git a/gdb/ada-typeprint.c b/gdb/ada-typeprint.c
index f89dd23..edbbe96 100644
--- a/gdb/ada-typeprint.c
+++ b/gdb/ada-typeprint.c
@@ -326,7 +326,8 @@ 
       if (i)
 	fprintf_filtered (stream, ", ");
       wrap_here ("    ");
-      fputs_filtered (ada_enum_name (TYPE_FIELD_NAME (type, i)), stream);
+      fputs_styled (ada_enum_name (TYPE_FIELD_NAME (type, i)),
+		    variable_name_style.style (), stream);
       if (lastval != TYPE_FIELD_ENUMVAL (type, i))
 	{
 	  fprintf_filtered (stream, " => %s",
diff --git a/gdb/ada-valprint.c b/gdb/ada-valprint.c
index 4bb9247..6ba9a5d 100644
--- a/gdb/ada-valprint.c
+++ b/gdb/ada-valprint.c
@@ -418,7 +418,8 @@ 
 	}
       if (i < len)
 	{
-	  fputs_filtered (ada_enum_name (TYPE_FIELD_NAME (type, i)), stream);
+	  fputs_styled (ada_enum_name (TYPE_FIELD_NAME (type, i)),
+			variable_name_style.style (), stream);
 	}
       else
 	{
@@ -956,9 +957,11 @@ 
       const char *name = ada_enum_name (TYPE_FIELD_NAME (type, i));
 
       if (name[0] == '\'')
-	fprintf_filtered (stream, "%ld %s", (long) val, name);
+	fprintf_filtered (stream, "%ld %ps", (long) val,
+			  styled_string (variable_name_style.style (),
+					 name));
       else
-	fputs_filtered (name, stream);
+	fputs_styled (name, variable_name_style.style (), stream);
     }
   else
     print_longest (stream, 'd', 0, val);
diff --git a/gdb/c-typeprint.c b/gdb/c-typeprint.c
index 677b85e..6fcdb3a 100644
--- a/gdb/c-typeprint.c
+++ b/gdb/c-typeprint.c
@@ -148,7 +148,7 @@ 
       if (code == TYPE_CODE_FUNC || code == TYPE_CODE_METHOD)
 	fputs_styled (varstring, function_name_style.style (), stream);
       else
-	fputs_filtered (varstring, stream);
+	fputs_styled (varstring, variable_name_style.style (), stream);
 
       /* For demangled function names, we have the arglist as part of
          the name, so don't print an additional pair of ()'s.  */
@@ -1595,7 +1595,8 @@ 
 	      if (i)
 		fprintf_filtered (stream, ", ");
 	      wrap_here ("    ");
-	      fputs_filtered (TYPE_FIELD_NAME (type, i), stream);
+	      fputs_styled (TYPE_FIELD_NAME (type, i),
+			    variable_name_style.style (), stream);
 	      if (lastval != TYPE_FIELD_ENUMVAL (type, i))
 		{
 		  fprintf_filtered (stream, " = %s",
diff --git a/gdb/cp-valprint.c b/gdb/cp-valprint.c
index e445d42..05a28d4 100644
--- a/gdb/cp-valprint.c
+++ b/gdb/cp-valprint.c
@@ -235,11 +235,16 @@ 
 	  annotate_field_begin (TYPE_FIELD_TYPE (type, i));
 
 	  if (field_is_static (&TYPE_FIELD (type, i)))
-	    fputs_filtered ("static ", stream);
-	  fprintf_symbol_filtered (stream,
-				   TYPE_FIELD_NAME (type, i),
-				   current_language->la_language,
-				   DMGL_PARAMS | DMGL_ANSI);
+	    {
+	      fputs_filtered ("static ", stream);
+	      fprintf_symbol_filtered (stream,
+				       TYPE_FIELD_NAME (type, i),
+				       current_language->la_language,
+				       DMGL_PARAMS | DMGL_ANSI);
+	    }
+	  else
+	    fputs_styled (TYPE_FIELD_NAME (type, i),
+			  variable_name_style.style (), stream);
 	  annotate_field_name_end ();
 
 	  /* We tweak various options in a few cases below.  */
@@ -782,7 +787,8 @@ 
       else
 	c_type_print_base (self_type, stream, 0, 0, &type_print_raw_options);
       fprintf_filtered (stream, "::");
-      fputs_filtered (TYPE_FIELD_NAME (self_type, fieldno), stream);
+      fputs_styled (TYPE_FIELD_NAME (self_type, fieldno),
+		    variable_name_style.style (), stream);
     }
   else
     fprintf_filtered (stream, "%ld", (long) val);
diff --git a/gdb/f-typeprint.c b/gdb/f-typeprint.c
index 027bcdd..e14a78d 100644
--- a/gdb/f-typeprint.c
+++ b/gdb/f-typeprint.c
@@ -435,7 +435,8 @@ 
 	      f_type_print_base (TYPE_FIELD_TYPE (type, index), stream,
 				 show - 1, level + 4);
 	      fputs_filtered (" :: ", stream);
-	      fputs_filtered (TYPE_FIELD_NAME (type, index), stream);
+	      fputs_styled (TYPE_FIELD_NAME (type, index),
+			    variable_name_style.style (), stream);
 	      f_type_print_varspec_suffix (TYPE_FIELD_TYPE (type, index),
 					   stream, show - 1, 0, 0, 0, false);
 	      fputs_filtered ("\n", stream);
diff --git a/gdb/f-valprint.c b/gdb/f-valprint.c
index d5515c8..e22fa6c 100644
--- a/gdb/f-valprint.c
+++ b/gdb/f-valprint.c
@@ -335,7 +335,8 @@ 
 	      field_name = TYPE_FIELD_NAME (type, index);
 	      if (field_name != NULL)
 		{
-		  fputs_filtered (field_name, stream);
+		  fputs_styled (field_name, variable_name_style.style (),
+				stream);
 		  fputs_filtered (" = ", stream);
 		}
 
diff --git a/gdb/m2-typeprint.c b/gdb/m2-typeprint.c
index 41cdc87..c65f58b 100644
--- a/gdb/m2-typeprint.c
+++ b/gdb/m2-typeprint.c
@@ -563,7 +563,8 @@ 
 	  QUIT;
 
 	  print_spaces_filtered (level + 4, stream);
-	  fputs_filtered (TYPE_FIELD_NAME (type, i), stream);
+	  fputs_styled (TYPE_FIELD_NAME (type, i),
+			variable_name_style.style (), stream);
 	  fputs_filtered (" : ", stream);
 	  m2_print_type (TYPE_FIELD_TYPE (type, i),
 			 "",
@@ -608,7 +609,8 @@ 
 	  if (i > 0)
 	    fprintf_filtered (stream, ", ");
 	  wrap_here ("    ");
-	  fputs_filtered (TYPE_FIELD_NAME (type, i), stream);
+	  fputs_styled (TYPE_FIELD_NAME (type, i),
+			variable_name_style.style (), stream);
 	  if (lastval != TYPE_FIELD_ENUMVAL (type, i))
 	    {
 	      fprintf_filtered (stream, " = %s",
diff --git a/gdb/p-valprint.c b/gdb/p-valprint.c
index e1e1a00..bf7273f 100644
--- a/gdb/p-valprint.c
+++ b/gdb/p-valprint.c
@@ -605,10 +605,16 @@ 
 	  annotate_field_begin (TYPE_FIELD_TYPE (type, i));
 
 	  if (field_is_static (&TYPE_FIELD (type, i)))
-	    fputs_filtered ("static ", stream);
-	  fprintf_symbol_filtered (stream, TYPE_FIELD_NAME (type, i),
-				   language_cplus,
-				   DMGL_PARAMS | DMGL_ANSI);
+	    {
+	      fputs_filtered ("static ", stream);
+	      fprintf_symbol_filtered (stream,
+				       TYPE_FIELD_NAME (type, i),
+				       current_language->la_language,
+				       DMGL_PARAMS | DMGL_ANSI);
+	    }
+	  else
+	    fputs_styled (TYPE_FIELD_NAME (type, i),
+			  variable_name_style.style (), stream);
 	  annotate_field_name_end ();
 	  fputs_filtered (" = ", stream);
 	  annotate_field_value ();
diff --git a/gdb/rust-lang.c b/gdb/rust-lang.c
index f9adb5d..860b600 100644
--- a/gdb/rust-lang.c
+++ b/gdb/rust-lang.c
@@ -436,7 +436,8 @@ 
 
       if (!is_tuple && !is_tuple_struct)
         {
-	  fputs_filtered (TYPE_FIELD_NAME (type, i), stream);
+	  fputs_styled (TYPE_FIELD_NAME (type, i),
+			variable_name_style.style (), stream);
 	  fputs_filtered (": ", stream);
         }
 
@@ -514,8 +515,9 @@ 
       first_field = false;
 
       if (!is_tuple)
-	fprintf_filtered (stream, "%s: ",
-			  TYPE_FIELD_NAME (variant_type, j));
+	fprintf_filtered (stream, "%ps: ",
+			  styled_string (variable_name_style.style (),
+					 TYPE_FIELD_NAME (variant_type, j)));
 
       val_print (TYPE_FIELD_TYPE (variant_type, j),
 		 (embedded_offset
@@ -791,9 +793,12 @@ 
       if (!for_rust_enum || flags->print_offsets)
 	print_spaces_filtered (level + 2, stream);
       if (is_enum)
-	fputs_filtered (TYPE_FIELD_NAME (type, i), stream);
+	fputs_styled (TYPE_FIELD_NAME (type, i), variable_name_style.style (),
+		      stream);
       else if (!is_tuple_struct)
-	fprintf_filtered (stream, "%s: ", TYPE_FIELD_NAME (type, i));
+	fprintf_filtered (stream, "%ps: ",
+			  styled_string (variable_name_style.style (),
+					 TYPE_FIELD_NAME (type, i)));
 
       rust_internal_print_type (TYPE_FIELD_TYPE (type, i), NULL,
 				stream, (is_enum ? show : show - 1),
@@ -942,7 +947,9 @@ 
 		&& name[len] == ':'
 		&& name[len + 1] == ':')
 	      name += len + 2;
-	    fprintfi_filtered (level + 2, stream, "%s,\n", name);
+	    fprintfi_filtered (level + 2, stream, "%ps,\n",
+			       styled_string (variable_name_style.style (),
+					      name));
 	  }
 
 	fputs_filtered ("}", stream);
diff --git a/gdb/testsuite/ChangeLog b/gdb/testsuite/ChangeLog
index d4c42bd..bdff578 100644
--- a/gdb/testsuite/ChangeLog
+++ b/gdb/testsuite/ChangeLog
@@ -1,3 +1,12 @@ 
+2019-11-24  Tom Tromey  <tom@tromey.com>
+
+	* gdb.rust/rust-style.rs: New file.
+	* gdb.rust/rust-style.exp: New file.
+	* gdb.base/style.exp: Test structure printing.
+	* gdb.base/style.c (struct some_struct): New type.
+	(enum etype): New type.
+	(struct_value): New global.
+
 2019-11-21  Peeter Joot  <peeter.joot@lzlabs.com>
 
 	* gdb.base/endianity.c: New test.
diff --git a/gdb/testsuite/gdb.base/style.c b/gdb/testsuite/gdb.base/style.c
index 82dca7d..d90eb27 100644
--- a/gdb/testsuite/gdb.base/style.c
+++ b/gdb/testsuite/gdb.base/style.c
@@ -15,6 +15,21 @@ 
 
 #define SOME_MACRO 23
 
+enum etype
+{
+  VALUE_ONE = 1,
+  VALUE_TWO = 2
+};
+
+struct some_struct
+{
+  int int_field;
+  char *string_field;
+  enum etype e_field;
+};
+
+struct some_struct struct_value = { 23, "skidoo", VALUE_TWO };
+
 int some_called_function (void)
 {
   return 0;
diff --git a/gdb/testsuite/gdb.base/style.exp b/gdb/testsuite/gdb.base/style.exp
index c450f16..b140cd5 100644
--- a/gdb/testsuite/gdb.base/style.exp
+++ b/gdb/testsuite/gdb.base/style.exp
@@ -87,6 +87,13 @@ 
     # Somewhere should see the call to the function.
     gdb_test "disassemble main" "[style $hex address].*$func.*"
 
+    set ifield [style int_field variable]
+    set sfield [style string_field variable]
+    set efield [style e_field variable]
+    set evalue [style VALUE_TWO variable]
+    gdb_test "print struct_value" \
+	"\{$ifield = 23,.*$sfield = .*,.*$efield = $evalue.*"
+
     gdb_exit
     gdb_spawn
 
diff --git a/gdb/testsuite/gdb.rust/rust-style.exp b/gdb/testsuite/gdb.rust/rust-style.exp
new file mode 100644
index 0000000..dba204b
--- /dev/null
+++ b/gdb/testsuite/gdb.rust/rust-style.exp
@@ -0,0 +1,44 @@ 
+# Copyright 2019 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/>.
+
+# Test CLI output styling for Rust.
+
+load_lib rust-support.exp
+if {[skip_rust_tests]} {
+    continue
+}
+
+save_vars { env(TERM) } {
+    # We need an ANSI-capable terminal to get the output.
+    setenv TERM ansi
+
+    standard_testfile .rs
+    if {[prepare_for_testing "failed to prepare" $testfile $srcfile \
+	     {debug rust}]} {
+	return -1
+    }
+
+    set line [gdb_get_line_number "breakpoint"]
+    if {![runto ${srcfile}:$line]} {
+	untested "could not run to breakpoint"
+	return -1
+    }
+
+    set vfield [style value variable]
+    set v2field [style value2 variable]
+    gdb_test "print v" \
+	"Two\{$vfield: 23, $v2field: 97\}"
+
+}
diff --git a/gdb/testsuite/gdb.rust/rust-style.rs b/gdb/testsuite/gdb.rust/rust-style.rs
new file mode 100644
index 0000000..c9fb21b
--- /dev/null
+++ b/gdb/testsuite/gdb.rust/rust-style.rs
@@ -0,0 +1,29 @@ 
+// Copyright (C) 2019 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/>.
+
+#![allow(dead_code)]
+#![allow(unused_variables)]
+#![allow(unused_assignments)]
+
+enum EnumType {
+    One(i32),
+    Two{value: i32, value2: i32},
+}
+
+fn main() {
+    let v = EnumType::Two{ value: 23, value2: 97 };
+
+    println!("");               // breakpoint
+}
diff --git a/gdb/valprint.c b/gdb/valprint.c
index ced0dbc..40e924c 100644
--- a/gdb/valprint.c
+++ b/gdb/valprint.c
@@ -624,7 +624,8 @@ 
     }
   if (i < len)
     {
-      fputs_filtered (TYPE_FIELD_NAME (type, i), stream);
+      fputs_styled (TYPE_FIELD_NAME (type, i), variable_name_style.style (),
+		    stream);
     }
   else if (TYPE_FLAG_ENUM (type))
     {
@@ -645,7 +646,8 @@ 
 	      first = 0;
 
 	      val &= ~TYPE_FIELD_ENUMVAL (type, i);
-	      fputs_filtered (TYPE_FIELD_NAME (type, i), stream);
+	      fputs_styled (TYPE_FIELD_NAME (type, i),
+			    variable_name_style.style (), stream);
 	    }
 	}
 
@@ -1244,8 +1246,10 @@ 
 	      && TYPE_FIELD_BITSIZE (type, field) == 1)
 	    {
 	      if (val & ((ULONGEST)1 << TYPE_FIELD_BITPOS (type, field)))
-		fprintf_filtered (stream, " %s",
-				  TYPE_FIELD_NAME (type, field));
+		fprintf_filtered
+		  (stream, " %ps",
+		   styled_string (variable_name_style.style (),
+				  TYPE_FIELD_NAME (type, field)));
 	    }
 	  else
 	    {
@@ -1255,8 +1259,9 @@ 
 
 	      if (field_len < sizeof (ULONGEST) * TARGET_CHAR_BIT)
 		field_val &= ((ULONGEST) 1 << field_len) - 1;
-	      fprintf_filtered (stream, " %s=",
-				TYPE_FIELD_NAME (type, field));
+	      fprintf_filtered (stream, " %ps=",
+				styled_string (variable_name_style.style (),
+					       TYPE_FIELD_NAME (type, field)));
 	      if (TYPE_CODE (field_type) == TYPE_CODE_ENUM)
 		generic_val_print_enum_1 (field_type, field_val, stream);
 	      else