Mirror of the gdb-patches mailing list
 help / color / mirror / Atom feed
* [PATCH 1/6] Remove OP_LABELED.
  2012-11-01  7:47 [PATCH 0/6] Remove unused operator OP_LABELED Yao Qi
                   ` (2 preceding siblings ...)
  2012-11-01  7:47 ` [PATCH 2/6] Update comment of evaluate_struct_tuple Yao Qi
@ 2012-11-01  7:47 ` Yao Qi
  2012-11-01 15:41   ` Tom Tromey
  2012-11-02 16:43   ` Pedro Alves
  2012-11-01  7:47 ` [PATCH 3/6] Remove 'variantno' Yao Qi
                   ` (2 subsequent siblings)
  6 siblings, 2 replies; 20+ messages in thread
From: Yao Qi @ 2012-11-01  7:47 UTC (permalink / raw)
  To: gdb-patches

This patch is to remove OP_LABELED.  Once OP_LABELED is removed,
function get_label always return NULL, so this function can be removed
as well, and its caller can be simplified.

Indentation will be done in follow-up patch.

gdb:

2012-11-01  Yao Qi  <yao@codesourcery.com>

	* std-operator.def: Remove OP_LABELED.

	* eval.c: Remove the declaration of 'get_label'.
	(get_label): Remove.
	(evaluate_struct_tuple): Remove code handling OP_LABELED.
	* expprint.c (print_subexp_standard): Likewise.
	(dump_subexp_body_standard): Likewise.
	* parse.c (operator_length_standard): Likewise.
---
 gdb/eval.c           |   86 --------------------------------------------------
 gdb/expprint.c       |   16 ---------
 gdb/parse.c          |    1 -
 gdb/std-operator.def |   13 -------
 4 files changed, 0 insertions(+), 116 deletions(-)

diff --git a/gdb/eval.c b/gdb/eval.c
index 26e0cc8..011b3fe 100644
--- a/gdb/eval.c
+++ b/gdb/eval.c
@@ -56,8 +56,6 @@ static struct value *evaluate_subexp_for_sizeof (struct expression *, int *);
 static struct value *evaluate_subexp_for_address (struct expression *,
 						  int *, enum noside);
 
-static char *get_label (struct expression *, int *);
-
 static struct value *evaluate_struct_tuple (struct value *,
 					    struct expression *, int *,
 					    enum noside, int);
@@ -280,25 +278,6 @@ extract_field_op (struct expression *exp, int *subexp)
   return result;
 }
 
-/* If the next expression is an OP_LABELED, skips past it,
-   returning the label.  Otherwise, does nothing and returns NULL.  */
-
-static char *
-get_label (struct expression *exp, int *pos)
-{
-  if (exp->elts[*pos].opcode == OP_LABELED)
-    {
-      int pc = (*pos)++;
-      char *name = &exp->elts[pc + 2].string;
-      int tem = longest_to_int (exp->elts[pc + 1].longconst);
-
-      (*pos) += 3 + BYTES_TO_EXP_ELEM (tem + 1);
-      return name;
-    }
-  else
-    return NULL;
-}
-
 /* This function evaluates tuples (in (the deleted) Chill) or
    brace-initializers (in C/C++) for structure types.  */
 
@@ -316,74 +295,10 @@ evaluate_struct_tuple (struct value *struct_val,
 
   while (--nargs >= 0)
     {
-      int pc = *pos;
       struct value *val = NULL;
-      int nlabels = 0;
       int bitpos, bitsize;
       bfd_byte *addr;
-
-      /* Skip past the labels, and count them.  */
-      while (get_label (exp, pos) != NULL)
-	nlabels++;
-
-      do
 	{
-	  char *label = get_label (exp, &pc);
-
-	  if (label)
-	    {
-	      for (fieldno = 0; fieldno < TYPE_NFIELDS (struct_type);
-		   fieldno++)
-		{
-		  const char *field_name =
-		    TYPE_FIELD_NAME (struct_type, fieldno);
-
-		  if (field_name != NULL && strcmp (field_name, label) == 0)
-		    {
-		      variantno = -1;
-		      subfieldno = fieldno;
-		      substruct_type = struct_type;
-		      goto found;
-		    }
-		}
-	      for (fieldno = 0; fieldno < TYPE_NFIELDS (struct_type);
-		   fieldno++)
-		{
-		  const char *field_name =
-		    TYPE_FIELD_NAME (struct_type, fieldno);
-
-		  field_type = TYPE_FIELD_TYPE (struct_type, fieldno);
-		  if ((field_name == 0 || *field_name == '\0')
-		      && TYPE_CODE (field_type) == TYPE_CODE_UNION)
-		    {
-		      variantno = 0;
-		      for (; variantno < TYPE_NFIELDS (field_type);
-			   variantno++)
-			{
-			  substruct_type
-			    = TYPE_FIELD_TYPE (field_type, variantno);
-			  if (TYPE_CODE (substruct_type) == TYPE_CODE_STRUCT)
-			    {
-			      for (subfieldno = 0;
-				 subfieldno < TYPE_NFIELDS (substruct_type);
-				   subfieldno++)
-				{
-				  if (strcmp(TYPE_FIELD_NAME (substruct_type,
-							      subfieldno),
-					     label) == 0)
-				    {
-				      goto found;
-				    }
-				}
-			    }
-			}
-		    }
-		}
-	      error (_("there is no field named %s"), label);
-	    found:
-	      ;
-	    }
-	  else
 	    {
 	      /* Unlabelled tuple element - go to next field.  */
 	      if (variantno >= 0)
@@ -444,7 +359,6 @@ evaluate_struct_tuple (struct value *struct_val,
 	    memcpy (addr, value_contents (val),
 		    TYPE_LENGTH (value_type (val)));
 	}
-      while (--nlabels > 0);
     }
   return struct_val;
 }
diff --git a/gdb/expprint.c b/gdb/expprint.c
index baaa08a..129dfa7 100644
--- a/gdb/expprint.c
+++ b/gdb/expprint.c
@@ -328,21 +328,6 @@ print_subexp_standard (struct expression *exp, int *pos,
 	}
       return;
 
-    case OP_LABELED:
-      tem = longest_to_int (exp->elts[pc + 1].longconst);
-      (*pos) += 3 + BYTES_TO_EXP_ELEM (tem + 1);
-      /* Gcc support both these syntaxes.  Unsure which is preferred.  */
-#if 1
-      fputs_filtered (&exp->elts[pc + 2].string, stream);
-      fputs_filtered (": ", stream);
-#else
-      fputs_filtered (".", stream);
-      fputs_filtered (&exp->elts[pc + 2].string, stream);
-      fputs_filtered ("=", stream);
-#endif
-      print_subexp (exp, pos, stream, PREC_SUFFIX);
-      return;
-
     case TERNOP_COND:
       if ((int) prec > (int) PREC_COMMA)
 	fputs_filtered ("(", stream);
@@ -1031,7 +1016,6 @@ dump_subexp_body_standard (struct expression *exp,
     case OP_BOOL:
     case OP_M2_STRING:
     case OP_THIS:
-    case OP_LABELED:
     case OP_NAME:
       fprintf_filtered (stream, "Unknown format");
     }
diff --git a/gdb/parse.c b/gdb/parse.c
index 3ec1e25..afe1c18 100644
--- a/gdb/parse.c
+++ b/gdb/parse.c
@@ -951,7 +951,6 @@ operator_length_standard (const struct expression *expr, int endpos,
       oplen++;
       break;
 
-    case OP_LABELED:
     case STRUCTOP_STRUCT:
     case STRUCTOP_PTR:
       args = 1;
diff --git a/gdb/std-operator.def b/gdb/std-operator.def
index dcba39f..5df0081 100644
--- a/gdb/std-operator.def
+++ b/gdb/std-operator.def
@@ -286,19 +286,6 @@ OP (OP_OBJC_SELECTOR)
    a string, which, of course, is variable length.  */
 OP (OP_SCOPE)
 
-/* Used to represent named structure field values in brace
-   initializers (or tuples as they are called in (the deleted)
-   Chill).
-
-   The gcc C syntax is NAME:VALUE or .NAME=VALUE, the (the
-   deleted) Chill syntax is .NAME:VALUE.  Multiple labels (as in
-   the (the deleted) Chill syntax .NAME1,.NAME2:VALUE) is
-   represented as if it were .NAME1:(.NAME2:VALUE) (though that is
-   not valid (the deleted) Chill syntax).
-
-   The NAME is represented as for STRUCTOP_STRUCT;  VALUE follows.  */
-OP (OP_LABELED)
-
 /* OP_TYPE is for parsing types, and used with the "ptype" command
    so we can look up types that are qualified by scope, either with
    the GDB "::" operator, or the Modula-2 '.' operator.  */
-- 
1.7.7.6


^ permalink raw reply	[flat|nested] 20+ messages in thread

* [PATCH 6/6] Test nested struct assign
  2012-11-01  7:47 [PATCH 0/6] Remove unused operator OP_LABELED Yao Qi
  2012-11-01  7:47 ` [PATCH 4/6] Remove 'substruct_type' and 'subfieldno' Yao Qi
