[PATCH v2 15/17] MIPS: relax gas testsuite whitespace expectations

Maciej W. Rozycki macro@orcam.me.uk
Mon Jul 15 12:12:11 GMT 2024


On Fri, 12 Jul 2024, Jan Beulich wrote:

> In a subsequent change the scrubber is going to be changed to retain
> further whitespace. Test case expectations generally would better not
> depend on the specific whitespace treatment by the scrubber, unless of
> course a test is specifically about it. Adjust relevant test cases to
> permit blanks where those will subsequently appear.

 I disagree.  I understand you might be unhappy to have to tediously go 
and mechanically update a bunch of test cases because the spacing has 
changed (well, actually you don't have: you can just produce replacement 
dumps and just verify that the diff to the previous one is sane, which is 
what I do on such occasions).  But this is occasional effort, not expected 
to happen often, however the extra regexp characters make test patterns 
harder for a human to read.

 Therefore I would rather the updated patterns reflected new formatting 
verbatim, at least in the MIPS target I care about, and if we have a need 
to change formatting again in 10 years' time, then so be it.

> ---
> This is adding just the blanks that are going to be needed; imo it would
> generally be better if test case expectations were, from the very
> beginning, written to focus on what is being tested, rather than taking
> verbatim copies of the respective tool's output.

 I'm sorry, how could the test case writer predict that extra whitespace 
might be added in the future?  This statement makes no sense to me, this 
way all tests should be rewritten such as to possibly expect whitespace 
between tokens where there currently isn't any.

 Also I think if you retain whitespace, you should be doing this 
consistently, i.e. if the source instruction is say:

	dext	$2, $3, 1 << 2, 1 << 4

then in error reporting this should be used:

dext $2, $3, 1 << 2, 1 << 4

rather than this:

dext $2,$3,1 << 2,1 << 4

which makes the instruction reported hard to read/parse to a human, as you 
suddenly have whitespace within operands, but not between them.  I guess 
this is something to handle in 17/17.

 NAK for this part then.

  Maciej


More information about the Binutils mailing list