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. > 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? 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. 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? Is the patch OK with the above changes?