@ 2012-11-01  7:47 ` Yao Qi
  2012-11-01 15:47   ` Tom Tromey
  2012-11-01  7:47 ` [PATCH 2/6] Update comment of evaluate_struct_tuple Yao Qi
                   ` (4 subsequent siblings)
  6 siblings, 1 reply; 20+ messages in thread
From: Yao Qi @ 2012-11-01  7:47 UTC (permalink / raw)
  To: gdb-patches

Existing testsuite doesn't exercise function evaluate_struct_tuple, so
this test is added to do so.

gdb/testsuite:

2012-11-01  Yao Qi  <yao@codesourcery.com>

	* gdb.base/setvar.exp: Test setting nested struct.
	* gdb.base/setvar.c (v_struct3): New.
---
 gdb/testsuite/gdb.base/setvar.c   |    7 +++++++
 gdb/testsuite/gdb.base/setvar.exp |    8 ++++++++
 2 files changed, 15 insertions(+), 0 deletions(-)

diff --git a/gdb/testsuite/gdb.base/setvar.c b/gdb/testsuite/gdb.base/setvar.c
index 83509cd..3a80b22 100644
--- a/gdb/testsuite/gdb.base/setvar.c
+++ b/gdb/testsuite/gdb.base/setvar.c
@@ -115,6 +115,13 @@ struct {
     double	v_double_member;
 } v_struct2;
 
