Re: [patch] pr10659 pretty-printing: Display vectors of vectors as matrix

[Chris Moller <cmoller@redhat.com>]

On 05/27/10 17:03, Phil Muldoon wrote:
> On 05/27/2010 05:43 PM, Chris Moller wrote:
>    
>> This patch adds the mechanism for gdb to respond to "matrix" hints from
>> printers.py.  (Just to exercise the testcase, the patch includes a
>> patched version of printers.py to src/gdb/testsuite/gdb.python.  It can
>> be removed once the patched printers.py is generally available.)
>>      
>
> This is not an official review, I just have some questions.
>
>    
>> @@ -397,7 +397,7 @@
>> 		struct ui_file *stream, int recurse,
>>   		const struct value_print_options *options,
>>   		const struct language_defn *language,
>> -		int is_py_none)
>> +		int is_py_none, int is_matrix)
>>   {
>>      
>
> I'm not sure why it is necessary to pass the hint in here.  All of the
> other hints are parsed internally within the function?
>    

You mean the is_matrix parm?  That refers to the outer-most type of the 
value.  A matrix in this context is defined as a vector of vectors (of 
vectors (of vectors...))--that's what printers.py identifies to return a 
"matrix" hint.  is_matrix is just a flag indicating that, even when 
dealing with a subordinate element, like a nested vector or value, the 
outer-most type was identified as a matrix.

>
>    
>> -         if (is_py_none)
>> -           fputs_filtered ("{", stream);
>> -         else
>> -           fputs_filtered (" = {", stream);
>> +	  if (is_matrix&&  recurse == 0)
>> +	    print_spaces_filtered (2 + 2 * recurse, stream);
>> +	  if (is_py_none)
>> +	    {
>> +	      if (is_matrix&&  strcmp (hint, "array"))
>> +		{
>>      
>
> I looked at this, and I'm not sure what this means.  Either a hint is
> a matrix or an array? Is this trying to deal with children?  I looked
> at the modified vector printer and it can return either "array" or
> "matrix" depending on the template argument.
>    

Again, is_matrix refers to the outer-most type.  This test identifies 
the current element as an array within a matrix the elements of which 
are not themselves arrays--i.e., they're simple scalars.  It's there so 
that the inner-most array will print as

    {a b c d ...}

rather than

    {
         a b c d ...
    }



>
>    
>> +gdb_breakpoint [gdb_get_line_number "break"]
>> +gdb_continue_to_breakpoint "break"
>> +
>> +gdb_test "p test1" "vector of length 2, capacity 2 =.*"
>> +gdb_test "p test2" "= $nl  {$nl    {.*"
>> +gdb_test "p test3" "= $nl  {$nl    {$nl      {.*"
>> +
>>      
>
> I cannot deduce from this test what the actual content of a vector
> printed with the matrix hint should look like beyond the expected
> bracket structure.
>
> To be really picky, I'd at least like to see an example of the output
> in comments or a more comprehensive test.  I've really gotten into
> the idea that tests should also serve as an example of usage somehow.
>
>    

All I was interested in verifying was the structure and since matrix 
output consists of lots of lines, I didn't want to write the huge regex 
necessary to represent it.  I will if necessary, but it would be easier 
to stick the formatted output in the testcase as a comment.

>    
>> Index: testsuite/gdb.python/printers.py
>> ===================================================================
>> RCS file: testsuite/gdb.python/printers.py
>> diff -N testsuite/gdb.python/printers.py
>>      
>
> This file cannot exist in the GDB testsuite. It is already hosted
> in GCC. You should post any changes to the vector printer there.
> For your testcase, I would write the test so it does not rely on this
> file.
>    

The test doesn't work without a patched version of printers.py, so I 
thought I'd just stick a copy here until the "official" version gets 
promulgated and take it out later.

Chris

> Cheers,
>
> Phil
>