On Sun, Apr 8, 2012 at 11:10 AM, Manuel López-Ibáñez
<lopeziba...@gmail.com> wrote:
> On 8 April 2012 17:14, Gabriel Dos Reis <g...@integrable-solutions.net> wrote:
>> On Sun, Apr 8, 2012 at 7:06 AM, Manuel López-Ibáñez
>> <lopeziba...@gmail.com> wrote:
>>> On 8 April 2012 06:09, Jason Merrill <ja...@redhat.com> wrote:
>>>> On 04/07/2012 06:29 PM, Manuel López-Ibáńez wrote:
>>>>>
>>>>> I'll be happy to change it to whatever is more understandable. I think
>>>>> in CSS is called "padding".
>>>>
>>>>
>>>> That wouldn't be any clearer; my confusion was that I thought you were
>>>> padding the right side of the source line, but you aren't; you are only
>>>> making sure that the caret isn't too close to the edge of the terminal. No
>>>> need to change anything here.
>>>>
>>>>> +getenv_columns (void)
>>>>
>>>>
>>>> I had been thinking to check COLUMNS once at the beginning of compilation;
>>>> I
>>>> don't think the value can change while the compiler is running since we
>>>> don't respond to SIGWINCH. And let's use this value in
>>>> c_common_initialize_diagnostics, too.
>>>
>>> That code is useless.
>>
>> I do not understand what you by that. Could you elaborate?
>> c-family/ChangeLog says it was split out of c_common_init_options.
>
> I mean it has no effect because cxx_initialize_diagnostics overrides
> it before returning. Step in that function and you will see
> diagnostic_line_cutoff (context) change from 0 to 80 to 0.
OK.
>
>>> It is overridden by cxx_initialize_diagnostics
>>> (line 2429). It is not strange that nobody noticed this because the
>>> pretty-printer code is hard to understand and impossible to debug.
>>
>> I find the syllogism a bit specious.
>
> Sorry, I didn't express myself in the best way out of frustration. I
> mean that the pretty-printer code is quite complex since there are
> many interacting parts, it emulates inheritance in plain C, and there
> is no encapsulation, so variables are sometimes modified directly and
> other times by using various functions. The abundant use of
> preprocessor macros as accessors and for implementing inheritance
> makes quite hard to debug the code in gdb. But this is a well-known
> problem of the whole GCC codebase. Probably, there is no better way to
> implement the same functionality in plain C, or perhaps gdb is not
> powerful enough, or perhaps I am using it incorrectly. In any case, I
> am having a very hard time following how the various pretty-printers
> are initialized.
Many of the pretty printer macros were introduced to abstract away
direct access to the fields. The fact that some fields are accessed
directly in client codes is a bug in client codes, not a feature of the
pretty printer interface. Alas C does not provide a convenient access
control, so these access violations go undetected when they slip
through careful reviews. GDB has support for macros but it true
that debugging macro-based interfaces require some gymnastics,
and this one is no exception.
>
>>> I'd rather not touch it if I can avoid it.
>>>
>>> I am not even sure if that is the correct way to set the line-cut-off
>>> in PP. The option fmessage-length uses pp_set_line_maximum_length. Who
>>> knows what is the correct way?
>>
>> The documentation of pp_base_set_line_maximum_length says:
>>
>> /* Sets the number of maximum characters per line PRETTY-PRINTER can
>> output in line-wrapping mode. A LENGTH value 0 suppresses
>> line-wrapping. */
>
> So when is it appropriate to use
>
> diagnostic_line_cutoff (context) = 80;
>
> like in the above code?
First let me say this: if you are doing diagnostics with caret, then you do
not want line wrapping, otherwise it gets a bit ugly. So you need
a way to tell the diagnostic outputter that you do not want line wrap.
diagnostic_line_cutoff suggests the maximum number of characters that
the diagnostic outputter could send before emitting a newline to begin
line wrap. 0 suggests no line wrap.
So, I would not try to reuse that field -- otherwise, it can get messed up.
You suggested in your previous mail to use a distinct field to store the
width the terminal; I think that is a better option.
>
>>> Also, using COLUMNS to initialize pp's line-cut-off will change the
>>> current default (which is unlimited despite what invoke.texi says).
>>>
>>> I can add "locus_max_width" to diagnostic_context, and use
>>> getenv_columns to initialize it once. Would that be ok?
>>
>> I am not sure how its semantics would differ from pp_line_cutoff
>> but if you are going to disable line-wrapping mode (which this
>> effectively does), then the structure pp_wrapping_mode_t is
>> where to place this kind of information.
>
> Actually, what Jason is proposing is to initialize line_cutoff to
> getevn("COLUMNS") if present or to unlimited if not, and then use that
> for the caret diagnostic. I see two problems with this:
>
> * We change the current default, which is no line-wrapping ever.
>
> * Where should the default be set? By each FE? by diagnostic.c? by
> pretty-printer.c?
>
I would suggest the front-ends that want it. If all front ends want it
then it makes sense to set it once for all in diagnostic.c.