+struct
+{
+  long v_long_member;
+  struct t_struct t;
+  char v_char_member;
+} v_struct3;
+
 /**** unions *******/
 
 union t_union {
diff --git a/gdb/testsuite/gdb.base/setvar.exp b/gdb/testsuite/gdb.base/setvar.exp
index 7faa10a..3c5251f 100644
--- a/gdb/testsuite/gdb.base/setvar.exp
+++ b/gdb/testsuite/gdb.base/setvar.exp
@@ -383,6 +383,14 @@ test_set "set variable v_struct1 = {'h', 1, 2, 3, 4.0, 5.0}" \
 v_long_member = 3,.*v_float_member = 4,.*v_double_member = 5.*\\}" \
   "set print structure #3"
 
+#
+# test "set variable" for nested struct
+#
+test_set "set variable v_struct3 = {1, {'h', 1, 2, 3, 4.0, 5.0}, 37}" \
+  "print v_struct3" \
+    ".*.\[0-9\]* = \\{.*v_long_member = 1,.*t = \\{.*v_char_member = 104 \'h\',.*v_short_member = 1,.*v_int_member = 2,.*v_long_member = 3,.*v_float_member = 4,.*v_double_member = 5.*\\},.*v_char_member = 37 \'%\'\\}" \
+  "set print structure #4"
+
 set timeout $prev_timeout
 
 # Test printing of enumeration bitfields.
-- 
1.7.7.6


^ permalink raw reply	[flat|nested] 20+ messages in thread

* [PATCH 5/6] Indentation.
  2012-11-01  7:47 [PATCH 0/6] Remove unused operator OP_LABELED Yao Qi
                   ` (4 preceding siblings ...)
  2012-11-01  7:47 ` [PATCH 3/6] Remove 'variantno' Yao Qi
