On Tue, Apr 10, 2012 at 2:37 PM, Manuel López-Ibáñez
<lopeziba...@gmail.com> wrote:
> On 10 April 2012 20:52, Gabriel Dos Reis <g...@integrable-solutions.net> 
> wrote:
>> On Tue, Apr 10, 2012 at 11:46 AM, Manuel López-Ibáñez
>> <lopeziba...@gmail.com> wrote:
>>> On 10 April 2012 00:28, Jason Merrill <ja...@redhat.com> wrote:
>>>> On 04/09/2012 04:01 PM, Manuel López-Ibáńez wrote:
>>>>>
>>>>> * It uses  the default cutoff as max_width, whatever it is (as
>>>>> controlled by -fmessage-length).
>>>>> * It uses the pretty-printer. The text cannot (should not) wrap
>>>>> because we still print only max_width chars at most.
>>>>
>>>>
>>>> Hmm, I think if pp_line_cutoff is 0 and we're on a terminal, we still want
>>>> to use COLUMNS to limit how much of the source we print.
>>>
>>> Like this?
>>
>> There is a novelty in this new version that I don't think we discussed
>> before: automatic expansion of tabs to 8 hard space characters.  That
>> number should not be hardcoded as there is no reason to believe a tab
>> character always expands to 8 space characters.  You should check
>> the environment first; if not present the default expansion number should
>> be a symbolic constant as opposed to a magic number sprinkled all over the
>> places.  I would also encourage you to introduce more abstraction to
>> reduce the size of diagnostic_show_locus.
>
> There is no novelty, it has been there from the beginning, and now I
> realize it was a mistake to do the extra work to implement this.

I wonder how I missed it in the previous reviews.

>
> The GCS says: "Calculate column numbers assuming that space and all
> ASCII printing characters have equal width, and assuming tab stops
> every 8 columns"
> http://www.gnu.org/prep/standards/standards.html#Errors

That is GNU coding convention for GNU codes.  We do not require that
programs GCC accept have to adhere to GNU coding convention, not
should we.  An important aspect of diagnostic with caret is that we are
writing back to the user what she wrote, not what GNU wanted her
to write.

> So, in fact, libcpp is buggy for not implementing this (as can be seen
> in emacs). If/When libcpp is fixed, the column info will be correct
> for tabs. And then, one may care about printing tabs as anything
> different from a single space. But until then, a single space is a
> good as anything. In fact this is what is done in c-ppoutput.c.
>

I would not necessarily say that libcpp is buggy in this aspect.
However, I do agree that removing the tab expansion simplifies
the code and is better.

Reply via email to