Patchwork Record nested types

login
register
mail settings
Submitter Keith Seitz
Date Nov. 30, 2017, 11:28 p.m.
Message ID <e8c180d6-5340-e3b8-5876-aa5034ecb288@redhat.com>
Download mbox | patch
Permalink /patch/24649/
State New
Headers show

Comments

Keith Seitz - Nov. 30, 2017, 11:28 p.m.
On 11/28/2017 06:39 AM, Pedro Alves wrote:
>>> W00t, lots of repetition.  OOC, did you consider using the preprocessor
>>> to build the types?
>>
>> Preprocessor? I used the logging feature of the test script to generate the struct. [When the log is enabled, it sends the entire struct (up to the nested type limit) to gdb.log.]
> 
> Ah, that's even better.  Could you add a comment at the top of
> the .cc file, so that the next person that needs to change/extend
> the file doesn't try to do it manually?

In fact, I've added a proc in the .exp file to generate the source code (sans GPL), but as I usually prefer, this only runs manually to generate the (entire) source file now. I've updated the .cc with the output of this new "make_source" procedure. [I've appended the diff between previous version of .exp below.]

> Nope, that's fine; I had assumed you had typed the whole thing!  :-P

:-)

>> I've rewritten this whole thing a little more clearly based on your suggestions.
>>
>>>
>>>   # stack pop $sid1;    # returns "b"
>>>   # stack pop $sid1;    # returns "a"
>>>   # stack pop $sid1;    # returns ""
>>>
>>> Though off hand I'd assume that poping from an empty
>>> stack would be an error instead.  But perhaps that's more
>>> common in TCL?  Or does it not work to push an empty
>>> element, like:
>>>
>>>   # stack push $sid1 ""
>>>
>>> ?
>>
>> Yeah, with the submitted patch as-is, empty string elements cannot be pushed onto stacks/queues. [Well, they can, but without using the empty proc, one cannot tell the difference. I've therefore changed both stack and queue to error when an empty stack/queue is popped.
> 
> Thanks.
> 
> One thing that crossed my mind is that it would be really nice
> to have unit tests for these tcl data structures we come up with.
> E.g., add a new gdb.whatever/testsuite-selftests.exp or something
> like that, that runs just as any other GDB testcase, but that instead
> of spawning GDB, it exercises the new APIs, and calls pass/fail
> accordingly.  That'd serve both to unsure test coverage of the whole API,
> and also as kind of API documentation, as folks that want to know how
> to use the API would find easy/isolated examples there.
> 
> Anyway, that's a IWBN; it can be done separately and I don't want
> to delay this patch further.

I'll add a TODO for that. This certainly can't hurt. [The good news is, data-structures.exp is completely independent of our testsuite. To double-check everything, I just loaded it into a vanilla tclsh.]


>> -/* Add a typedef defined in the scope of the FIP's class.  */
>> +/* Can the type given by DIE define another type?  */
>> +
>> +static bool
>> +type_can_define_types (const struct die_info *die)
>> +{
>> +  switch (die->tag)
>> +    {
>> +    case DW_TAG_typedef:
>> +    case DW_TAG_class_type:
>> +    case  DW_TAG_structure_type:
> 
> Spurious double space above.

Fixed.

>> +# A namespace/commands to support a queue.
>> +#
>> +# To create a queue, call ::Queue::new, recording the returned queue ID
>> +# for future calls to manipulate the queue object.
>> +#
>> +# Example:
>> +#
>> +# set qid [::Queue::new]
>> +# queue push $qid a
>> +# queue push $qid b
>> +# queue empty $qid;  # returns false
>> +# queue pop $qid;    # returns "b"
>> +# queue pop $qid;    # returns "a"
> 
> I assume the first pop returns "a" instead.  :-P
> 

Oh boy. Cut-n-paste strikes again!

If you'd like me to submit in full again, please let me know. Otherwise, I've appended the diff for nested-types.exp with changes necessary to get "output source file" working. I apparently broke it along the way.

Keith
Pedro Alves - Dec. 1, 2017, 12:36 p.m.
On 11/30/2017 11:28 PM, Keith Seitz wrote:
> On 11/28/2017 06:39 AM, Pedro Alves wrote:
>>>> W00t, lots of repetition.  OOC, did you consider using the preprocessor
>>>> to build the types?
>>>
>>> Preprocessor? I used the logging feature of the test script to generate the struct. [When the log is enabled, it sends the entire struct (up to the nested type limit) to gdb.log.]
>>
>> Ah, that's even better.  Could you add a comment at the top of
>> the .cc file, so that the next person that needs to change/extend
>> the file doesn't try to do it manually?
> 
> In fact, I've added a proc in the .exp file to generate the source code (sans GPL), but as I usually prefer, this only runs manually to generate the (entire) source file now. I've updated the .cc with the output of this new "make_source" procedure. [I've appended the diff between previous version of .exp below.]

...

> If you'd like me to submit in full again, please let me know. Otherwise, I've appended the diff for nested-types.exp with changes necessary to get "output source file" working. I apparently broke it along the way.

It would have been nice to have seen what the new .cc looks like too.

But if the end result includes a comment in the .cc files mentioned that
this is a generated file (maybe even include a "-*- buffer-read-only: t"
marker at the top), then this is fine with me.

Thanks,
Pedro Alves
Keith Seitz - Dec. 1, 2017, 6:20 p.m.
On 12/01/2017 04:36 AM, Pedro Alves wrote:
>> If you'd like me to submit in full again, please let me know. Otherwise, I've appended the diff for nested-types.exp with changes necessary to get "output source file" working. I apparently broke it along the way.
> 
> It would have been nice to have seen what the new .cc looks like too.
> 
> But if the end result includes a comment in the .cc files mentioned that
> this is a generated file (maybe even include a "-*- buffer-read-only: t"
> marker at the top), then this is fine with me.

How does this look?

Keith

/* Code in this file is generated.  -*- buffer-read-only: t -*- vi:set ro:
   See the procedure `make_source' in nested-types.exp.  */

/* This testcase is part of GDB, the GNU debugger.

   Copyright 2017 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/>.  */

struct S10 {
  enum E10 {A10, B10, C10};
  union U10 {
    int a;
    char c;
  };

  int i10;
  E10 e10;
  U10 u10;

  struct S11 {
    enum E11 {A11, B11, C11};
    union U11 {
      int a;
      char c;
    };

    int i11;
    E11 e11;
    U11 u11;

    struct S12 {
      enum E12 {A12, B12, C12};
      union U12 {
        int a;
        char c;
      };

      int i12;
      E12 e12;
      U12 u12;

      struct S13 {
        enum E13 {A13, B13, C13};
        union U13 {
          int a;
          char c;
        };

        int i13;
        E13 e13;
        U13 u13;

        struct S14 {
          enum E14 {A14, B14, C14};
          union U14 {
            int a;
            char c;
          };

          int i14;
          E14 e14;
          U14 u14;

          struct S15 {
            enum E15 {A15, B15, C15};
            union U15 {
              int a;
              char c;
            };

            int i15;
            E15 e15;
            U15 u15;

            struct S16 {
              enum E16 {A16, B16, C16};
              union U16 {
                int a;
                char c;
              };

              int i16;
              E16 e16;
              U16 u16;

              struct S17 {
                enum E17 {A17, B17, C17};
                union U17 {
                  int a;
                  char c;
                };

                int i17;
                E17 e17;
                U17 u17;

                struct S18 {
                  enum E18 {A18, B18, C18};
                  union U18 {
                    int a;
                    char c;
                  };

                  int i18;
                  E18 e18;
                  U18 u18;

                  struct S19 {
                    enum E19 {A19, B19, C19};
                    union U19 {
                      int a;
                      char c;
                    };

                    int i19;
                    E19 e19;
                    U19 u19;

                  };
                };
              };
            };
          };
        };
      };
    };
  };
  struct S21 {
    enum E21 {A21, B21, C21};
    union U21 {
      int a;
      char c;
    };

    int i21;
    E21 e21;
    U21 u21;

    struct S22 {
      enum E22 {A22, B22, C22};
      union U22 {
        int a;
        char c;
      };

      int i22;
      E22 e22;
      U22 u22;

      struct S23 {
        enum E23 {A23, B23, C23};
        union U23 {
          int a;
          char c;
        };

        int i23;
        E23 e23;
        U23 u23;

        struct S24 {
          enum E24 {A24, B24, C24};
          union U24 {
            int a;
            char c;
          };

          int i24;
          E24 e24;
          U24 u24;

          struct S25 {
            enum E25 {A25, B25, C25};
            union U25 {
              int a;
              char c;
            };

            int i25;
            E25 e25;
            U25 u25;

            struct S26 {
              enum E26 {A26, B26, C26};
              union U26 {
                int a;
                char c;
              };

              int i26;
              E26 e26;
              U26 u26;

              struct S27 {
                enum E27 {A27, B27, C27};
                union U27 {
                  int a;
                  char c;
                };

                int i27;
                E27 e27;
                U27 u27;

                struct S28 {
                  enum E28 {A28, B28, C28};
                  union U28 {
                    int a;
                    char c;
                  };

                  int i28;
                  E28 e28;
                  U28 u28;

                  struct S29 {
                    enum E29 {A29, B29, C29};
                    union U29 {
                      int a;
                      char c;
                    };

                    int i29;
                    E29 e29;
                    U29 u29;

                  };
                };
              };
            };
          };
        };
      };
    };
  };
  struct S31 {
    enum E31 {A31, B31, C31};
    union U31 {
      int a;
      char c;
    };

    int i31;
    E31 e31;
    U31 u31;

    struct S32 {
      enum E32 {A32, B32, C32};
      union U32 {
        int a;
        char c;
      };

      int i32;
      E32 e32;
      U32 u32;

      struct S33 {
        enum E33 {A33, B33, C33};
        union U33 {
          int a;
          char c;
        };

        int i33;
        E33 e33;
        U33 u33;

        struct S34 {
          enum E34 {A34, B34, C34};
          union U34 {
            int a;
            char c;
          };

          int i34;
          E34 e34;
          U34 u34;

          struct S35 {
            enum E35 {A35, B35, C35};
            union U35 {
              int a;
              char c;
            };

            int i35;
            E35 e35;
            U35 u35;

            struct S36 {
              enum E36 {A36, B36, C36};
              union U36 {
                int a;
                char c;
              };

              int i36;
              E36 e36;
              U36 u36;

              struct S37 {
                enum E37 {A37, B37, C37};
                union U37 {
                  int a;
                  char c;
                };

                int i37;
                E37 e37;
                U37 u37;

                struct S38 {
                  enum E38 {A38, B38, C38};
                  union U38 {
                    int a;
                    char c;
                  };

                  int i38;
                  E38 e38;
                  U38 u38;

                  struct S39 {
                    enum E39 {A39, B39, C39};
                    union U39 {
                      int a;
                      char c;
                    };

                    int i39;
                    E39 e39;
                    U39 u39;

                  };
                };
              };
            };
          };
        };
      };
    };
  };
  struct S41 {
    enum E41 {A41, B41, C41};
    union U41 {
      int a;
      char c;
    };

    int i41;
    E41 e41;
    U41 u41;

    struct S42 {
      enum E42 {A42, B42, C42};
      union U42 {
        int a;
        char c;
      };

      int i42;
      E42 e42;
      U42 u42;

      struct S43 {
        enum E43 {A43, B43, C43};
        union U43 {
          int a;
          char c;
        };

        int i43;
        E43 e43;
        U43 u43;

        struct S44 {
          enum E44 {A44, B44, C44};
          union U44 {
            int a;
            char c;
          };

          int i44;
          E44 e44;
          U44 u44;

          struct S45 {
            enum E45 {A45, B45, C45};
            union U45 {
              int a;
              char c;
            };

            int i45;
            E45 e45;
            U45 u45;

            struct S46 {
              enum E46 {A46, B46, C46};
              union U46 {
                int a;
                char c;
              };

              int i46;
              E46 e46;
              U46 u46;

              struct S47 {
                enum E47 {A47, B47, C47};
                union U47 {
                  int a;
                  char c;
                };

                int i47;
                E47 e47;
                U47 u47;

                struct S48 {
                  enum E48 {A48, B48, C48};
                  union U48 {
                    int a;
                    char c;
                  };

                  int i48;
                  E48 e48;
                  U48 u48;

                  struct S49 {
                    enum E49 {A49, B49, C49};
                    union U49 {
                      int a;
                      char c;
                    };

                    int i49;
                    E49 e49;
                    U49 u49;

                  };
                };
              };
            };
          };
        };
      };
    };
  };
  struct S51 {
    enum E51 {A51, B51, C51};
    union U51 {
      int a;
      char c;
    };

    int i51;
    E51 e51;
    U51 u51;

    struct S52 {
      enum E52 {A52, B52, C52};
      union U52 {
        int a;
        char c;
      };

      int i52;
      E52 e52;
      U52 u52;

      struct S53 {
        enum E53 {A53, B53, C53};
        union U53 {
          int a;
          char c;
        };

        int i53;
        E53 e53;
        U53 u53;

        struct S54 {
          enum E54 {A54, B54, C54};
          union U54 {
            int a;
            char c;
          };

          int i54;
          E54 e54;
          U54 u54;

          struct S55 {
            enum E55 {A55, B55, C55};
            union U55 {
              int a;
              char c;
            };

            int i55;
            E55 e55;
            U55 u55;

            struct S56 {
              enum E56 {A56, B56, C56};
              union U56 {
                int a;
                char c;
              };

              int i56;
              E56 e56;
              U56 u56;

              struct S57 {
                enum E57 {A57, B57, C57};
                union U57 {
                  int a;
                  char c;
                };

                int i57;
                E57 e57;
                U57 u57;

                struct S58 {
                  enum E58 {A58, B58, C58};
                  union U58 {
                    int a;
                    char c;
                  };

                  int i58;
                  E58 e58;
                  U58 u58;

                  struct S59 {
                    enum E59 {A59, B59, C59};
                    union U59 {
                      int a;
                      char c;
                    };

                    int i59;
                    E59 e59;
                    U59 u59;

                  };
                };
              };
            };
          };
        };
      };
    };
  };
};

int
main ()
{
  S10 s10;
  S10::S11 s11;
  S10::S11::S12 s12;
  S10::S11::S12::S13 s13;
  S10::S11::S12::S13::S14 s14;
  S10::S11::S12::S13::S14::S15 s15;
  S10::S11::S12::S13::S14::S15::S16 s16;
  S10::S11::S12::S13::S14::S15::S16::S17 s17;
  S10::S11::S12::S13::S14::S15::S16::S17::S18 s18;
  S10::S11::S12::S13::S14::S15::S16::S17::S18::S19 s19;

  S10::S21 s21;
  S10::S21::S22 s22;
  S10::S21::S22::S23 s23;
  S10::S21::S22::S23::S24 s24;
  S10::S21::S22::S23::S24::S25 s25;
  S10::S21::S22::S23::S24::S25::S26 s26;
  S10::S21::S22::S23::S24::S25::S26::S27 s27;
  S10::S21::S22::S23::S24::S25::S26::S27::S28 s28;
  S10::S21::S22::S23::S24::S25::S26::S27::S28::S29 s29;

  S10::S31 s31;
  S10::S31::S32 s32;
  S10::S31::S32::S33 s33;
  S10::S31::S32::S33::S34 s34;
  S10::S31::S32::S33::S34::S35 s35;
  S10::S31::S32::S33::S34::S35::S36 s36;
  S10::S31::S32::S33::S34::S35::S36::S37 s37;
  S10::S31::S32::S33::S34::S35::S36::S37::S38 s38;
  S10::S31::S32::S33::S34::S35::S36::S37::S38::S39 s39;

  S10::S41 s41;
  S10::S41::S42 s42;
  S10::S41::S42::S43 s43;
  S10::S41::S42::S43::S44 s44;
  S10::S41::S42::S43::S44::S45 s45;
  S10::S41::S42::S43::S44::S45::S46 s46;
  S10::S41::S42::S43::S44::S45::S46::S47 s47;
  S10::S41::S42::S43::S44::S45::S46::S47::S48 s48;
  S10::S41::S42::S43::S44::S45::S46::S47::S48::S49 s49;

  S10::S51 s51;
  S10::S51::S52 s52;
  S10::S51::S52::S53 s53;
  S10::S51::S52::S53::S54 s54;
  S10::S51::S52::S53::S54::S55 s55;
  S10::S51::S52::S53::S54::S55::S56 s56;
  S10::S51::S52::S53::S54::S55::S56::S57 s57;
  S10::S51::S52::S53::S54::S55::S56::S57::S58 s58;
  S10::S51::S52::S53::S54::S55::S56::S57::S58::S59 s59;
  return 0;
}
Pedro Alves - Dec. 1, 2017, 7:13 p.m.
On 12/01/2017 06:20 PM, Keith Seitz wrote:
> On 12/01/2017 04:36 AM, Pedro Alves wrote:
>>> If you'd like me to submit in full again, please let me know. Otherwise, I've appended the diff for nested-types.exp with changes necessary to get "output source file" working. I apparently broke it along the way.
>>
>> It would have been nice to have seen what the new .cc looks like too.
>>
>> But if the end result includes a comment in the .cc files mentioned that
>> this is a generated file (maybe even include a "-*- buffer-read-only: t"
>> marker at the top), then this is fine with me.
> 
> How does this look?

Looks fine, thanks.

Pedro Alves
Keith Seitz - Dec. 7, 2017, 11:03 p.m.
On 12/01/2017 11:13 AM, Pedro Alves wrote:
> On 12/01/2017 06:20 PM, Keith Seitz wrote:
>> How does this look?
> 
> Looks fine, thanks.

I've pushed this patch, thank you for your reviews.

Keith

Patch

diff --git a/gdb/testsuite/gdb.cp/nested-types.exp b/gdb/testsuite/gdb.cp/nested-types.exp
index 2b540e9907..6d387dd6be 100644
--- a/gdb/testsuite/gdb.cp/nested-types.exp
+++ b/gdb/testsuite/gdb.cp/nested-types.exp
@@ -86,6 +86,29 @@  proc qual_name {child parent_list} {
     }
 }
 
+# Output the test source to the log.
+
+proc make_source {} {
+    # Output the structure.
+    test_nested_limit 10 true
+
+    # Output main().
+    send_log "int\nmain \(\)\n\{\n"
+    set plist {}
+    for {set i 10} {$i < 60} {incr i} {
+	if {$i > 10 && [expr {$i % 10}] == 0} {
+	    incr i
+	    set plist {"S10"}
+	    send_log "\n"
+	}
+	send_log "  [qual_name S$i $plist] s$i;\n"
+	lappend plist "S$i"
+    }
+
+    send_log "  return 0;\n"
+    send_log "\}\n"
+}
+
 # Output to the log and/or create the result list for the fields of node ID.
 
 proc make_fields {result_var id parent_list indent_lvl log} {
@@ -159,25 +182,47 @@  proc node_result {result_var id limit parent_list indent_lvl log} {
 
     if {$log} {
 	indent $indent_lvl
-	send_log "struct $s \{\n"
+	send_log "struct $my_name \{\n"
+    } else {
+	# Add this node to the parent list so that its name appears in
+	# qualified names, but only if we are not logging.  Again, ptype and
+	# actual source code are not exactly interchangeable.
+	lappend parent_list "$my_name"
     }
 
-    # Add this node to the parent list so that its name appears in
-    # qualified names.
-    lappend parent_list "$my_name"
-
-    # Output field list to a local children list.
-    set children_list {}
-    make_fields children_list $id $parent_list [expr {$indent_lvl + 1}] $log
-
-    # Output type definitions to the local children list.
-    # The first number of ID gives us the depth of the node.
-    if {[string index $id 1] < $limit || $limit < 0} {
-	if {$log} {
+    # ptype outputs fields before type definitions, but in order to
+    # properly log the struct, we must output these in reverse.
+
+    if {!$log} {
+	# Output field list to a local children list.
+	set children_list {}
+	make_fields children_list $id $parent_list \
+	    [expr {$indent_lvl + 1}] $log
+
+	# Output type definitions to the local children list.
+	# The first number of ID gives us the depth of the node.
+	if {[string index $id 1] < $limit || $limit < 0} {
+	    make_enum children_list $id $parent_list \
+		[expr {$indent_lvl + 1}] $log
+	    make_union children_list $id $parent_list \
+		[expr {$indent_lvl + 1}] $log
+	}
+    } else {
+	# Output type definitions to the local children list.
+	# The first number of ID gives us the depth of the node.
+	if {[string index $id 1] < $limit || $limit < 0} {
+	    make_enum children_list $id $parent_list \
+		[expr {$indent_lvl + 1}] $log
+	    make_union children_list $id $parent_list \
+		[expr {$indent_lvl + 1}] $log
 	    send_log "\n"
 	}
-	make_enum children_list $id $parent_list [expr {$indent_lvl + 1}] $log
-	make_union children_list $id $parent_list [expr {$indent_lvl + 1}] $log
+
+	# Output field list to a local children list.
+	set children_list {}
+	make_fields children_list $id $parent_list \
+	    [expr {$indent_lvl + 1}] $log
+	send_log "\n"
     }
 
     # Output the children to the local children list.
@@ -205,35 +250,42 @@  proc node_result {result_var id limit parent_list indent_lvl log} {
 # Test nested type definitions.  LIMIT specifies how many nested levels
 # of definitions to test.  If LOG is true, output the tree to the log in
 # a human-readable format mimicing the source code.
+#
+# Only test when not logging.  Generating source code usable by the
+# test is not quite the same as how GDB outputs it.
 
 proc test_nested_limit {limit log} {
     set result {}
 
-    # Set the number of nested definitions to print.
-    gdb_test_no_output "set print type nested-type-limit $limit"
+    if {!$log} {
+	# Set the number of nested definitions to print.
+	gdb_test_no_output "set print type nested-type-limit $limit"
 
-    # Check the output of "show type print nested-type-limit"
-    if {$limit < 0} {
-	set lstr "unlimited"
+	# Check the output of "show type print nested-type-limit"
+	if {$limit < 0} {
+	    set lstr "unlimited"
+	} else {
+	    set lstr $limit
+	}
+	gdb_test "show print type nested-type-limit" \
+	    "Will print $lstr nested types defined in a class" \
+	    "show print type nested-type-limit ($limit)"
     } else {
-	set lstr $limit
-    }
-    gdb_test "show print type nested-type-limit" \
-	"Will print $lstr nested types defined in a class" \
-	"show print type nested-type-limit ($limit)"
-
-    # Generate the result list and test it.
-    if {$log} {
 	send_log "Tree to $limit levels:\n"
     }
+
+    # Generate the result list.
     node_result result 10 $limit {} 0 $log
 
-    # The only output we check for is the contents of the struct, ignoring
-    # the leading "type = struct S10 {" and trailing "}" of the outermost node.
-    set result [lindex $result 0]
-    lassign $result type access key name children
-    cp_test_ptype_class $name "ptype $name (limit = $limit)" $key \
-	$name $children
+    if {!$log} {
+	# The only output we check for is the contents of the struct,
+	# ignoring the leading "type = struct S10 {" and trailing "}" of
+	# the outermost node.
+	set result [lindex $result 0]
+	lassign $result type access key name children
+	cp_test_ptype_class $name "ptype $name (limit = $limit)" $key \
+	    $name $children
+    }
 }
 
 # Build a tree of nodes describing the structures in the source file.
@@ -264,4 +316,7 @@  for {set i 1} {$i < 9} {incr i} {
     test_nested_limit $i false
 }
 
+# To output the test code to the log, uncomment the following line:
+#make_source
+
 unset -nocomplain nodes result