@ 2012-11-01  7:47 ` Yao Qi
  2012-11-01 15:45   ` Tom Tromey
  2012-11-01 15:47 ` [PATCH 0/6] Remove unused operator OP_LABELED Tom Tromey
  6 siblings, 1 reply; 20+ messages in thread
From: Yao Qi @ 2012-11-01  7:47 UTC (permalink / raw)
  To: gdb-patches

gdb:

2012-11-01  Yao Qi  <yao@codesourcery.com>

	* eval.c (evaluate_struct_tuple): Indent code.
---
 gdb/eval.c |   88 ++++++++++++++++++++++++++++-------------------------------
 1 files changed, 42 insertions(+), 46 deletions(-)

diff --git a/gdb/eval.c b/gdb/eval.c
index 62b26a9..f655957 100644
--- a/gdb/eval.c
+++ b/gdb/eval.c
@@ -295,53 +295,49 @@ evaluate_struct_tuple (struct value *struct_val,
       struct value *val = NULL;
       int bitpos, bitsize;
       bfd_byte *addr;
-	{
-	    {
-		{
-		  fieldno++;
-		  /* Skip static fields.  */
-		  while (fieldno < TYPE_NFIELDS (struct_type)
-			 && field_is_static (&TYPE_FIELD (struct_type,
-							  fieldno)))
-		    fieldno++;
-		  if (fieldno >= TYPE_NFIELDS (struct_type))
-		    error (_("too many initializers"));
-		  field_type = TYPE_FIELD_TYPE (struct_type, fieldno);
-		  if (TYPE_CODE (field_type) == TYPE_CODE_UNION
-		      && TYPE_FIELD_NAME (struct_type, fieldno)[0] == '0')
-		    error (_("don't know which variant you want to set"));
-		}
-	    }
 
-	  /* Here, struct_type is the type of the inner struct,
-	     while substruct_type is the type of the inner struct.
-	     These are the same for normal structures, but a variant struct
-	     contains anonymous union fields that contain substruct fields.
-	     The value fieldno is the index of the top-level (normal or
-	     anonymous union) field in struct_field, while the value
-	     subfieldno is the index of the actual real (named inner) field
-	     in substruct_type.  */
-
-	  field_type = TYPE_FIELD_TYPE (struct_type, fieldno);
-	  if (val == 0)
-	    val = evaluate_subexp (field_type, exp, pos, noside);
-
-	  /* Now actually set the field in struct_val.  */
-
-	  /* Assign val to field fieldno.  */
-	  if (value_type (val) != field_type)
-	    val = value_cast (field_type, val);
-
-	  bitsize = TYPE_FIELD_BITSIZE (struct_type, fieldno);
-	  bitpos = TYPE_FIELD_BITPOS (struct_type, fieldno);
-	  addr = value_contents_writeable (struct_val) + bitpos / 8;
-	  if (bitsize)
-	    modify_field (struct_type, addr,
-			  value_as_long (val), bitpos % 8, bitsize);
-	  else
-	    memcpy (addr, value_contents (val),
-		    TYPE_LENGTH (value_type (val)));
-	}
+      fieldno++;
+      /* Skip static fields.  */
+      while (fieldno < TYPE_NFIELDS (struct_type)
+	     && field_is_static (&TYPE_FIELD (struct_type,
+					      fieldno)))
+	fieldno++;
+      if (fieldno >= TYPE_NFIELDS (struct_type))
+	error (_("too many initializers"));
+      field_type = TYPE_FIELD_TYPE (struct_type, fieldno);
+      if (TYPE_CODE (field_type) == TYPE_CODE_UNION
+	  && TYPE_FIELD_NAME (struct_type, fieldno)[0] == '0')
+	error (_("don't know which variant you want to set"));
+
+      /* Here, struct_type is the type of the inner struct,
+	 while substruct_type is the type of the inner struct.
+	 These are the same for normal structures, but a variant struct
+	 contains anonymous union fields that contain substruct fields.
+	 The value fieldno is the index of the top-level (normal or
+	 anonymous union) field in struct_field, while the value
+	 subfieldno is the index of the actual real (named inner) field
+	 in substruct_type.  */
+
+      field_type = TYPE_FIELD_TYPE (struct_type, fieldno);
+      if (val == 0)
+	val = evaluate_subexp (field_type, exp, pos, noside);
+
+      /* Now actually set the field in struct_val.  */
+
+      /* Assign val to field fieldno.  */
+      if (value_type (val) != field_type)
+	val = value_cast (field_type, val);
+
+      bitsize = TYPE_FIELD_BITSIZE (struct_type, fieldno);
+      bitpos = TYPE_FIELD_BITPOS (struct_type, fieldno);
+      addr = value_contents_writeable (struct_val) + bitpos / 8;
+      if (bitsize)
+	modify_field (struct_type, addr,
+		      value_as_long (val), bitpos % 8, bitsize);
+      else
+	memcpy (addr, value_contents (val),
+		TYPE_LENGTH (value_type (val)));
+
     }
   return struct_val;
 }
-- 
1.7.7.6


^ permalink raw reply	[flat|nested] 20+ messages in thread

* [PATCH 2/6] Update comment of evaluate_struct_tuple.
  2012-11-01  7:47 [PATCH 0/6] Remove unused operator OP_LABELED Yao Qi
  2012-11-01  7:47 ` [PATCH 4/6] Remove 'substruct_type' and 'subfieldno' Yao Qi
  2012-11-01  7:47 ` [PATCH 6/6] Test nested struct assign Yao Qi
@ 2012-11-01  7:47 ` Yao Qi
  2012-11-01 15:42   ` Tom Tromey
  2012-11-01  7:47 ` [PATCH 1/6] Remove OP_LABELED Yao Qi
                   ` (3 subsequent siblings)
  6 siblings, 1 reply; 20+ messages in thread
From: Yao Qi @ 2012-11-01  7:47 UTC (permalink / raw)
  To: gdb-patches

gdb:

2012-11-01  Yao Qi  <yao@codesourcery.com>

	* eval.c (evaluate_struct_tuple): Update comment.
---
 gdb/eval.c |    4 ++--
 1 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/gdb/eval.c b/gdb/eval.c
index 011b3fe..681012b 100644
--- a/gdb/eval.c
+++ b/gdb/eval.c
@@ -278,8 +278,8 @@ extract_field_op (struct expression *exp, int *subexp)
   return result;
 }
 
-/* This function evaluates tuples (in (the deleted) Chill) or
-   brace-initializers (in C/C++) for structure types.  */
+/* This function evaluates brace-initializers (in C/C++) for
+   structure types.  */
 
 static struct value *
 evaluate_struct_tuple (struct value *struct_val,
-- 
1.7.7.6


^ permalink raw reply	[flat|nested] 20+ messages in thread

* [PATCH 0/6] Remove unused operator OP_LABELED
@ 2012-11-01  7:47 Yao Qi
  2012-11-01  7:47 ` [PATCH 4/6] Remove 'substruct_type' and 'subfieldno' Yao Qi
                   ` (6 more replies)
  0 siblings, 7 replies; 20+ messages in thread
From: Yao Qi @ 2012-11-01  7:47 UTC (permalink / raw)
  To: gdb-patches

Hello,
In Aug., I posted patches to remove unused operators except
OP_LABELED,

  [RFC 0/3] Remove unused operators OP_BITSTRING TERNOP_SLICE_COUNT
and TYPE_CODE_BITSTRING
  http://sourceware.org/ml/gdb-patches/2012-08/msg00307.html

This patch set removes the unused operator OP_LABELED.  Since
OP_LABELED related code was coupled with others, the removal is done
step by step in order to make each patch easy to read.  Patch 1/6 is
to remove OP_LABELED, and patch 2/6 - 4/6 are to remove unused code as
a result of patch 1/6.  Patch 5/6 is for indentation, and patch 6/6 is
to add a test to make sure my change is correct.

The whole patch set is tested on x86_64-linux.  Is it OK?


^ permalink raw reply	[flat|nested] 20+ messages in thread

* [PATCH 3/6] Remove 'variantno'.
  2012-11-01  7:47 [PATCH 0/6] Remove unused operator OP_LABELED Yao Qi
                   ` (3 preceding siblings ...)
  2012-11-01  7:47 ` [PATCH 1/6] Remove OP_LABELED Yao Qi
@ 2012-11-01  7:47 ` Yao Qi
  2012-11-01 15:44   ` Tom Tromey
  2012-11-01  7:47 ` [PATCH 5/6] Indentation Yao Qi
  2012-11-01 15:47 ` [PATCH 0/6] Remove unused operator OP_LABELED Tom Tromey
  6 siblings, 1 reply; 20+ messages in thread
From: Yao Qi @ 2012-11-01  7:47 UTC (permalink / raw)
  To: gdb-patches

As local variable 'variantno' becomens a constant -1, some code can be
removed.

gdb:

2012-11-01  Yao Qi  <yao@codesourcery.com>

	* eval.c (evaluate_struct_tuple): Remove local variable
	'variantno' and related code.
---
 gdb/eval.c |   14 --------------
 1 files changed, 0 insertions(+), 14 deletions(-)

diff --git a/gdb/eval.c b/gdb/eval.c
index 681012b..3eb4896 100644
--- a/gdb/eval.c
+++ b/gdb/eval.c
@@ -290,7 +290,6 @@ evaluate_struct_tuple (struct value *struct_val,
   struct type *substruct_type = struct_type;
   struct type *field_type;
   int fieldno = -1;
-  int variantno = -1;
   int subfieldno = -1;
 
   while (--nargs >= 0)
@@ -300,17 +299,6 @@ evaluate_struct_tuple (struct value *struct_val,
       bfd_byte *addr;
 	{
 	    {
-	      /* Unlabelled tuple element - go to next field.  */
-	      if (variantno >= 0)
-		{
-		  subfieldno++;
-		  if (subfieldno >= TYPE_NFIELDS (substruct_type))
-		    {
-		      variantno = -1;
-		      substruct_type = struct_type;
-		    }
-		}
-	      if (variantno < 0)
 		{
 		  fieldno++;
 		  /* Skip static fields.  */
@@ -349,8 +337,6 @@ evaluate_struct_tuple (struct value *struct_val,
 
 	  bitsize = TYPE_FIELD_BITSIZE (substruct_type, subfieldno);
 	  bitpos = TYPE_FIELD_BITPOS (struct_type, fieldno);
-	  if (variantno >= 0)
-	    bitpos += TYPE_FIELD_BITPOS (substruct_type, subfieldno);
 	  addr = value_contents_writeable (struct_val) + bitpos / 8;
 	  if (bitsize)
 	    modify_field (struct_type, addr,
-- 
1.7.7.6


^ permalink raw reply	[flat|nested] 20+ messages in thread

* [PATCH 4/6] Remove 'substruct_type' and 'subfieldno'.
  2012-11-01  7:47 [PATCH 0/6] Remove unused operator OP_LABELED Yao Qi
@ 2012-11-01  7:47 ` Yao Qi
  2012-11-01 15:44   ` Tom Tromey
  2012-11-01  7:47 ` [PATCH 6/6] Test nested struct assign Yao Qi
                   ` (5 subsequent siblings)
  6 siblings, 1 reply; 20+ messages in thread
From: Yao Qi @ 2012-11-01  7:47 UTC (permalink / raw)
  To: gdb-patches

gdb:

2012-11-01  Yao Qi  <yao@codesourcery.com>

	* eval.c (evaluate_struct_tuple): Replace 'substruct_type' with
	'struct_type'.  Replace 'subfieldno' with 'fieldno'.
---
 gdb/eval.c |    7 ++-----
 1 files changed, 2 insertions(+), 5 deletions(-)

diff --git a/gdb/eval.c b/gdb/eval.c
index 3eb4896..62b26a9 100644
--- a/gdb/eval.c
+++ b/gdb/eval.c
@@ -287,10 +287,8 @@ evaluate_struct_tuple (struct value *struct_val,
 		       int *pos, enum noside noside, int nargs)
 {
   struct type *struct_type = check_typedef (value_type (struct_val));
-  struct type *substruct_type = struct_type;
   struct type *field_type;
   int fieldno = -1;
-  int subfieldno = -1;
 
   while (--nargs >= 0)
     {
@@ -306,7 +304,6 @@ evaluate_struct_tuple (struct value *struct_val,
 			 && field_is_static (&TYPE_FIELD (struct_type,
 							  fieldno)))
 		    fieldno++;
-		  subfieldno = fieldno;
 		  if (fieldno >= TYPE_NFIELDS (struct_type))
 		    error (_("too many initializers"));
 		  field_type = TYPE_FIELD_TYPE (struct_type, fieldno);
@@ -325,7 +322,7 @@ evaluate_struct_tuple (struct value *struct_val,
 	     subfieldno is the index of the actual real (named inner) field
 	     in substruct_type.  */
 
-	  field_type = TYPE_FIELD_TYPE (substruct_type, subfieldno);
+	  field_type = TYPE_FIELD_TYPE (struct_type, fieldno);
 	  if (val == 0)
 	    val = evaluate_subexp (field_type, exp, pos, noside);
 
@@ -335,7 +332,7 @@ evaluate_struct_tuple (struct value *struct_val,
 	  if (value_type (val) != field_type)
 	    val = value_cast (field_type, val);
 
-	  bitsize = TYPE_FIELD_BITSIZE (substruct_type, subfieldno);
+	  bitsize = TYPE_FIELD_BITSIZE (struct_type, fieldno);
 	  bitpos = TYPE_FIELD_BITPOS (struct_type, fieldno);
 	  addr = value_contents_writeable (struct_val) + bitpos / 8;
 	  if (bitsize)
-- 
1.7.7.6


^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [PATCH 1/6] Remove OP_LABELED.
  2012-11-01  7:47 ` [PATCH 1/6] Remove OP_LABELED Yao Qi
