Commit Message
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
Comments
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
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;
}
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
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
@@ -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