This is the mail archive of the gcc-patches@gcc.gnu.org mailing list for the GCC project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

Re: [PATCH] Caret diagnostics


On 8 April 2012 17:14, Gabriel Dos Reis <gdr@integrable-solutions.net> wrote:
> On Sun, Apr 8, 2012 at 7:06 AM, Manuel LÃpez-IbÃÃez
> <lopezibanez@gmail.com> wrote:
>> On 8 April 2012 06:09, Jason Merrill <jason@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.

>> 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.

>> Â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?

>> 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?

Cheers,

Manuel.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]