@ 2012-11-01 15:41   ` Tom Tromey
  2012-11-02 16:43   ` Pedro Alves
  1 sibling, 0 replies; 20+ messages in thread
From: Tom Tromey @ 2012-11-01 15:41 UTC (permalink / raw)
  To: Yao Qi; +Cc: gdb-patches

>>>>> "Yao" == Yao Qi <yao@codesourcery.com> writes:

Yao> This patch is to remove OP_LABELED.  Once OP_LABELED is removed,
Yao> function get_label always return NULL, so this function can be removed
Yao> as well, and its caller can be simplified.

Ok.

Tom


^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [PATCH 2/6] Update comment of evaluate_struct_tuple.
  2012-11-01  7:47 ` [PATCH 2/6] Update comment of evaluate_struct_tuple Yao Qi
@ 2012-11-01 15:42   ` Tom Tromey
  0 siblings, 0 replies; 20+ messages in thread
From: Tom Tromey @ 2012-11-01 15:42 UTC (permalink / raw)
  To: Yao Qi; +Cc: gdb-patches

>>>>> "Yao" == Yao Qi <yao@codesourcery.com> writes:

Yao> 2012-11-01  Yao Qi  <yao@codesourcery.com>
Yao> 	* eval.c (evaluate_struct_tuple): Update comment.

