Re: [C/ObjC/C++/ObjC++] cleanup diagnostics initialization
I created PR 53061 to track this. I prefer to focus on other things, so feel free to take the patch there and do whatever you need to get it approved. Cheers, Manuel.
[C/ObjC/C++/ObjC++] cleanup diagnostics initialization
This patch cleans up the diagnostic initialization of the C-family FEs. It keeps the default of no line-wrapping. It moves together all initializations and deletes code that has no effect. Bootstrapped and regression tested on x86-unknown-linux-gnu with enable-languages=all,ada,obj-c++. OK? 2012-04-09 Manuel López-Ibáñez m...@gcc.gnu.org * doc/invoke.texi (fmessage-length): New. * pretty-print.h (getenv_columns): New. * c-objc-common.c (c_objc_common_init): Do not do diagnostics initialization here. c-family/ * c-opts.c (c_common_initialize_diagnostics): Rename as c_common_diagnostics_defaults. Set defaults here. * c-common.h (c_common_initialize_diagnostics): Likewise. cp/ * cp-objcp-common.c (cxx_initialize_diagnostics): Move from here to ... * error.c: ... here. (init_error): Delete. * cp-tree.h (init_error): Delete. * lex.c (cxx_init): Do not call init_error. * cxx-pretty-print.c (pp_cxx_pretty_printer_init): Do not set default message length here. diagnostic-init.diff Description: Binary data
Re: [C/ObjC/C++/ObjC++] cleanup diagnostics initialization
On Mon, Apr 9, 2012 at 5:12 AM, Manuel López-Ibáñez lopeziba...@gmail.com wrote: This patch cleans up the diagnostic initialization of the C-family FEs. It keeps the default of no line-wrapping. It moves together all initializations and deletes code that has no effect. Bootstrapped and regression tested on x86-unknown-linux-gnu with enable-languages=all,ada,obj-c++. OK? 2012-04-09 Manuel López-Ibáñez m...@gcc.gnu.org * doc/invoke.texi (fmessage-length): New. This ChangeLog description should be Update; not New. * pretty-print.h (getenv_columns): New. * c-objc-common.c (c_objc_common_init): Do not do diagnostics initialization here. c-family/ * c-opts.c (c_common_initialize_diagnostics): Rename as c_common_diagnostics_defaults. Set defaults here. * c-common.h (c_common_initialize_diagnostics): Likewise. Make the comment less personal; we don't who I is in I'm putting them here in three months (nor should we have to know.) I suggest to just remove that comment. cp/ * cp-objcp-common.c (cxx_initialize_diagnostics): Move from here to ... * error.c: ... here. Replace the free with XDELETE, to match usage elsewhere. (init_error): Delete. This deletion moves the initialization of cxx_pp to cxx_initialize_diagnostics. That is the wrong place. As the comment says, cxx_pp is not for diagnostics, so it should be initialized separately -- if possible as early as possible. * cp-tree.h (init_error): Delete. * lex.c (cxx_init): Do not call init_error. this should still call a routine that initializes cxx_pp. * cxx-pretty-print.c (pp_cxx_pretty_printer_init): Do not set default message length here.
Re: [C/ObjC/C++/ObjC++] cleanup diagnostics initialization
On Mon, Apr 9, 2012 at 6:19 AM, Manuel López-Ibáñez lopeziba...@gmail.com wrote: On 9 April 2012 12:43, Gabriel Dos Reis g...@integrable-solutions.net wrote: On Mon, Apr 9, 2012 at 5:12 AM, Manuel López-Ibáñez lopeziba...@gmail.com wrote: * c-common.h (c_common_initialize_diagnostics): Likewise. Make the comment less personal; we don't who I is in I'm putting them here in three months (nor should we have to know.) I suggest to just remove that comment. That comment has been there for ages, I just moved it around. But I am happy to remove it. yes, thanks. This deletion moves the initialization of cxx_pp to cxx_initialize_diagnostics. That is the wrong place. As the comment says, cxx_pp is not for diagnostics, so it should be initialized separately -- if possible as early as possible. * cp-tree.h (init_error): Delete. * lex.c (cxx_init): Do not call init_error. this should still call a routine that initializes cxx_pp. Where? Maybe in a dedicated function called construct_cxx_pp(), called from cxx_init? cxx_initialize_diagnostics is run earlier than cxx_init, so it is now initialized earlier than before. Moreover, by putting both together, it is clear to anyone that there are two pretty-printers, and the comment clarifies what the second is for. I understand that init_error means initialize_error routines, and indeed it contained code initializing diagnostics. since it has nothing to do with diagnostics, it is better not to place its initialization there. Otherwise, in 3 months somewhere will come and complain that the diagnostics and pretty printers are hard to debug etc ;-) However, if the above does not convince you. What about renaming init_error to cxx_pp_init, and move the cxx_pp initialization there so it is clear that this function is ONLY to initialize cxx_pp and not for anything else? that or the suggestion above. Is the patch OK with the above changes? I appreciate your impatience but I would like to look at the revised version first.