Re: FDO usage: -Wcoverage-mismatch should not ignore -Wno-error
On Thu, 21 Apr 2011, Xinliang David Li wrote: Please review the new patch. The new patch is OK with a suitable ChangeLog entry. -- Joseph S. Myers jos...@codesourcery.com
Re: FDO usage: -Wcoverage-mismatch should not ignore -Wno-error
Ping .. David On Wed, Apr 20, 2011 at 9:34 AM, Xinliang David Li davi...@google.com wrote: This would work if there is a way to set Werror=coverage-mismatch without having to explicitly set the option classification as DK_ERROR. Does this mechanism exist? Thanks, David On Tue, Apr 19, 2011 at 12:52 AM, Richard Guenther richard.guent...@gmail.com wrote: On Tue, Apr 19, 2011 at 9:13 AM, Xinliang David Li davi...@google.com wrote: -Wcoverage-mismatch is enabled by default, and the warning is promoted to error by default. However in the current implementation -Wno-error can not demote the error back to warning. The patch was ported from one contributed by Neil. OK for trunk after regression testing? I am sure there is a better way to achieve this, like making Werror=coverage-mismatch the default. Joseph? Richard. 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. The following test case can be added, but the test harness does not like the extra warnings -- how can they be pruned? Thanks, David /* { dg-options -O2 -Wcoverage-mismatch -Wno-error } */ int __attribute__((noinline)) bar (void) { } #ifdef _PROFILE_USE int foo (int i) { if (i) bar (); else bar (); return 0; } #else int foo (int i) { if (i) bar (); return 0; } #endif int main(int argc, char **argv) { foo (argc); return 0; }
Re: FDO usage: -Wcoverage-mismatch should not ignore -Wno-error
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. 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. -- Joseph S. Myers jos...@codesourcery.com
Re: FDO usage: -Wcoverage-mismatch should not ignore -Wno-error
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
Re: FDO usage: -Wcoverage-mismatch should not ignore -Wno-error
On Thu, 21 Apr 2011, Xinliang David Li wrote: 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 I am referring to the state before the patch. But in general decode_options is the wrong place for any once-only initialization. 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 I referred to global_options_set.x_warnings_are_errors, not warning_as_error_requested. -- Joseph S. Myers jos...@codesourcery.com
Re: FDO usage: -Wcoverage-mismatch should not ignore -Wno-error
Please review the new patch. Thanks, David On Thu, Apr 21, 2011 at 3:59 PM, Joseph S. Myers jos...@codesourcery.com wrote: On Thu, 21 Apr 2011, Xinliang David Li wrote: 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 I am referring to the state before the patch. But in general decode_options is the wrong place for any once-only initialization. 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 I referred to global_options_set.x_warnings_are_errors, not warning_as_error_requested. -- Joseph S. Myers jos...@codesourcery.com wc2.p Description: Binary data
Re: FDO usage: -Wcoverage-mismatch should not ignore -Wno-error
This would work if there is a way to set Werror=coverage-mismatch without having to explicitly set the option classification as DK_ERROR. Does this mechanism exist? Thanks, David On Tue, Apr 19, 2011 at 12:52 AM, Richard Guenther richard.guent...@gmail.com wrote: On Tue, Apr 19, 2011 at 9:13 AM, Xinliang David Li davi...@google.com wrote: -Wcoverage-mismatch is enabled by default, and the warning is promoted to error by default. However in the current implementation -Wno-error can not demote the error back to warning. The patch was ported from one contributed by Neil. OK for trunk after regression testing? I am sure there is a better way to achieve this, like making Werror=coverage-mismatch the default. Joseph? Richard. 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. The following test case can be added, but the test harness does not like the extra warnings -- how can they be pruned? Thanks, David /* { dg-options -O2 -Wcoverage-mismatch -Wno-error } */ int __attribute__((noinline)) bar (void) { } #ifdef _PROFILE_USE int foo (int i) { if (i) bar (); else bar (); return 0; } #else int foo (int i) { if (i) bar (); return 0; } #endif int main(int argc, char **argv) { foo (argc); return 0; }
FDO usage: -Wcoverage-mismatch should not ignore -Wno-error
-Wcoverage-mismatch is enabled by default, and the warning is promoted to error by default. However in the current implementation -Wno-error can not demote the error back to warning. The patch was ported from one contributed by Neil. OK for trunk after regression testing? 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. The following test case can be added, but the test harness does not like the extra warnings -- how can they be pruned? Thanks, David /* { dg-options -O2 -Wcoverage-mismatch -Wno-error } */ int __attribute__((noinline)) bar (void) { } #ifdef _PROFILE_USE int foo (int i) { if (i) bar (); else bar (); return 0; } #else int foo (int i) { if (i) bar (); return 0; } #endif int main(int argc, char **argv) { foo (argc); return 0; } wc.p Description: Binary data
Re: FDO usage: -Wcoverage-mismatch should not ignore -Wno-error
On Tue, Apr 19, 2011 at 9:13 AM, Xinliang David Li davi...@google.com wrote: -Wcoverage-mismatch is enabled by default, and the warning is promoted to error by default. However in the current implementation -Wno-error can not demote the error back to warning. The patch was ported from one contributed by Neil. OK for trunk after regression testing? I am sure there is a better way to achieve this, like making Werror=coverage-mismatch the default. Joseph? Richard. 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. The following test case can be added, but the test harness does not like the extra warnings -- how can they be pruned? Thanks, David /* { dg-options -O2 -Wcoverage-mismatch -Wno-error } */ int __attribute__((noinline)) bar (void) { } #ifdef _PROFILE_USE int foo (int i) { if (i) bar (); else bar (); return 0; } #else int foo (int i) { if (i) bar (); return 0; } #endif int main(int argc, char **argv) { foo (argc); return 0; }