Ok, even obvious.
I think I would have stuck this in patch #1.

Tom


^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [PATCH 3/6] Remove 'variantno'.
  2012-11-01  7:47 ` [PATCH 3/6] Remove 'variantno' Yao Qi
@ 2012-11-01 15:44   ` Tom Tromey
  0 siblings, 0 replies; 20+ messages in thread
From: Tom Tromey @ 2012-11-01 15:44 UTC (permalink / raw)
  To: Yao Qi; +Cc: gdb-patches

>>>>> "Yao" == Yao Qi <yao@codesourcery.com> writes:

Yao> 2012-11-01  Yao Qi  <yao@codesourcery.com>
Yao> 	* eval.c (evaluate_struct_tuple): Remove local variable
Yao> 	'variantno' and related code.

Ok.

Tom


^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [PATCH 4/6] Remove 'substruct_type' and 'subfieldno'.
  2012-11-01  7:47 ` [PATCH 4/6] Remove 'substruct_type' and 'subfieldno' Yao Qi
@ 2012-11-01 15:44   ` Tom Tromey
  0 siblings, 0 replies; 20+ messages in thread
From: Tom Tromey @ 2012-11-01 15:44 UTC (permalink / raw)
  To: Yao Qi; +Cc: gdb-patches

>>>>> "Yao" == Yao Qi <yao@codesourcery.com> writes:

Yao> 2012-11-01  Yao Qi  <yao@codesourcery.com>
Yao> 	* eval.c (evaluate_struct_tuple): Replace 'substruct_type' with
Yao> 	'struct_type'.  Replace 'subfieldno' with 'fieldno'.

Ok.

Tom


^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [PATCH 5/6] Indentation.
  2012-11-01  7:47 ` [PATCH 5/6] Indentation Yao Qi
@ 2012-11-01 15:45   ` Tom Tromey
  0 siblings, 0 replies; 20+ messages in thread
From: Tom Tromey @ 2012-11-01 15:45 UTC (permalink / raw)
  To: Yao Qi; +Cc: gdb-patches

>>>>> "Yao" == Yao Qi <yao@codesourcery.com> writes:

Yao> 2012-11-01  Yao Qi  <yao@codesourcery.com>
Yao> 	* eval.c (evaluate_struct_tuple): Indent code.

Ok, but really obvious given the others.

Tom


^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [PATCH 6/6] Test nested struct assign
  2012-11-01  7:47 ` [PATCH 6/6] Test nested struct assign Yao Qi
@ 2012-11-01 15:47   ` Tom Tromey
  0 siblings, 0 replies; 20+ messages in thread
