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.
