On Thu, Apr 21, 2011 at 12:54 PM, Joseph S. Myers <jos...@codesourcery.com> wrote: > On Tue, 19 Apr 2011, Xinliang David Li wrote: > >> 2011-04-18 Neil Vachharajani <nvach...@gmail.com> >> >> * flags.c: New flag variable. >> * opts.c (common_handle_options): Set flag_werror_set. >> * opts-global.c (decode_options): Delay Werror decision >> for Wcoverage-mismatch util after options are parsed. > > This patch is certainly wrong, since common_handle_option must not set any > global state, only state pointed to by its arguments. > > That said, setting -Werror=coverage-mismatch in decode_options at all is > bad because decode_options is called when optimize attributes are > processed; as-is, a -Wno-error=coverage-mismatch option will be overridden > if such attributes are used.
Not sure if I understand the comment on the 'option be overriden' -- this is not happening with the patch. As long as the the diagnostic_classification for coverage-mismatch is explicitly specified, it will be honored. > > So the right place to set this is probably later, in process_options. And > this can check global_options_set.x_warnings_are_errors to see if an > explicit -Werror/-Wno-error option was passed. The problem is that when warning_as_error_requested is 0, there is no way to tell if it is the default or it is user has specified -Wno-error. Maybe we should not make -Wcoverage-mismatch warnings to be promoted to errors by default and relies on it to be turned on explicitly via -Werror, or -Werror=coverage-mismatch. There are probably not many people depending on the old behavior. Honza, what is your opinion on this? Thanks, David > > -- > Joseph S. Myers > jos...@codesourcery.com >