From: Tom Tromey @ 2012-11-01 15:47 UTC (permalink / raw)
  To: Yao Qi; +Cc: gdb-patches

>>>>> "Yao" == Yao Qi <yao@codesourcery.com> writes:

Yao> Existing testsuite doesn't exercise function evaluate_struct_tuple, so
Yao> this test is added to do so.

Thank you very much.
I've recently been looking at code coverage from our test suite a bit,
and this kind of addition is very welcome.

Yao> 2012-11-01  Yao Qi  <yao@codesourcery.com>

Yao> 	* gdb.base/setvar.exp: Test setting nested struct.
Yao> 	* gdb.base/setvar.c (v_struct3): New.

Ok.

Tom


^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [PATCH 0/6] Remove unused operator OP_LABELED
  2012-11-01  7:47 [PATCH 0/6] Remove unused operator OP_LABELED Yao Qi
                   ` (5 preceding siblings ...)
  2012-11-01  7:47 ` [PATCH 5/6] Indentation Yao Qi
@ 2012-11-01 15:47 ` Tom Tromey
  2012-11-02  0:20   ` [committed]: " Yao Qi
  6 siblings, 1 reply; 20+ messages in thread
From: Tom Tromey @ 2012-11-01 15:47 UTC (permalink / raw)
  To: Yao Qi; +Cc: gdb-patches

>>>>> "Yao" == Yao Qi <yao@codesourcery.com> writes:

Yao> Since OP_LABELED related code was coupled with others, the removal
Yao> is done step by step in order to make each patch easy to read.

Nicely done.  Thank you.

Tom


^ permalink raw reply	[flat|nested] 20+ messages in thread

* [committed]: [PATCH 0/6] Remove unused operator OP_LABELED
  2012-11-01 15:47 ` [PATCH 0/6] Remove unused operator OP_LABELED Tom Tromey
@ 2012-11-02  0:20   ` Yao Qi
  0 siblings, 0 replies; 20+ messages in thread
From: Yao Qi @ 2012-11-02  0:20 UTC (permalink / raw)
  Cc: gdb-patches

On 11/01/2012 11:47 PM, Tom Tromey wrote:
>>>>>> "Yao" == Yao Qi <yao@codesourcery.com> writes:
>
> Yao> Since OP_LABELED related code was coupled with others, the removal
> Yao> is done step by step in order to make each patch easy to read.
>
> Nicely done.  Thank you.
>

Committed.

-- 
Yao


^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [PATCH 1/6] Remove OP_LABELED.
  2012-11-01  7:47 ` [PATCH 1/6] Remove OP_LABELED Yao Qi
  2012-11-01 15:41   ` Tom Tromey
@ 2012-11-02 16:43   ` Pedro Alves
  2012-11-02 16:49     ` Tom Tromey
  1 sibling, 1 reply; 20+ messages in thread
From: Pedro Alves @ 2012-11-02 16:43 UTC (permalink / raw)
  To: Yao Qi; +Cc: gdb-patches

On 11/01/2012 07:46 AM, Yao Qi wrote:
>  
> -/* Used to represent named structure field values in brace
> -   initializers (or tuples as they are called in (the deleted)
> -   Chill).
> -
> -   The gcc C syntax is NAME:VALUE or .NAME=VALUE, the (the
> -   deleted) Chill syntax is .NAME:VALUE.  Multiple labels (as in
> -   the (the deleted) Chill syntax .NAME1,.NAME2:VALUE) is
> -   represented as if it were .NAME1:(.NAME2:VALUE) (though that is
> -   not valid (the deleted) Chill syntax).

I found this mention of C syntax curious, and tried it, to see if
we're implementing that in some other way, perhaps.  It seems to be
talking about designated initializers
(<http://gcc.gnu.org/onlinedocs/gcc/Designated-Inits.html>,
at least the .NAME=VALUE variant).  Doesn't work unfortunately:

(debug gdb, and run to main)

(top-gdb) p args
$1 = {argc = 8450672, argv = 0x457990 <_start>, use_windows = -9104, interpreter_p = 0x0}
(top-gdb) p args = {0}
$2 = {argc = 0, argv = 0x0, use_windows = 0, interpreter_p = 0x0}
(top-gdb) p args = {1, 0}
$3 = {argc = 1, argv = 0x0, use_windows = 0, interpreter_p = 0x0}
(top-gdb) p args = {.argc = 1, 0}
A syntax error in expression, near `.argc = 1, 0}'.

Seems like something we should/could support.  I'll file a PR, unless I
missed something.

I have no idea if the OP_LABELED implementation was a good basis for this
or not.

> -
> -   The NAME is represented as for STRUCTOP_STRUCT;  VALUE follows.  */
> -OP (OP_LABELED)
> -


-- 
Pedro Alves


