Re: [Diagnostic Patch] don't print column zero

[Eric Gallager <egall at gwmail dot gwu dot edu>]

On 10/26/17, Nathan Sidwell <nathan@acm.org> wrote:
> On 10/26/2017 10:34 AM, David Malcolm wrote:
>> [CCing Rainer and Mike for the gcc-dg.exp part]
>
>> Alternate idea: could show_column become a tri-state:
>>    * default: show non-zero columns
>>    * never: never show columns
>>    * always: always show a column, printing 0 for the no-column case
>> and then use "always" in our testsuite
>
> One of the things this patch shows up is the number of places where
> we're default accepting a zero column.  IMHO it is best to explicitly
> mark such tests.
>
>>> +      size_t l = sprintf (result, col ? ":%d:%d" : ":%d", line, col);
>>
>> Possibly a silly question, but is it OK to have a formatted string
>> call in which some of the arguments aren't consumed? (here "col" is only
>> consumed for the true case, which consumes 2 arguments; it's not consumed
>> for the false case).
>
> Yes.

I think I remember clang disagreeing; I remember it printing warnings
from -Wformat-extra-args in a similar situation in gnulib's
error_at_line module

>
>>> +      gcc_checking_assert (l + 1 < sizeof (result));
>>
>> Would snprintf be safer?
>
> I guess. but the assert's still needed.
>
>> Please create a selftest for the function, covering these cases:
>>
>> * line == 0
>> * line > 0 and col == 0
>> * line > 0 and col > 0 (checking output for these cases)
>> * line == INT_MAX and col == INT_MAX (without checking output, just to
>> tickle the assert)
>> * line == INT_MIN and col == INT_MIN (likewise)
>
> Ok, I'll investigate this new fangled self-testing framework :)
>
>> There are some testcases where we deliberately don't have a *line*
>> number; what happens to these?
>
> Those don't change.  the dg-harness already does NOT expect a column
> when lineno=0.
>
>> My Tcl skills aren't great, so hopefully someone else can review this;
>> CCing Rainer and Mike.
>>
>> Also, is the proposed syntax for "no columns" OK?  (note the tristate
>> idea above)
>
> I'm not wedded to '-:', but as mentioned above, I think the tests should
> be explicit about whether a column is expected or not (and the default
> needs to be 'expect column', because of history)
>
> thanks for your comments.
>
> nathan
>
> --
> Nathan Sidwell
>