Re: [PATCH v4] GDB PR tdep/8282: MIPS: Wire in `set disassembler-options'

[Simon Marchi <simon.marchi@polymtl.ca>]

On 2018-06-21 15:55, Maciej W. Rozycki wrote:
>  I think you chose the wrong example, after all what's the problem for
> tools to notice that "foo (bar)" and "foo (bar) (timeout)" are the same
> test?  If the number of parenthesis pairs is different between two
> messages, then strip the extra ones along with the text between from 
> the
> message that has more of them and only then compare the resulting 
> strings.

We can do that if we know that they are supposed to be the different 
test.  But how do we know whether they are different tests or the same 
test with a special outcome in the second case?

>  What the problem would be are different failures in different runs, 
> e.g.
> if we have "foo (bar) (timeout)" in one and "foo (bar) (internal 
> error)"
> in the other one, then indeed we have an issue.  We could strip
> parenthesis pairs one by one from both messages at a time, but then 
> we'd
> incorrectly consider "foo (bar)" and "foo (baz)" the same test.
> 
>  Does it matter in reality?  I suppose so, given the outcome of the
> discussion referred from the wiki, although I don't see the scenario I
> have outlined here actually mentioned there (parts of the discussion 
> seem
> to be missing though from the archive).

The only purpose I am aware of is the script on the buildbot that 
analyzes results.  It would show a timeout as

   new FAIL: foo (timeout)

instead of now:

   PASS -> FAIL: foo

So it's not terribly important, but it helps making the analysis output 
a bit better.

>> That does not really apply to your case though.  I think here you can 
>> just
>> remove the parenthesis, and maybe add a comma.
>> 
>>   mips_disassemble_test bar "move\tv0,a4" "disassemble ABI, n64"
>> 
>> As long as it's clear.
> 
>  OK, works for me.  I'll make that adjustment as I commit the change 
> once
> the binutils parts have been approved.
> 
>  Thank you for your review.
> 
>   Maciej

Thanks,

Simon