^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [PATCH 1/6] Remove OP_LABELED.
  2012-11-02 16:43   ` Pedro Alves
@ 2012-11-02 16:49     ` Tom Tromey
  2012-11-02 16:51       ` Pedro Alves
  0 siblings, 1 reply; 20+ messages in thread
From: Tom Tromey @ 2012-11-02 16:49 UTC (permalink / raw)
  To: Pedro Alves; +Cc: Yao Qi, gdb-patches

>>>>> "Pedro" == Pedro Alves <palves@redhat.com> writes:

Pedro> I have no idea if the OP_LABELED implementation was a good basis
Pedro> for this or not.

We can stick a link to this discussion in the PR, in case somebody wants
to look.

Tom


^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [PATCH 1/6] Remove OP_LABELED.
  2012-11-02 16:49     ` Tom Tromey
@ 2012-11-02 16:51       ` Pedro Alves
  2012-11-02 17:09         ` Tom Tromey
  0 siblings, 1 reply; 20+ messages in thread
From: Pedro Alves @ 2012-11-02 16:51 UTC (permalink / raw)
  To: Tom Tromey; +Cc: Yao Qi, gdb-patches

On 11/02/2012 04:48 PM, Tom Tromey wrote:
>>>>>> "Pedro" == Pedro Alves <palves@redhat.com> writes:
> 
> Pedro> I have no idea if the OP_LABELED implementation was a good basis
> Pedro> for this or not.
> 
> We can stick a link to this discussion in the PR, in case somebody wants
> to look.

Yep, that was the plan.  I'll take that as confirmation that designated
initializers are not known to be, or should be working through some
other means.  :-)  Thanks.

-- 
Pedro Alves


^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [PATCH 1/6] Remove OP_LABELED.
  2012-11-02 16:51       ` Pedro Alves
@ 2012-11-02 17:09         ` Tom Tromey
  2012-11-02 17:25           ` Pedro Alves
  0 siblings, 1 reply; 20+ messages in thread
From: Tom Tromey @ 2012-11-02 17:09 UTC (permalink / raw)
  To: Pedro Alves; +Cc: Yao Qi, gdb-patches

>>>>> "Pedro" == Pedro Alves <palves@redhat.com> writes:

Pedro> Yep, that was the plan.  I'll take that as confirmation that designated
Pedro> initializers are not known to be, or should be working through some
Pedro> other means.  :-)  Thanks.

Yeah, sorry about this -- I actually thought about the C designated
initializer issue when ok'ing the series, but I neglected to mention
this in my review.

It isn't totally clear that we want to support this feature.  In C it is
just for initializers, not in arbitrary expressions.

In gdb terms you could only use it to assign to an existing struct
object, because the evaluator would need an "expected type".

So, it seemed marginal to me; and on top of that it is unclear whether
the old code is at all useful to C anyway -- hence my ok.

I don't object to a PR of course, and if somebody implements it I won't
reject it -- I think it is just marginal, but not truly bad :)

Tom


^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [PATCH 1/6] Remove OP_LABELED.
  2012-11-02 17:09         ` Tom Tromey
@ 2012-11-02 17:25           ` Pedro Alves
  0 siblings, 0 replies; 20+ messages in thread
From: Pedro Alves @ 2012-11-02 17:25 UTC (permalink / raw)
  To: Tom Tromey; +Cc: Yao Qi, gdb-patches

On 11/02/2012 05:09 PM, Tom Tromey wrote:

> I don't object to a PR of course, and if somebody implements it I won't
> reject it -- I think it is just marginal, but not truly bad :)

Alright, here it is:

http://sourceware.org/bugzilla/show_bug.cgi?id=14800

-- 
Pedro Alves


^ permalink raw reply	[flat|nested] 20+ messages in thread

end of thread, other threads:[~2012-11-02 17:25 UTC | newest]

Thread overview: 20+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2012-11-01  7:47 [PATCH 0/6] Remove unused operator OP_LABELED Yao Qi
2012-11-01  7:47 ` [PATCH 4/6] Remove 'substruct_type' and 'subfieldno' Yao Qi
2012-11-01 15:44   ` Tom Tromey
2012-11-01  7:47 ` [PATCH 6/6] Test nested struct assign Yao Qi
2012-11-01 15:47   ` Tom Tromey
2012-11-01  7:47 ` [PATCH 2/6] Update comment of evaluate_struct_tuple Yao Qi
2012-11-01 15:42   ` Tom Tromey
2012-11-01  7:47 ` [PATCH 1/6] Remove OP_LABELED Yao Qi
2012-11-01 15:41   ` Tom Tromey
2012-11-02 16:43   ` Pedro Alves
2012-11-02 16:49     ` Tom Tromey
2012-11-02 16:51       ` Pedro Alves
2012-11-02 17:09         ` Tom Tromey
2012-11-02 17:25           ` Pedro Alves
2012-11-01  7:47 ` [PATCH 3/6] Remove 'variantno' Yao Qi
2012-11-01 15:44   ` Tom Tromey
2012-11-01  7:47 ` [PATCH 5/6] Indentation Yao Qi
2012-11-01 15:45   ` Tom Tromey
2012-11-01 15:47 ` [PATCH 0/6] Remove unused operator OP_LABELED Tom Tromey
2012-11-02  0:20   ` [committed]: " Yao Qi

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