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.